Well it looks like the else is trying to "correct" the error where there is a spurious '\r' by writing the full /r/n to the output stream and trys again. But it doesn't try to correct for a '\n' in the same way (it just writes it out in state 0). This does not look particularly valuable.
Is it possible that we would be reading over some binary data that might have some valid \r or \n hex values? In that case we should try to correct for the spurious '\n' as well ... Otherwise throwing a IOException seems prudent. Ortwin Glück wrote: > As suggested by Martin Elwin. > > Patch is for .../src/java/ > > Should we throw an Exception instead of this: > } else { > // this was not CRLF, so now write '\r' + this char > baos.write('\r'); > baos.write(b); > state = 0; > } > > I mean having \r inside a chunk size field is illegal and thus a > protocol violation. Comments from the "real world"? > > ------------------------------------------------------------------------ > ? patch.diff > Index: org/apache/commons/httpclient/ChunkedInputStream.java > =================================================================== > RCS file: >/home/cvspublic/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/ChunkedInputStream.java,v > retrieving revision 1.3 > diff -u -w -r1.3 ChunkedInputStream.java > --- org/apache/commons/httpclient/ChunkedInputStream.java 12 Sep 2002 11:36:45 >-0000 1.3 > +++ org/apache/commons/httpclient/ChunkedInputStream.java 23 Sep 2002 14:13:24 >-0000 > @@ -134,7 +134,7 @@ > nextChunk(); > if (eof) return -1; > } > - len = Math.min(len, chunkSize); > + len = Math.min(len, chunkSize - pos); > int count = in.read(b, off, len); > pos += count; > return count; > @@ -172,22 +172,30 @@ > */ > private static int getChunkSizeFromInputStream(final InputStream in) throws >IOException { > ByteArrayOutputStream baos = new ByteArrayOutputStream(); > - int b = in.read(); > int state = 0; > while (state != 2) { > + int b = in.read(); > if (b == -1) throw new IOException("chunked stream ended unexpectedly"); > switch (state) { > case 0: > - if (b == '\r') state = 1; > + if (b == '\r') { > + state = 1; > + } else { > + baos.write(b); > + } > break; > case 1: > - if (b == '\n') state = 2; > - else state = 0; > + if (b == '\n') { > + state = 2; > + } else { > + // this was not CRLF, so now write '\r' + this char > + baos.write('\r'); > + baos.write(b); > + state = 0; > + } > break; > default: throw new RuntimeException("assertion failed"); > } > - if (state == 0) baos.write(b); > - if (state != 2) b = in.read(); > } > > //parse data > @@ -199,7 +207,7 @@ > > int result; > try { > - result = Integer.parseInt(dataString, 16); > + result = Integer.parseInt(dataString.trim(), 16); > } catch(NumberFormatException e) { > throw new IOException("Bad chunk size: "+dataString); > } > Index: org/apache/commons/httpclient/HttpConnection.java > =================================================================== > RCS file: >/home/cvspublic/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/HttpConnection.java,v > retrieving revision 1.20 > diff -u -w -r1.20 HttpConnection.java > --- org/apache/commons/httpclient/HttpConnection.java 8 Sep 2002 05:35:09 -0000 > 1.20 > +++ org/apache/commons/httpclient/HttpConnection.java 23 Sep 2002 14:13:26 -0000 > @@ -455,7 +455,7 @@ > */ > public InputStream getResponseInputStream(HttpMethod method) > throws IOException, IllegalStateException { > - log.trace("enter HttpConnection.getRequestOutputStream(HttpMethod)"); > + log.trace("enter HttpConnection.getResponseInputStream(HttpMethod)"); > assertOpen(); > return _input; > } > Index: org/apache/commons/httpclient/WireLogInputStream.java > =================================================================== > RCS file: >/home/cvspublic/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/WireLogInputStream.java,v > retrieving revision 1.3 > diff -u -w -r1.3 WireLogInputStream.java > > ------------------------------------------------------------------------ > -- > To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]> > For additional commands, e-mail: <mailto:[EMAIL PROTECTED]> > > ------------------------------------------------------------------------ > -- > To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]> > For additional commands, e-mail: <mailto:[EMAIL PROTECTED]> -- To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]> For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>