Edge case with socket becoming NULL
Helge Heß
me at helgehess.eu
Sun Aug 2 18:21:18 EDT 2009
On 02.08.2009, at 23:54, Joe Orton wrote:
>> code section, ne_request.c, Neon 0.28.5:
>> ---snip---
>> static int read_message_header(ne_request *req, char *buf, size_t
>> buflen)
>> {
>> ssize_t n;
>> ne_socket *sock = req->session->socket;
>>
>> n = ne_sock_readline(sock, buf, buflen);
>> ---snap---
>>
>> I've seen the 'sock' ptr become NULL during debugging which then
>> leads
>> to subsequent NULL-ptr deref.
>
> Are you calling ne_begin_request/ne_end_request manually, or via
> ne_request_dispatch?
Manually.
> Note that if you're calling ne_begin_request(), you must not call
> ne_end_request() if a ne_read_response_block() call fails - the API is
> perhaps not clear on this point.
Hm. I'll recheck that. Could be an issue. Couldn't
ne_read_response_block set a flag which signals ne_end_request that it
should perform as a noop?
We do:
- ne_begin_request
- ne_get_status
- ne_response_header_iterate
- ne_discard_response
- ne_end_request
Attached is a code exerpt, maybe you get the idea ...
Thanks,
Helge
---snip---
do {
int statCode;
stat = NULL;
Z1DESTROY(lastNeonError);
this->_ResetResponse();
assert(this->response == NULL);
this->response = new Z1HttpResponse();
/* start request */
if (this->connection != NULL) {
if (!this->connection->WillSendRequest(this->request)) {
if (this->log != NULL) {
this->log->PrintF("%s: WillSendRequest() stopped send",
"Z1HttpTransaction::Run");
}
return false;
}
}
if (this->log != NULL) {
this->log->PrintF("%s: Send %s %s",
"Z1HttpTransaction::Run",
Z1StringUTF8(this->request->Method()),
Z1StringUTF8(this->request->Path()));
this->request->Dump(this->log);
}
if (rq == NULL) {
if (this->log != NULL) {
this->log->PrintF("%s: RePrepare request, was NULL",
"Z1HttpTransaction::Run");
}
if (!this->Prepare()) {
if (this->log != NULL) {
this->log->PrintF("%s: ERROR failed to RePrepare request.",
"Z1HttpTransaction::Run");
}
rc = NE_ERROR;
break;
}
}
assert(rq != NULL);
if (rq == NULL) {
rc = NE_ERROR;
break;
}
rc = ne_begin_request(rq);
if (rc != NE_OK) {
/* this still happens, eg with SOGo Task-Delete which issues
* 2 subsequent GETs. The second one fails with:
* 1 and "Could not parse response status line"
*
* Easy to reproduce with SOGo and persistent connections,
* just do a few moves.
* ne_parse_statusline() gets this input (why 8859-1?):
* <?xml version="1.0" encoding="ISO-8859-1"?>
* probably an old buffer?
*/
/* Also seen this:
* "Could not resolve hostname `sogo-demo.inverse.ca': The
requested
* name is valid and was found in the database, but it does
not
* have the correct associated data being resolved for."
* This seems to be Winsock Error WSANO_DATA aka 11004.
*/
const char *ne = ne_get_error((ne_session *)this-
>ConnectionHandle());
if (this->log != NULL) {
this->log->PrintF("%s: begin_request failed: %i: %s (close-
retry=%i)",
"Z1HttpTransaction::Run", rc, ne, closeRetryCount);
}
if (ne != NULL && strstr(ne, "Could not read status line") ==
NULL)
ne = NULL;
if (ne != NULL && strstr(ne, "closed by server") == NULL)
ne = NULL;
this->_CloseConnection(); // better be sure, close it
if (ne != NULL && closeRetryCount < 3) {
closeRetryCount++;
if (this->log != NULL)
this->log->PrintF("%s: retry after close ...",
"Z1HttpTransaction::Run");
continue; /* try again */
}
else if (ne != NULL) {
if (this->log != NULL) {
this->log->PrintF("%s: NO retry, already tried %i times.",
"Z1HttpTransaction::Run", closeRetryCount);
}
}
else {
if (this->log != NULL)
this->log->PrintF("%s: NO retry, other error.",
"Z1HttpTransaction::Run");
}
break;
}
/* check status */
stat = ne_get_status(rq);
statCode = stat->code;
this->response->SetStatus(statCode);
if (this->connection != NULL)
this->connection->DidSendRequest(this->request);
/* copy response headers into Z1HttpResponse */
void *itCursor = NULL;
const char *headerKey = NULL, *headerValue = NULL;
// Google: Server: GFE/1.3
while ((itCursor = ne_response_header_iterate((ne_request*)this-
>handle,
itCursor,
&headerKey, &headerValue)))
{
this->response->AddHeaderForKeyAndReleaseBoth
(Z1String::New(headerValue), z1HeaderKey(headerKey));
}
this->response->Dump(this->log);
#if 0 // yes, this happens with iCal Server when the content is
transferred chunked
if (Z1IsEmpty(this->response-
>HeaderForKey(Z1HttpHeader_ContentLength))) {
ne_request *nrq = (ne_request*)this->handle;
const char *clen = ne_get_response_header(nrq, "content-length");
if (this->log != NULL)
this->log->PrintF("CLEN: %s", clen);
}
#endif
stat = NULL; // pointer into request, be sure we don't use it
anymore
/* read response */
if (this->connection != NULL) {
if (!this->connection->WillReceiveResponse(this->response)) {
if (this->log != NULL) {
this->log->PrintF("%s: WillReceiveResponse() stopped
receive",
"Z1HttpTransaction::Run");
}
if (rq != NULL) ne_end_request(rq);
/* close connection. We did not read the content, hence it
become
* b0rked. */
this->_CloseConnection(); // better be sure, close it
return false;
}
}
if (this->log != NULL) {
this->log->PrintF("%s: receive response (status=%i)",
"Z1HttpTransaction::Run", this->response->Status());
}
/* read response */
// calls ne_read_response_block until it returns 0
// returns != 0 on error
this->readCount = 0;
if (ne_discard_response(rq) == NE_OK /* SUCCESS */) {
if (this->response != NULL && this->responseData != NULL) {
this->response->SetContentAndRelease(this->responseData);
this->responseData = NULL;
#if DEBUG
double ratio = this->response->GetCompressionRatio();
if (ratio > 1 && this->log != NULL) {
this->log->PrintF("%s: compression: %0.3f",
"Z1HttpTransaction::Run", ratio);
}
#endif
}
/* close request, if it returns NE_RETRY, retry ... */
rc = ne_end_request(rq);
if (rc != NE_OK && rc != NE_RETRY) {
const char *nes = ne_get_error((ne_session *)this-
>ConnectionHandle());
if (nes != NULL) { // copy, it might go away on con-close!
assert(lastNeonError == NULL);
lastNeonError = Z1String::New(nes);
}
}
if (this->connection != NULL)
this->connection->DidReceiveResponse(this->response);
}
else { /* ne_discard_response returned non-0 */
/* close request, if it returns NE_RETRY, retry ... */
// TBD: is it correct to call ne_end_request if
ne_discard_response failed?
rc = rq != NULL ? ne_end_request(rq) : NE_ERROR;
if (rc != NE_OK && rc != NE_RETRY) {
const char *nes = ne_get_error((ne_session *)this-
>ConnectionHandle());
if (nes != NULL) { // copy, it might go away on con-close!
assert(lastNeonError == NULL);
lastNeonError = Z1String::New(nes);
}
}
this->_CloseConnection(); // better be sure, close it
}
if (this->log != NULL) {
if (rc == NE_RETRY) { // eg this is entered on 401
this->log->PrintF("%s: got a NE_RETRY %i ...",
"Z1HttpTransaction::Run", statCode);
}
else if (rc == NE_AUTH) {
// we should never enter this?
this->log->PrintF("%s: got a NE_AUTH %i: %s",
"Z1HttpTransaction::Run", statCode,
Z1StringUTF8(lastNeonError));
}
else if (rc != NE_OK) {
this->log->PrintF("%s: got a error code %i, stat %i: %s",
"Z1HttpTransaction::Run", rc, statCode,
Z1StringUTF8(lastNeonError));
}
}
/* hack to workaround Neon digest bug (exits if the nounce
expired!) */
if (rc == NE_AUTH && digestRetryCount < 3 && this->response !=
NULL) {
const wchar_t *wwwAuthenticate =
Z1StringWide(this->response-
>StringHeaderForKey(Z1HttpHeader_WwwAuthenticate));
wwwAuthenticate = z1_wcs_strstr(wwwAuthenticate, -1,
"digest", true /* skipsp */, true /* ig case */);
wwwAuthenticate = z1_wcs_strstr(wwwAuthenticate, -1,
"stale", true /* skipsp */, true /* ig case */);
if (wwwAuthenticate != NULL) {
digestRetryCount++;
rc = NE_RETRY;
if (this->log != NULL) {
this->log->PrintF("%s: WARN retry after stale digest nounce",
"Z1HttpTransaction::Run");
}
}
this->_CloseConnection(); // better be sure, close it
}
}
while (rc == NE_RETRY);
if (rc != NE_OK && this->response != NULL) {
this->response->SetErrorCode(rc);
if (lastNeonError != NULL)
this->response->SetHeaderForKey(lastNeonError, L"X-NEON-ERROR");
if (rc == NE_ERROR || rc == NE_LOOKUP) {
this->response->SetContentAsString
(ne_get_error((ne_session *)this->ConnectionHandle()));
}
}
if (this->log != NULL && this->response != NULL) {
this->log->PrintF("%s: Received response %i (read=%d)",
"Z1HttpTransaction::Run", this->response->Status(), this-
>readCount);
this->response->Dump(this->log);
if (this->okOut != NULL) {
this->log->PrintF("%s: [the output was streamed]",
"Z1HttpTransaction::Run");
}
}
/* teardown */
if (this->decompress != NULL) {
ne_decompress_destroy((ne_decompress *)this->decompress);
this->decompress = NULL;
}
if (this->handle != NULL) {
ne_request_destroy((ne_request *)this->handle);
this->handle = NULL;
}
---snap---
More information about the neon
mailing list