Jeff, Mike, Odi
How about this for a starter?
Cheers
Oleg
PS: I'll be off-line for a few days (gonna be boarding in Davos)
On Thu, 2003-02-13 at 20:58, Jeffrey Dever wrote:
> >
> >
> >
> >Firstly, there's end of stream check missing, but it's just a minor bug
> >
> > ch = inputStream.read();
> > if (ch == -1) {
> > break;
> > }
> > if (ch == '\n') {
> > break;
> > } else {
> > buf.append('\r');
> > }
> >
> >
> Deja vu. Didn't I already fix that in HttpConnection, which now reads:
> int ch = inputStream.read();
> while (ch >= 0) {
> if (ch == '\r') {
> ch = inputStream.read();
> if (ch == '\n') {
> break;
> } else {
> buf.append('\r');
> }
> }
> buf.append((char) ch);
> ch = inputStream.read();
> }
>
> >Secondly, I am a bit unhappy with this line
> >
> > buf.append((char) ch);
> >
> You are right. We should be using a byte[] here as the buffer instead
> of a StringBuffer. Then call the correct string conversion routine as
> you stated. This is only buffering one line of headers here, so its
> good from a performance perspective.
>
> >Bottom line. Let's tackle problems step by step through a series of
> >small patches and if we see that wire logging cannot be made
> >comprehensive, so be it.
> >
> >Personally I am a strong believer in making things work 100% or not
> >making them at all. I'd rather remove wire logging all together, or
> >restrict it to exclusively dealing with request/response headers, rather
> >than having it occasionally spit out some bits and pieces.
> >
> >
> Like I said, if we split the wire logging responsibilities into headers
> and bodies, I think that we could be a lot more flexible. Lets face it,
> the information in a header is completely different than the data in a
> body, including the content encoding. HttpClient already handles these
> sperately by reading the headers completely, and then passing the body
> to the client as an output stream.
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [EMAIL PROTECTED]
> For additional commands, e-mail: [EMAIL PROTECTED]
>
Index: java/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.14
diff -u -r1.14 ChunkedInputStream.java
--- java/org/apache/commons/httpclient/ChunkedInputStream.java 13 Feb 2003 21:31:53 -0000 1.14
+++ java/org/apache/commons/httpclient/ChunkedInputStream.java 13 Feb 2003 22:40:17 -0000
@@ -67,7 +67,6 @@
import java.io.IOException;
import java.io.InputStream;
-import org.apache.commons.httpclient.util.HeaderParser;
/**
* <p>Transparently coalesces chunks of a HTTP stream that uses
@@ -340,7 +339,7 @@
* @throws IOException If an IO problem occurs
*/
private void parseTrailerHeaders() throws IOException {
- Header[] footers = HeaderParser.parseHeaders(in);
+ Header[] footers = HttpParser.parseHeaders(in);
for (int i = 0; i < footers.length; i++) {
method.addResponseFooter(footers[i]);
Index: java/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.44
diff -u -r1.44 HttpConnection.java
--- java/org/apache/commons/httpclient/HttpConnection.java 13 Feb 2003 21:31:53 -0000 1.44
+++ java/org/apache/commons/httpclient/HttpConnection.java 13 Feb 2003 22:40:14 -0000
@@ -110,42 +110,6 @@
*/
public class HttpConnection {
- /**
- * Read up to <tt>"\r\n"</tt> from an (unchunked) input stream.
- * If the stream ends before the line terminator is found,
- * the last part of the string will still be returned.
- * '\r' and '\n' are allowed to appear individually in the stream.
- *
- * @param inputStream the stream to read from
- *
- * @throws IOException if an I/O problem occurs
- * @return a line from the stream
- *
- * @since 2.0beta1
- */
- public static String readLine(InputStream inputStream) throws IOException {
- LOG.trace("enter HttpConnection.readLine()");
-
- StringBuffer buf = new StringBuffer();
- int ch = inputStream.read();
- while (ch >= 0) {
- if (ch == '\r') {
- ch = inputStream.read();
- if (ch == '\n') {
- break;
- } else {
- buf.append('\r');
- }
- }
- buf.append((char) ch);
- ch = inputStream.read();
- }
- if (WIRE_LOG.isDebugEnabled()) {
- WIRE_LOG.debug("<< \"" + buf.toString() + (ch>0 ? "\" [\\r\\n]" : ""));
- }
- return (buf.toString());
- }
-
// ----------------------------------------------------------- Constructors
/**
@@ -911,7 +875,7 @@
LOG.trace("enter HttpConnection.readLine()");
assertOpen();
- return HttpConnection.readLine(inputStream);
+ return HttpParser.readLine(inputStream);
}
/**
Index: java/org/apache/commons/httpclient/HttpMethodBase.java
===================================================================
RCS file: /home/cvspublic/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/HttpMethodBase.java,v
retrieving revision 1.111
diff -u -r1.111 HttpMethodBase.java
--- java/org/apache/commons/httpclient/HttpMethodBase.java 11 Feb 2003 03:23:05 -0000 1.111
+++ java/org/apache/commons/httpclient/HttpMethodBase.java 13 Feb 2003 22:40:30 -0000
@@ -75,7 +75,6 @@
import org.apache.commons.httpclient.cookie.CookiePolicy;
import org.apache.commons.httpclient.cookie.CookieSpec;
-import org.apache.commons.httpclient.util.HeaderParser;
import org.apache.commons.httpclient.util.URIUtil;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
@@ -1862,7 +1861,7 @@
+ "HttpConnection)");
getResponseHeaderGroup().clear();
- Header[] headers = HeaderParser.parseHeaders(conn.getResponseInputStream());
+ Header[] headers = HttpParser.parseHeaders(conn.getResponseInputStream());
getResponseHeaderGroup().setHeaders(headers);
}
Index: java/org/apache/commons/httpclient/HttpParser.java
===================================================================
RCS file: java/org/apache/commons/httpclient/HttpParser.java
diff -N java/org/apache/commons/httpclient/HttpParser.java
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ java/org/apache/commons/httpclient/HttpParser.java 13 Feb 2003 22:40:15 -0000
@@ -0,0 +1,147 @@
+package org.apache.commons.httpclient;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.ByteArrayOutputStream;
+import java.util.ArrayList;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+
+/**
+ * A utility class for parsing http header values.
+ *
+ * @author Michael Becke
+ * @author <a href="mailto:[EMAIL PROTECTED]";>Oleg Kalnichevski</a>
+ *
+ * @since 2.0beta1
+ */
+public class HttpParser {
+
+ /** Log object for this class. */
+ private static final Log LOG = LogFactory.getLog(HttpParser.class);
+
+ /** Log for any wire messages. */
+ private static final Log WIRE_LOG = LogFactory.getLog("httpclient.wire");
+ /**
+ * Constructor for HttpParser.
+ */
+ private HttpParser() {}
+
+ /**
+ * Return byte array from an (unchunked) input stream.
+ * Stop reading when <tt>"\r\n"</tt> terminator encountered
+ * If the stream ends before the line terminator is found,
+ * the last part of the string will still be returned.
+ * '\r' and '\n' are allowed to appear individually in the stream.
+ *
+ * @param inputStream the stream to read from
+ *
+ * @throws IOException if an I/O problem occurs
+ * @return a byte array from the stream
+ */
+ public static byte[] readRawLine(InputStream inputStream) throws IOException {
+ LOG.trace("enter HttpConnection.readRawLine()");
+
+ ByteArrayOutputStream buf = new ByteArrayOutputStream();
+ int ch;
+ while ((ch = inputStream.read()) >= 0) {
+ buf.write(ch);
+ if (ch == '\r') {
+ ch = inputStream.read();
+ if (ch < 0) {
+ break;
+ }
+ buf.write(ch);
+ if (ch == '\n') {
+ break;
+ }
+ }
+ }
+ if (WIRE_LOG.isDebugEnabled()) {
+ WIRE_LOG.debug("<< \"" + buf.toString() + (ch>0 ? "\" [\\r\\n]" : ""));
+ }
+ return buf.toByteArray();
+ }
+
+ /**
+ * Read up to <tt>"\r\n"</tt> from an (unchunked) input stream.
+ * If the stream ends before the line terminator is found,
+ * the last part of the string will still be returned.
+ * '\r' and '\n' are allowed to appear individually in the stream.
+ *
+ * @param inputStream the stream to read from
+ *
+ * @throws IOException if an I/O problem occurs
+ * @return a line from the stream
+ */
+
+ public static String readLine(InputStream inputStream) throws IOException {
+ LOG.trace("enter HttpConnection.readLine()");
+ byte[] rawdata = readRawLine(inputStream);
+ if ((rawdata.length >= 2) && (rawdata[0] == '\r') && (rawdata[1] == '\n')) {
+ return HttpConstants.getString(rawdata, 0, rawdata.length - 2);
+ } else {
+ return HttpConstants.getString(rawdata);
+ }
+ }
+
+ /**
+ * Parses headers from the given stream. Headers with the same name are not
+ * combined.
+ *
+ * @param is the stream to read headers from
+ *
+ * @return an array of headers in the order in which they were parsed
+ *
+ * @throws IOException if an IO error occurs while reading from the stream
+ * @throws HttpException if there is an error parsing a header value
+ */
+ public static Header[] parseHeaders(InputStream is) throws IOException, HttpException {
+ LOG.trace("enter HeaderParser.parseHeaders(HttpConnection, HeaderGroup)");
+
+ ArrayList headers = new ArrayList();
+ String name = null;
+ StringBuffer value = null;
+ for (; ;) {
+ String line = HttpParser.readLine(is);
+ if ((line == null) || (line.length() < 1)) {
+ break;
+ }
+
+ // Parse the header name and value
+ // Check for folded headers first
+ // Detect LWS-char see HTTP/1.0 or HTTP/1.1 Section 2.2
+ // discussion on folded headers
+ if ((line.charAt(0) == ' ') || (line.charAt(0) == '\t')) {
+ // we have continuation folded header
+ // so append value
+ value.append(' ');
+ value.append(line.trim());
+ } else {
+ // make sure we save the previous name,value pair if present
+ if (name != null) {
+ headers.add(new Header(name, value.toString()));
+ }
+
+ // Otherwise we should have normal HTTP header line
+ // Parse the header name and value
+ int colon = line.indexOf(":");
+ if (colon < 0) {
+ throw new HttpException("Unable to parse header: " + line);
+ }
+ name = line.substring(0, colon).trim();
+ value = new StringBuffer(line.substring(colon + 1).trim());
+ }
+
+ }
+
+ // make sure we save the last name,value pair if present
+ if (name != null) {
+ headers.add(new Header(name, value.toString()));
+ }
+
+ return (Header[]) headers.toArray(new Header[headers.size()]);
+ }
+
+}
Index: java/org/apache/commons/httpclient/util/HeaderParser.java
===================================================================
RCS file: java/org/apache/commons/httpclient/util/HeaderParser.java
diff -N java/org/apache/commons/httpclient/util/HeaderParser.java
--- java/org/apache/commons/httpclient/util/HeaderParser.java 11 Feb 2003 05:04:10 -0000 1.1
+++ /dev/null 1 Jan 1970 00:00:00 -0000
@@ -1,88 +0,0 @@
-package org.apache.commons.httpclient.util;
-
-import java.io.IOException;
-import java.io.InputStream;
-import java.util.ArrayList;
-
-import org.apache.commons.httpclient.Header;
-import org.apache.commons.httpclient.HttpConnection;
-import org.apache.commons.httpclient.HttpException;
-import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
-
-/**
- * A utility class for parsing http header values.
- *
- * @author Michael Becke
- *
- * @since 2.0beta1
- */
-public class HeaderParser {
-
- /** Log object for this class. */
- private static final Log LOG = LogFactory.getLog(HeaderParser.class);
-
- /**
- * Constructor for HeaderParser.
- */
- private HeaderParser() {}
-
- /**
- * Parses headers from the given stream. Headers with the same name are not
- * combined.
- *
- * @param is the stream to read headers from
- *
- * @return an array of headers in the order in which they were parsed
- *
- * @throws IOException if an IO error occurs while reading from the stream
- * @throws HttpException if there is an error parsing a header value
- */
- public static Header[] parseHeaders(InputStream is) throws IOException, HttpException {
- LOG.trace("enter HeaderParser.parseHeaders(HttpConnection, HeaderGroup)");
-
- ArrayList headers = new ArrayList();
- String name = null;
- StringBuffer value = null;
- for (; ;) {
- String line = HttpConnection.readLine(is);
- if ((line == null) || (line.length() < 1)) {
- break;
- }
-
- // Parse the header name and value
- // Check for folded headers first
- // Detect LWS-char see HTTP/1.0 or HTTP/1.1 Section 2.2
- // discussion on folded headers
- if ((line.charAt(0) == ' ') || (line.charAt(0) == '\t')) {
- // we have continuation folded header
- // so append value
- value.append(' ');
- value.append(line.trim());
- } else {
- // make sure we save the previous name,value pair if present
- if (name != null) {
- headers.add(new Header(name, value.toString()));
- }
-
- // Otherwise we should have normal HTTP header line
- // Parse the header name and value
- int colon = line.indexOf(":");
- if (colon < 0) {
- throw new HttpException("Unable to parse header: " + line);
- }
- name = line.substring(0, colon).trim();
- value = new StringBuffer(line.substring(colon + 1).trim());
- }
-
- }
-
- // make sure we save the last name,value pair if present
- if (name != null) {
- headers.add(new Header(name, value.toString()));
- }
-
- return (Header[]) headers.toArray(new Header[headers.size()]);
- }
-
-}
Index: test/org/apache/commons/httpclient/SimpleHttpConnection.java
===================================================================
RCS file: /home/cvspublic/jakarta-commons/httpclient/src/test/org/apache/commons/httpclient/SimpleHttpConnection.java,v
retrieving revision 1.9
diff -u -r1.9 SimpleHttpConnection.java
--- test/org/apache/commons/httpclient/SimpleHttpConnection.java 11 Feb 2003 03:23:05 -0000 1.9
+++ test/org/apache/commons/httpclient/SimpleHttpConnection.java 13 Feb 2003 22:40:08 -0000
@@ -186,7 +186,7 @@
public String readLine()
throws IOException, IllegalStateException {
- String str = HttpConnection.readLine(inputStream);
+ String str = HttpParser.readLine(inputStream);
log.debug("read: " + str);
return str;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
