Created attachment 29874 [details] Patch to fix the bug In my case client is processing user's InputStream and then sending data in chunks to server. At server end I read till EOF. Client can be simple Java program or a web application. I am facing issue when client gets error on user's InputStream and closes connection with server. In that case ChunkedInputFilter is returning -1. I have added logs and confirmed that endChunk is false. Issue was fixed after I added following just before "return result;" if (result == -1 && !endChunk) throw new EOFException("Unexpected end of stream while reading chunk body"); I think ChunkedInputFilter should return -1 only when it gets endChunk. Also, from client end I am always sending complete chunk. So, in ChunkedInputFilter I should get EOF from socket stream while reading next chunk header (which is handled properly in ChunkedInputFilter) but that is not happening. I have attached patch. Patch was generated against tomcat/tc7.0.x/trunk
Which HTTP connector are you using? I'm having trouble reproducing this. I'm not against the patch but I'd like to understand what is going on first.
Restoring the severity to something sensible. Need requested information to progress this.
I found the issue after we switched to protocol="HTTP/1.1". But I have tested my patch with "HTTP/1.1" as well as NIO connector. I am getting EOFException("Unexpected end of stream while reading chunk body") with both. Client is sending 8kB chunks. The test cases that are failing send around 18mB data before closing connection. Why I was not getting EOF in my servlet with NIO connector is something I need to check. ChunkedInputFilter is returning -1 for sure. I wanted to check that before giving update here but not getting time. Will check and give details as soon as possible. May be some time next week. One sure way of reproducing the issue is setting chunk size as 200 and sending only 100 bytes and then closing connection. My test case is not doing that and why is it happening in my case is something I need to check. I am sending complete 8kB chunks from client. In my case it should fail while reading header (which is handled properly).
The following lines in ChunkedInputFilter.java (162-164 in trunk) if (pos >= lastValid) { readBytes(); } is the only place in this file where the return value of "readBytes()" is not checked. Looking at the code, if the method returns -1 then (lastValid - pos) will be -1 and it will proceed with "chunk.setBytes(buf, pos, result);", which seems a wrong thing to do in such a case. I have not tested what is the actual behaviour here, but an explicit "return -1" or "throw new EOFException()" would be better.
(In reply to comment #4) > "throw new EOFException()" would be better. On a thought, it would be better to use org.apache.catalina.connector.ClientAbortException or a generic IOException. It is abnormal abort of connection by the client. It is not a proper end of stream.
Tested with protocol="HTTP/1.1" and <async-supported>false</async-supported> Took latest code from trunk and then added following debug statements to ChunkedInputFilter. if (pos >= lastValid) { int readBytes = readBytes(); if (readBytes <= 0){ System.out.println("ChunkedInputFilter.doRead()1 readBytes returned : " + readBytes); } } if (remaining > (lastValid - pos)) { result = lastValid - pos; System.out.println("ChunkedInputFilter.doRead()2 result: " + result + " remaining: " + remaining + " lastValid: " + lastValid + " pos: " + pos); Output on console ChunkedInputFilter.doRead()1 readBytes returned : -1 ChunkedInputFilter.doRead()2 result: -1 remaining: 8192 lastValid: 334 pos: 335 Method returns -1, which is treated as normal EOF. And application ends up processing incomplete data. "return -1" won't do as that is what is being returned. "throw new EOFException("Unexpected end of stream while reading chunk body");" or "throw new org.apache.catalina.connector.ClientAbortException()" looks appropriate.
(In reply to comment #5) > On a thought, it would be better to use > org.apache.catalina.connector.ClientAbortException or a generic IOException. ClientAbortException is a disallowed import for org.apache.coyote. It will have to be IOException.
Fixed in trunk and 7.0.x and will be included in 7.0.37 onwards.
(In reply to comment #8) > Fixed in trunk and 7.0.x and will be included in 7.0.37 onwards. 7.0.37 or 7.0.36? Fix in 7.0.36 (if possible) would help a lot. We are already in production and can't use dev build.
Which part of "will be included in 7.0.37 onwards" was unclear?
Sorry for bothering you. I was just confirming as current release is 7.0.35.
I proposed backport of r1443430 to 6.0.
Fixed in Tomcat 6 by r1476544 , will be in 6.0.37.