esphttpclient icon indicating copy to clipboard operation
esphttpclient copied to clipboard

Body data not followed by \0 when chunked

Open hallard opened this issue 7 years ago • 2 comments

First of all, thanks for this awesome library, I'm using it and I'm happy with it. But I think I found a bug in case of chunked content

I've put a print of decoded buffer of received content, in disconnect_callback of httpclient.c so you can see from where in the orig file I'm displaying traces

		// FIXME: make sure this is not a partial response, using the Content-Length header.

			const char * version10 = "HTTP/1.0 ";
			const char * version11 = "HTTP/1.1 ";
			if (os_strncmp(req->buffer, version10, strlen(version10)) != 0
			 && os_strncmp(req->buffer, version11, strlen(version11)) != 0) {
				os_printf("Invalid version in %s\n", req->buffer);
			}
			else {
				http_status = atoi(req->buffer + strlen(version10));

				os_printf("Received buffer[%d]\n",req->buffer_size);
				hex_dump(req->buffer, req->buffer_size);

				/* find body and zero terminate headers */
				body = (char *)os_strstr(req->buffer, "\r\n\r\n") + 2;
				*body++ = '\0';
				*body++ = '\0';

				body_size = req->buffer_size - (body - req->buffer);

				if(os_strstr(req->buffer, "Transfer-Encoding: chunked"))
				{
					body_size = chunked_decode(body, body_size);
				}
			}
		}

		os_printf("Decoded buffer[%d]\n",body_size);
		hex_dump(body, body_size+8); // Add last 8 char to see following data

and here the results, as you can see the 1st display in the trace is corect

get http://192.168.1.34:8080/version1
Received buffer[227]
  0000  48 54 54 50 2f 31 2e 31 20 32 30 30 20 4f 4b 0d  HTTP/1.1 200 OK.
  0010  0a 43 6f 6e 74 65 6e 74 2d 54 79 70 65 3a 20 74  .Content-Type: t
  0020  65 78 74 2f 6a 73 6f 6e 0d 0a 44 61 74 65 3a 20  ext/json..Date: 
  0030  46 72 69 2c 20 33 31 20 4d 61 72 20 32 30 31 37  Fri, 31 Mar 2017
  0040  20 31 31 3a 30 36 3a 33 32 20 47 4d 54 0d 0a 43   11:06:32 GMT..C
  0050  6f 6e 6e 65 63 74 69 6f 6e 3a 20 63 6c 6f 73 65  onnection: close
  0060  0d 0a 54 72 61 6e 73 66 65 72 2d 45 6e 63 6f 64  ..Transfer-Encod
  0070  69 6e 67 3a 20 63 68 75 6e 6b 65 64 0d 0a 0d 0a  ing: chunked....
  0080  35 37 0d 0a 7b 22 76 65 72 73 69 6f 6e 22 3a 22  57..{"version":"
  0090  31 2e 30 2e 30 22 2c 22 66 69 6c 65 22 3a 22 2f  1.0.0","file":"/
  00a0  75 70 64 61 74 65 2f 75 73 65 72 31 2e 62 69 6e  update/user1.bin
  00b0  22 2c 22 6d 64 35 22 3a 22 34 30 39 65 62 61 63  ","md5":"409ebac
  00c0  35 36 30 39 30 62 33 64 31 66 36 34 33 64 30 62  56090b3d1f643d0b
  00d0  65 37 36 30 31 34 30 62 66 22 7d 0d 0a 30 0d 0a  e760140bf"}..0..
  00e0  0d 0a 00                                         ...

But when decoded (unchuncked) , data has been moved (wich is normal) but not the \0, so we're missing the \0 at the good place and got some more char in the string, even if the body_size var is correct

In the dump below I added 8 char mode than the real body_size to see buffer content after size

Decoded buffer[87]
  0000  7b 22 76 65 72 73 69 6f 6e 22 3a 22 31 2e 30 2e  {"version":"1.0.
  0010  30 22 2c 22 66 69 6c 65 22 3a 22 2f 75 70 64 61  0","file":"/upda
  0020  74 65 2f 75 73 65 72 31 2e 62 69 6e 22 2c 22 6d  te/user1.bin","m
  0030  64 35 22 3a 22 34 30 39 65 62 61 63 35 36 30 39  d5":"409ebac5609
  0040  30 62 33 64 31 66 36 34 33 64 30 62 65 37 36 30  0b3d1f643d0be760
  0050  31 34 30 62 66 22 7d 62 66 22 7d 0d 0a 30 0d     140bf"}bf"}..0.

To correct this I added body[body_size] = '\0' ;

if(os_strstr(req->buffer, "Transfer-Encoding: chunked"))
{
	body_size = chunked_decode(body, body_size);
	body[body_size] = `\0`;
}

And now results are fine, see the 0 at pos 0x57 ;-)

Decoded buffer[87]
  0000  7b 22 76 65 72 73 69 6f 6e 22 3a 22 31 2e 30 2e  {"version":"1.0.
  0010  30 22 2c 22 66 69 6c 65 22 3a 22 2f 75 70 64 61  0","file":"/upda
  0020  74 65 2f 75 73 65 72 31 2e 62 69 6e 22 2c 22 6d  te/user1.bin","m
  0030  64 35 22 3a 22 36 62 37 61 34 37 38 64 36 36 63  d5":"6b7a478d66c
  0040  31 61 30 63 35 36 34 34 38 63 31 37 32 63 33 61  1a0c56448c172c3a
  0050  33 35 30 36 38 22 7d 00 38 22 7d 0d 0a 30 0d     35068"}.8"}..0.

hallard avatar Mar 31 '17 11:03 hallard

Yeah I came across the same issue.

skwijeratne avatar Apr 05 '17 13:04 skwijeratne

Thank you @hallard, you are right.

Caerbannog avatar Jul 23 '17 18:07 Caerbannog