Transfer-Encoding: chunked does not give an error when the stream is closed before all data is received

RESOLVED FIXED in mozilla33

Status

()

Core
Networking: HTTP
RESOLVED FIXED
7 years ago
4 years ago

People

(Reporter: Eric Hodel, Assigned: bagder)

Tracking

unspecified
mozilla33
x86
Mac OS X
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

7 years ago
User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_6_7; en-us) AppleWebKit/533.21.1 (KHTML, like Gecko) Version/5.0.5 Safari/533.21.1
Build Identifier: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0.1) Gecko/20100101 Firefox/4.0.1

When receiving a chunk of a Transfer-Encoding chunked response if the connection is closed before all bytes in the chunk have been received Firefox does not give an error.

curl and Safari both indicate an error if there is insufficient data in the chunked stream:

$ cat chunkfail.txt 
HTTP/1.1 200 OK
Content-Type: text/html
Transfer-Encoding: chunked

22
<!DOCTYPE html>
<h1>off by 1</h1
$ od -a chunkfail.txt
0000000    H   T   T   P   /   1   .   1  sp   2   0   0  sp   O   K  cr
0000020   nl   C   o   n   t   e   n   t   -   T   y   p   e   :  sp   t
0000040    e   x   t   /   h   t   m   l  cr  nl   T   r   a   n   s   f
0000060    e   r   -   E   n   c   o   d   i   n   g   :  sp   c   h   u
0000100    n   k   e   d  cr  nl  cr  nl   2   2  cr  nl   <   !   D   O
0000120    C   T   Y   P   E  sp   h   t   m   l   >  nl   <   h   1   >
0000140    o   f   f  sp   b   y  sp   1   <   /   h   1  nl            
0000155
$ nc -l 8000 < chunkfail.txt &
[1] 67013
$ curl localhost:8000
GET / HTTP/1.1
User-Agent: curl/7.19.7 (universal-apple-darwin10.0) libcurl/7.19.7 OpenSSL/0.9.8l zlib/1.2.3
Host: localhost:8000
Accept: */*

<!DOCTYPE html>
<h1>off by 1</h1
curl: (18) transfer closed with outstanding read data remaining
[1]+  Done                    nc -l 8000 < chunkfail.txt
$


Reproducible: Always

Steps to Reproduce:
1. Download chunkfail.txt
2. nc -l 8000 < chunkfail.txt
3. open http://localhost:8000

Actual Results:  
Body displays

Expected Results:  
An error indicating the connection was terminated before all data was received.  For example, Safari displays:


Safari can’t open the page “http://localhost:8000/”. The error is: “The operation couldn’t be completed. (kCFErrorDomainCFNetwork error 303.)”

And curl displays:

transfer closed with outstanding read data remaining

Safari does not show an error if the chunk size is accurate but the last-chunk is not sent by the sever ("0\r\n" and the terminating "\r\n", see RFC 2616 section 3.6.1).

Neither Firefox nor Safari require the last-chunk.  This bug is not about a failure to require a last-chunk.
(Reporter)

Comment 1

7 years ago
Created attachment 529846 [details]
HTTP response for use by nc -l 8000
Why is aborting the load and showing an error page (which is what it sounds like Safari does) preferable to showing the data we received so far the way we do for other sorts of network errors?
(Reporter)

Comment 3

7 years ago
When the connection is closed at the end of a chunk it's more likely that the content is complete and the server is broken.

When the connection is closed in the middle of a chunk it's more likely that the content is incomplete and the request should be retried automatically (if that's safe and reasonable) or the option to retry should be passed to the user.

I would prefer to know that something is possibly broken when it's reasonably true.
the patch in 237623 will serve as a compromise here.. the error will be detected at the channel level but that won't effect normal rendering (as most errors do not - it just truncates).. but something like the download manager will be able to flag it, and perhaps more importantly the cache won't store it.
Depends on: 237623
(Assignee)

Comment 5

4 years ago
(In reply to Patrick McManus [:mcmanus] from comment #4)
> the patch in 237623 will serve as a compromise here.. the error will be
> detected at the channel level but that won't effect normal rendering (as
> most errors do not - it just truncates).. but something like the download
> manager will be able to flag it, and perhaps more importantly the cache
> won't store it.

The patch in Bug 237623 is however pretty custom made for Content-Length checking. I believe we should have similarly "strict" rules for chunk sizes but I think we may need additional checks for that. Also, I think we can be more certain that contents that are sent chunked may not be resumable so in the download manager case we may want to handle the two different cases slightly different.
(Assignee)

Updated

4 years ago
Assignee: nobody → daniel
(Assignee)

Comment 6

4 years ago
I submitted a patch to Bug 237623 that also will fix this bug: https://bugzilla.mozilla.org/attachment.cgi?id=8389842&action=edit
(Assignee)

Comment 7

4 years ago
This is now also resolved as part of the fix in bug 237623, and is tested with new test cases provided there.
Status: UNCONFIRMED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Flags: in-testsuite+
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.