[prev in list] [next in list] [prev in thread] [next in thread]
List: jakarta-commons-dev
Subject: [HttpClient] Bug 5018 PATCH
From: "Paul C. Bryan" <pbryan () creatia ! com>
Date: 2001-11-28 21:43:47
[Download RAW message or body]
Summary:
Chunked transfer encoding not isolated from application.
Description:
Chunked transfer encoding is not being supported transparently by the
HttpMethodBase object, causing chunk data to be embedded in response
body data and forcing the application to handle the HTTP/1.1
implementation of chunked transfer encoding.
The included patch now properly parses chunk data as per HTTP/1.1 and
provides body content consistently, regardless of whether chunked
transfer encoding was used by the server or not. This relieves the
application from the requirement of implementing HTTP/1.1.
Patch Notes:
HTTP/1.1 specifies optional footer fields that are supplied after the
end of the chunked data body. These are parsed, but added as "header"
fields because I didn't want to change the HttpMethod interface. The
result is that footer fields such as signatures must be retrieved as
headers. The downside to this approach is there is no way to distinguish
between a header field and a footer field when interrogating the
HttpMethod.
I've defined a constant SIZE_buf to use as a buffer size for allocated
copy buffers. I've only used it in my patch (because it is referred to
more than once in the method), but propose that it be used by any
methods that perform copy operations with the allocation of a buffer.
Comments?
Because a ResponseInputStream is used to read the body, the response
footers cannot be retrived using the conn.readLine() method. Instead,
the ResponseInputStream.readLine method must be used. For this reason,
response footers are read after the body in the readResponseBody method.
I'm not sure why only get request method is supporting writing content
to disk. I believe the useDisk functionality should be moved to
HttpMethodBase. I was tempted to do so, but didn't want to jeopardize
the acceptance of this patch. If someone gives me the go ahead, I am
willing to move useDisk into HttpMethodBase.
GetMethod.readResponseBody(HttpState, HttpConnection) duplicated much of
HttpMethodBase.readResponseBody(HttpState, HttpConnection)
functionality. I moved this into a new common
readResponseBody(ResponseInputStream, OutputStream) that is shared by
HttpMethodBase and GetMethod. If useDisk moves into HttpMethodBase,
there should be no longer be a need for this separate method.
The included patches have been extensively tested against a WebLogic 6.1
SP1 server, which makes use of chunked transfer encoding when content
length is not known ahead of time.
I've gone out of my way to keep the style of this patch consistent with
the existing source code. I'm curious how much liberty I can take in the
future when it comes to formatting.
Yours truly,
Paul C. Bryan
pbryan@creatia.com
["chunk-patches.txt" (text/plain)]
--- HttpMethodBase.java.orig Wed Nov 21 13:54:38 2001
+++ HttpMethodBase.java Wed Nov 21 14:37:06 2001
@@ -924,6 +924,37 @@
}
/**
+ * Parses the response header, populating the response headers
+ * map.
+ *
+ * @see #readResponseHeaders
+ * @see #readResponseBody
+ *
+ * @param line the line of text containing the header name and value
+ */
+ protected void parseHeader(String line) throws HttpException {
+ int colon = line.indexOf(":");
+ if (colon < 0) {
+ throw new HttpException("Unable to parse header: " + line);
+ }
+ String name = line.substring(0, colon).trim();
+ String match = name.toLowerCase();
+ String value = line.substring(colon + 1).trim();
+ Header header = (Header)(responseHeaders.get(match));
+ if(null == header) {
+ header = new Header(name, value);
+ } else {
+ String oldvalue = header.getValue();
+ if(null != oldvalue) {
+ header = new Header(name,oldvalue + ", " + value);
+ } else {
+ header = new Header(name,value);
+ }
+ }
+ responseHeaders.put(match, header);
+ }
+
+ /**
* Read response headers from the given {@link HttpConnection},
* populating the response headers map.
* <p>
@@ -953,25 +984,7 @@
}
// Parse the header name and value
- int colon = line.indexOf(":");
- if (colon < 0) {
- throw new HttpException("Unable to parse header: " + line);
- }
- String name = line.substring(0, colon).trim();
- String match = name.toLowerCase();
- String value = line.substring(colon + 1).trim();
- Header header = (Header)(responseHeaders.get(match));
- if(null == header) {
- header = new Header(name, value);
- } else {
- String oldvalue = header.getValue();
- if(null != oldvalue) {
- header = new Header(name,oldvalue + ", " + value);
- } else {
- header = new Header(name,value);
- }
- }
- responseHeaders.put(match, header);
+ parseHeader(line);
}
}
@@ -1015,6 +1028,106 @@
}
}
+ /** Size of copy buffer for reading response body. */
+ private static final int SIZE_buf = 4096;
+
+ /**
+ * Reads the input stream, one line at a time. Reads until it reaches
+ * the end of file or a newline character. Carriage returns and new
+ * lines are not returned in the result.
+ *
+ * @param in input stream on which the bytes are read
+ * @return the line that was read
+ * @exception IOException if an input or output exception has occurred
+ */
+ private String readLineFromStream(InputStream in) throws IOException {
+ StringBuffer sb = new StringBuffer();
+ while (true) {
+ int c = in.read();
+ if (c < 0) {
+ break;
+ } else if (c == '\r') {
+ continue;
+ } else if (c == '\n') {
+ break;
+ }
+ sb.append((char)c);
+ }
+ return (sb.toString());
+ }
+
+ /**
+ * Reads the response body from the given input stream and writes
+ * to the given output stream.
+ *
+ * @see #readResponseBody
+ *
+ * @param is input stream to read body from
+ * @param out the ouput stream to write body to
+ */
+ protected void readResponseBody(InputStream is, OutputStream out) throws \
IOException, HttpException { + int expectedLength = -1;
+ Header header = getResponseHeader("Transfer-Encoding");
+ boolean chunked = (header != null && \
header.getValue().equalsIgnoreCase("chunked")); + if (!chunked) {
+ Header lengthHeader = getResponseHeader("Content-Length");
+ if(null != lengthHeader) {
+ try {
+ expectedLength = Integer.parseInt(lengthHeader.getValue());
+ } catch (NumberFormatException e) {
+ // ignored
+ }
+ }
+ }
+ byte[] buffer = new byte[SIZE_buf];
+ int nb = 0;
+ chunks: for (;;) {
+ if (chunked) {
+ String chunkData = readLineFromStream(is);
+ try {
+ expectedLength = Integer.parseInt(chunkData, 16);
+ if (expectedLength == 0) {
+ break;
+ }
+ }
+ catch (NumberFormatException nfe) {
+ throw new HttpException("Unable to parse chunk data: " + \
chunkData); + }
+ }
+ int foundLength = 0;
+ while (expectedLength < 0 || foundLength < expectedLength) {
+ int len = (expectedLength < 0 ? SIZE_buf : Math.min(expectedLength - \
foundLength, SIZE_buf)); + nb = is.read(buffer, 0, len);
+ if(nb == -1) {
+ break chunks;
+ }
+ if(out == null) {
+ throw new IOException("Unable to buffer data");
+ }
+ if(wireLog.isInfoEnabled()) {
+ wireLog.info("<< \"" + new String(buffer,0,nb) + "\"");
+ }
+ out.write(buffer, 0, nb);
+ foundLength += nb;
+ }
+ if (!chunked) {
+ break;
+ }
+ if (readLineFromStream(is).length() != 0) {
+ throw new HttpException("Expected blank line at end of chunk");
+ }
+ }
+
+ if (chunked) {
+ for(;;) {
+ String line = readLineFromStream(is);
+ if((line == null) || (line.length() < 1)) {
+ break;
+ }
+ parseHeader(line);
+ }
+ }
+ }
/**
* Read the response body from the given {@link HttpConnection}.
@@ -1037,48 +1150,8 @@
log.debug("HttpMethodBase.readResponseBody(HttpState,HttpConnection)");
responseBody = null;
ByteArrayOutputStream out = new ByteArrayOutputStream();
- int expectedLength = 0;
- int foundLength = 0;
- {
- Header lengthHeader = getResponseHeader("Content-Length");
- Header transferEncodingHeader = getResponseHeader("Transfer-Encoding");
- if(null != lengthHeader) {
- try {
- expectedLength = Integer.parseInt(lengthHeader.getValue());
- } catch(NumberFormatException e) {
- // ignored
- }
- } else if(null != transferEncodingHeader) {
- if("chunked".equalsIgnoreCase(transferEncodingHeader.getValue())) {
- expectedLength = -1;
- }
- }
- }
InputStream is = conn.getResponseInputStream(this);
- byte[] buffer = new byte[4096];
- int nb = 0;
- while(expectedLength == -1 || foundLength < expectedLength) {
- nb = is.read(buffer);
- if(nb == -1) {
- break;
- }
- if(out == null) {
- throw new IOException("Unable to buffer data");
- }
- if(wireLog.isInfoEnabled()) {
- wireLog.info("<< \"" + new String(buffer,0,nb) + "\"");
- }
- out.write(buffer, 0, nb);
- foundLength += nb;
- if(expectedLength > -1) {
- if(foundLength == expectedLength) {
- break;
- } else if(foundLength > expectedLength) {
- log.warn("HttpMethodBase.readResponseBody(): expected length (" \
+ expectedLength + ") exceeded. Found " + foundLength + " bytes.");
- break;
- }
- }
- }
+ readResponseBody(is, out);
out.close();
responseBody = out.toByteArray();
}
--- methods/GetMethod.java.orig Wed Nov 21 11:10:33 2001
+++ methods/GetMethod.java Wed Nov 21 14:37:44 2001
@@ -73,6 +73,7 @@
import org.apache.commons.httpclient.ResponseInputStream;
import org.apache.commons.httpclient.log.Log;
import org.apache.commons.httpclient.log.LogSource;
+import org.apache.commons.httpclient.HttpException;
/**
* GET Method.
@@ -351,7 +352,7 @@
* Overrides method in {@link HttpMethodBase} to
* write data to the appropriate buffer.
*/
- protected void readResponseBody(HttpState state, HttpConnection conn) throws \
IOException { + protected void readResponseBody(HttpState state, HttpConnection \
conn) throws IOException, HttpException {
log.debug("GetMethod.readResponseBody(HttpState,HttpConnection)");
OutputStream out = null;
if (useDisk) {
@@ -380,43 +381,8 @@
} else {
out = new ByteArrayOutputStream();
}
-
- int expectedLength = -1;
- int foundLength = 0;
- {
- Header lengthHeader = getResponseHeader("Content-Length");
- if(null != lengthHeader) {
- try {
- expectedLength = Integer.parseInt(lengthHeader.getValue());
- } catch(NumberFormatException e) {
- // ignored
- }
- }
- }
InputStream is = conn.getResponseInputStream(this);
- byte[] buffer = new byte[4096];
- int nb = 0;
- while (true) {
- nb = is.read(buffer);
- if (nb == -1)
- break;
- if (out == null)
- throw new IOException("Unable to buffer data");
- if(wireLog.isInfoEnabled()) {
- wireLog.info("<< \"" + new String(buffer,0,nb) + "\"");
- }
- out.write(buffer, 0, nb);
- foundLength += nb;
- if(expectedLength > -1) {
- if(foundLength == expectedLength) {
- break;
- } else if(foundLength > expectedLength) {
- log.warn("GetMethod.readResponseBody(): expected length (" + \
expectedLength + ") exceeded. Found " + foundLength + " bytes.");
- break;
- }
- }
- }
-
+ readResponseBody(is, out);
if (!useDisk) {
memoryData = ((ByteArrayOutputStream) out).toByteArray();
}
--
To unsubscribe, e-mail: <mailto:commons-dev-unsubscribe@jakarta.apache.org>
For additional commands, e-mail: <mailto:commons-dev-help@jakarta.apache.org>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic