Problems with wrong Transfer-Encoding chunked decoding in cowboy_http:te_decoding #481

Closed
maxlapshin opened this Issue Mar 26, 2013 · 11 comments

Comments

Projects
None yet
3 participants
@maxlapshin
Contributor

maxlapshin commented Mar 26, 2013

Problem is:

1 packet with 1460 bytes:

«49,55,56,13,10,71, ...., 13,10,53,50,52,13,10,71,....48,48»

it is decoded in cowboy_req:transfer_decode to

{ok, «Bin:376/binary», «53,50,52,13,10,71,64,0,17...» = «_:1077/binary», {0,376}}.

which is ok: 1460 bytes, 3 bytes for first chunk length, 4 bytes for two "\r\n"

Then transfer_decode is launched is 1077 bytes and {0,376} as a TransferState. Decoding of [53,50,52] should tell us that 1316 bytes long. cowboy_http:te_chunked returns:

{more, 1318, «71,64,0,17,..» = «_:1072/binary», {1316,376}}

We need now to download only 1316 - 1072 = 244 bytes, but cowboy_http:te_chunked returns that 1318 must be downloaded. Cowboy fails to download 1318 bytes and returns with timeout.

It looks like if

te_chunked(Data, {ChunkRem, Streamed}) ->
{more, ChunkRem + 2, Data, {ChunkRem, Streamed}}.

is buggy, because ChunkRem must be not ChunkRem, but ChunkLength and so it should be:

te_chunked(Data, {ChunkLength, Streamed}) ->
{more, ChunkLength + 2 - byte_size(Data), Data, {ChunkLength, Streamed}}.

But it doesn't help, because transfer_encoding is soo complicated that I can't understand how to fix it.

Right now it doesn't work =(

@essen

This comment has been minimized.

Show comment Hide comment
@essen

essen Mar 26, 2013

Member

What client?

Member

essen commented Mar 26, 2013

What client?

@maxlapshin

This comment has been minimized.

Show comment Hide comment
@maxlapshin

maxlapshin Mar 26, 2013

Contributor

Just a plain erlang code, running via localhost

Can this be an issue?

Contributor

maxlapshin commented Mar 26, 2013

Just a plain erlang code, running via localhost

Can this be an issue?

@essen

This comment has been minimized.

Show comment Hide comment
@essen

essen Mar 26, 2013

Member

Chunked works like this:

Size1\r\n
Data1\r\n
Size2\r\n
Data2\r\n
...

Chances are you are missing a \r\n there.

Member

essen commented Mar 26, 2013

Chunked works like this:

Size1\r\n
Data1\r\n
Size2\r\n
Data2\r\n
...

Chances are you are missing a \r\n there.

@maxlapshin

This comment has been minimized.

Show comment Hide comment
@maxlapshin

maxlapshin Mar 26, 2013

Contributor

ok = gen_tcp:send(Sock, [integer_to_list(iolist_size(D),16), "\r\n",D,"\r\n"]);

I've explained you the problem: cowboy is trying to receive from socket not the rest of chunk, but whole chunk again and it fails with timeout.

Contributor

maxlapshin commented Mar 26, 2013

ok = gen_tcp:send(Sock, [integer_to_list(iolist_size(D),16), "\r\n",D,"\r\n"]);

I've explained you the problem: cowboy is trying to receive from socket not the rest of chunk, but whole chunk again and it fails with timeout.

@essen

This comment has been minimized.

Show comment Hide comment
@essen

essen Mar 26, 2013

Member

I can't really read your ticket, please add formatting so that < characters don't get eaten.

Member

essen commented Mar 26, 2013

I can't really read your ticket, please add formatting so that < characters don't get eaten.

@maxlapshin

This comment has been minimized.

Show comment Hide comment
@maxlapshin

maxlapshin Mar 26, 2013

Contributor

Oh, sorry, Löic. I've updated ticket and changed binary borders to « and » symbols.

Contributor

maxlapshin commented Mar 26, 2013

Oh, sorry, Löic. I've updated ticket and changed binary borders to « and » symbols.

@essen

This comment has been minimized.

Show comment Hide comment
@essen

essen Mar 26, 2013

Member

The part you point out isn't wrong. I'll need to take a closer look. Just got back from SF so might not be before tomorrow.

Member

essen commented Mar 26, 2013

The part you point out isn't wrong. I'll need to take a closer look. Just got back from SF so might not be before tomorrow.

@maxlapshin

This comment has been minimized.

Show comment Hide comment
@maxlapshin

maxlapshin Mar 26, 2013

Contributor

Ok, maybe I'm mistakening, but I have a good reproduceable example.

I'll post it a bit later.

Contributor

maxlapshin commented Mar 26, 2013

Ok, maybe I'm mistakening, but I have a good reproduceable example.

I'll post it a bit later.

@essen

This comment has been minimized.

Show comment Hide comment
@essen

essen Apr 11, 2013

Member

Have you found anything else?

Member

essen commented Apr 11, 2013

Have you found anything else?

@benoitc

This comment has been minimized.

Show comment Hide comment
@benoitc

benoitc May 31, 2013

I reproduce it here as well. Sending a blob as binary with curl return an {error, timeout}. I will try to provide a simple test case but a simple curl command was enough to troubleshout a somewhat simple handler [1]:

$ curl -v -X PUT  -H"Content-Type: data/octer-stream" -H "Transfer-Encoding: chunked" --data-binary @/home/benoitc/Downloads/f0g localhost:8080/default/hashtype-testabcdefgh890101993
* About to connect() to localhost port 8080 (#0)
*   Trying 127.0.0.1...
* Connected to localhost (127.0.0.1) port 8080 (#0)
> PUT /default/hashtype-testabcdefgh890101993 HTTP/1.1
> User-Agent: curl/7.29.0
> Host: localhost:8080
> Accept: */*
> Content-Type: data/octer-stream
> Transfer-Encoding: chunked
> Expect: 100-continue
> 
< HTTP/1.1 100 Continue

< HTTP/1.1 500 Internal Server Error
< connection: keep-alive
< server: Cowboy
< date: Fri, 31 May 2013 04:10:49 GMT
< content-length: 0
< 

[1] https://github.com/refuge/coffer/blob/master/src/coffer_http_blob.erl#L118

benoitc commented May 31, 2013

I reproduce it here as well. Sending a blob as binary with curl return an {error, timeout}. I will try to provide a simple test case but a simple curl command was enough to troubleshout a somewhat simple handler [1]:

$ curl -v -X PUT  -H"Content-Type: data/octer-stream" -H "Transfer-Encoding: chunked" --data-binary @/home/benoitc/Downloads/f0g localhost:8080/default/hashtype-testabcdefgh890101993
* About to connect() to localhost port 8080 (#0)
*   Trying 127.0.0.1...
* Connected to localhost (127.0.0.1) port 8080 (#0)
> PUT /default/hashtype-testabcdefgh890101993 HTTP/1.1
> User-Agent: curl/7.29.0
> Host: localhost:8080
> Accept: */*
> Content-Type: data/octer-stream
> Transfer-Encoding: chunked
> Expect: 100-continue
> 
< HTTP/1.1 100 Continue

< HTTP/1.1 500 Internal Server Error
< connection: keep-alive
< server: Cowboy
< date: Fri, 31 May 2013 04:10:49 GMT
< content-length: 0
< 

[1] https://github.com/refuge/coffer/blob/master/src/coffer_http_blob.erl#L118

@essen

This comment has been minimized.

Show comment Hide comment
@essen

essen Jul 12, 2013

Member

Should be fixed by the above merged PR. Thanks!

Member

essen commented Jul 12, 2013

Should be fixed by the above merged PR. Thanks!

@essen essen closed this Jul 12, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment