Closed Bug 654577 Opened 13 years ago Closed 10 years ago

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

Categories

(Core :: Networking: HTTP, defect)

Product:
Core
Component:
Networking: HTTP
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla33

People

(Reporter: drbrain+bugzilla, Assigned: bagder)

References

Details

Attachments

(1 file)

HTTP response for use by nc -l 8000
(deleted), text/plain
Details 
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.
Attached file HTTP response for use by nc -l 8000 (deleted) — 

    
    

    
  
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?

    
    

    
  
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

    
    

    
  
(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: nobody → daniel

    
    

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

    
    

    
  
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
Closed: 10 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.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: