Error in successCallBack when a header is passed in to LLHttpRequest

[abdiel-mireles]

Hi,

We noticed an issue when getting an API response back. The response could be truncated in certain cases and nowhere near our buffer limit. When we investigated we found that the length of the response copied happened to always match the length of the header. Upon closer inspection it seems like the incorrect value is being used as the length for the memcpy from the raw response to the user specified location (pResponse) in the code below.

I think what was meant to happen is the copy length would be the min between the length of the content and the length of the user specified buffer. However, what is actually happening is that the length of the header gets used and most of the response is lost.

This snippet of code is from https://github.com/loupeteam/LLHttp/blob/main/src/Ar/LLHttp/HttpRequest.c line 25.

void successCallback(LLHttpRequest_typ* t, LLHttpServiceLink_typ* api, LLHttpHeader_typ* header, unsigned long content) {
	if(t) {
		t->internal.error = 0;
		t->internal.done  = 1;
		t->internal.busy = 0;
		
		if(header) {
			memcpy(&t->header, header, sizeof(t->header));
			t->contentLength = t->header.contentLength;
			if(content) {
				unsigned long length = MIN(t->header.contentLength, t->responseSize);
				if(t->pResponse) memcpy((void*)t->pResponse, (void*)content, length);
			}
		}
	}
}

If this turns out to be a real issue, then a more sophisticated fix will be necessary. For now, we simply replaced length on the last line of code in this snippet with t->responseSize (which is the user specified length of response buffer). Slightly wasteful, but it works.

void successCallback(LLHttpRequest_typ* t, LLHttpServiceLink_typ* api, LLHttpHeader_typ* header, unsigned long content) {
	if(t) {
		t->internal.error = 0;
		t->internal.done  = 1;
		t->internal.busy = 0;
		
		if(header) {
			memcpy(&t->header, header, sizeof(t->header));
			//t->contentLength = t->header.contentLength;
			if(content) {
				//unsigned long length = MIN(t->header.contentLength, t->responseSize);
				if(t->pResponse) memcpy((void*)t->pResponse, (void*)content, t->responseSize);
			}
		}
	}
}

[Joshpolansky]

Is it possible the content length from the server is incorrect? How big is the content that you expect?

[abdiel-mireles]

I actually don't see a content-length header in the server's response, but I'm not sure if I'm looking into it correctly.

This is the raw data from the LLHTTP client:

This is what makes it into our response structure:

The content length without our modification gets changed to 7 in this example (although this number changes depending on the response) as shown here.

I also have a question. If contentLength is defined as an input and I am passing in a value, should it be overwritten by the function itself? Because it definitely is being overwritten.

[sclaiborne]

In the example message above there no content. You can see the string ends after $r$n$r$n. In the example shown I would expect contentLength to be 0, and responseBody to be empty.

[abdiel-mireles]

The content is a json packet that starts at {"data" on the rawRecvData[7] line.

[abdiel-mireles]

Same call working correctly on postman for reference:

[Joshpolansky]

It looks like the transfer-encoding is chunked instead of using content-length. I don’t think the client supports transfer-encoding currently. Support would need to be added.

In this case I think the e7 before the data indicates the size in bytes as hex

[abdiel-mireles]

I think you are right, it is the length in hex. Thanks for looking into it and the clarification. We are still going to use the library and use our workaround for the time being but adding support would be great.

Also Bishop says to say hi.

[abdiel-mireles]

Turns out the library does support chunked transfer-encoding, there is just a small bug in how the content-length is parsed. The hexStringToInteger function used to convert the content-length does not account for the hex string being lowercase. I tried to make a hotfix branch to do a PR but permission is denied.

In the HttpParse.c file, I changed the function to this:

unsigned int hexStringToInteger(const char *hex, int length) {
	unsigned int result = 0;
	for (int i = 0; i < length; i++) {
		result <<= 4; // shift left by 4 bits (making room for the next hex digit)       
		if (hex[i] >= '0' && hex[i] <= '9') {
			result += hex[i] - '0';
		} else if (hex[i] >= 'A' && hex[i] <= 'F') {
			result += hex[i] - 'A' + 10;
		} else if (hex[i] >= 'a' && hex[i] <= 'f') {
			result += hex[i] - 'a' + 10; 
		}
		 
	}
	return result;
}

The original looked like this:

unsigned int hexStringToInteger(const char *hex, int length) {
	unsigned int result = 0;
	for (int i = 0; i < length; i++) {
		result <<= 4; // shift left by 4 bits (making room for the next hex digit)       
		if (hex[i] >= '0' && hex[i] <= '9') {
			result += hex[i] - '0';
		} else if (hex[i] >= 'A' && hex[i] <= 'F') {
			result += hex[i] - 'A' + 10;
		} 
	}
	return result;
}