I have been trying to resolve the bug related to 'expect - continue'
handshake. We have been making some progress but I still have no clear
idea what may be causing this problem.
However, while working on this bug I have come to believe that 'expect:
100-continue' stuff in HttpClient is flaky and is in need of
improvement. The 'expect: 100-continue' related code is spread around a
few classes, is difficult to understand and to debug, and most
importantly error prone.
Here is my attempt at fixing the situation.
You feedback will be appreciated, as always, even if you end up tearing
it apart, again. ;-)
Cheers
Oleg
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.117
diff -u -r1.117 HttpMethodBase.java
--- java/org/apache/commons/httpclient/HttpMethodBase.java 25 Feb 2003 23:20:23 -0000 1.117
+++ java/org/apache/commons/httpclient/HttpMethodBase.java 26 Feb 2003 21:13:33 -0000
@@ -195,9 +195,6 @@
/** Buffer for the response */
private byte[] responseBody = null;
- /** Whether or not the request body has been sent. */
- private boolean bodySent = false;
-
/** Whether or not I should automatically follow redirects. */
private boolean followRedirects = false;
@@ -956,12 +953,12 @@
LOG.debug("Execute loop try " + forwardCount);
}
+ // Discard status line
+ this.statusLine = null;
+
//write the request and read the response, will retry
processRequest(state, conn);
- //if SC_CONTINUE write the request body
- writeRemainingRequestBody(state, conn);
-
if (!isRetryNeeded(statusLine.getStatusCode(), state, conn)) {
// nope, no retry needed, exit loop.
break;
@@ -1166,7 +1163,6 @@
statusLine = null;
used = false;
http11 = true;
- bodySent = false;
responseBody = null;
recoverableExceptionCount = 0;
inExecute = false;
@@ -1918,6 +1914,10 @@
throws IOException, HttpRecoverableException, HttpException {
LOG.trace("enter HttpMethodBase.readStatusLine(HttpState, HttpConnection)");
+ if (this.statusLine != null) {
+ // Status line already present
+ return;
+ }
//read out the HTTP status string
String statusString = conn.readLine();
while ((statusString != null) && !statusString.startsWith("HTTP/")) {
@@ -1994,7 +1994,39 @@
writeRequestLine(state, conn);
writeRequestHeaders(state, conn);
conn.writeLine(); // close head
- bodySent = writeRequestBody(state, conn);
+
+ Header expectheader = getRequestHeader("Expect");
+ String expectvalue = null;
+ if (expectheader != null) {
+ expectvalue = expectheader.getValue();
+ }
+ if ((expectvalue != null)
+ && (expectvalue.compareToIgnoreCase("100-continue") == 0)) {
+ if (this.isHttp11()) {
+ if (conn.waitForResponse(RESPONSE_WAIT_TIME_MS)) {
+ LOG.debug("Response available");
+ readStatusLine(state, conn);
+ if (this.statusLine.getStatusCode() == HttpStatus.SC_CONTINUE) {
+ // Discard status line
+ this.statusLine = null;
+ LOG.debug("OK to continue received");
+ } else {
+ return;
+ }
+ } else {
+ // Most probably Expect header is not recongnized
+ // Remove the header to signal the method
+ // that it's okay to go ahead with sending data
+ removeRequestHeader("Expect");
+ LOG.debug("Response not available. Send the request body");
+ }
+ } else {
+ removeRequestHeader("Expect");
+ LOG.info("'Expect: 100-continue' handshake is only supported by HTTP/1.1 or higher");
+ }
+ }
+
+ writeRequestBody(state, conn);
}
/**
@@ -2109,43 +2141,6 @@
}
/**
- * Determines if the provided value is a valid IPv4 internet address.
- *
- * @param value - value to check
- *
- * @return boolean - true if value is valid, otherwise false
- */
- private static boolean isIpAddress(String value) {
- LOG.trace("enter HttpMethodBase.isIpAddress(String)");
-
- value = value.trim();
-
- // prevent input values of 127.0.0.1. or .127.0.0.1, etc.
- if (value.startsWith(".") || value.endsWith(".")) {
- return false;
- }
-
- StringTokenizer tokenizer = new StringTokenizer(value, ".");
- if (tokenizer.countTokens() == 4) {
- while (tokenizer.hasMoreTokens()) {
- try {
- int i = Integer.parseInt(tokenizer.nextToken());
- if ((i < 0) || (i > 255)) {
- // parsed section of address is not in the proper range
- return false;
- }
- } catch (NumberFormatException nfe) {
- return false;
- }
- }
- } else {
- // wrong number of tokens
- return false;
- }
- return true;
- }
-
- /**
* Per RFC 2616 section 4.3, some response can never contain a message
* body.
*
@@ -2305,83 +2300,20 @@
}
} while (retryCount <= maxRetries);
-
- // Should we expect a response at this point?
- boolean responseExpected = true;
- if (!this.bodySent) {
- if (connection.waitForResponse(RESPONSE_WAIT_TIME_MS)) {
- responseExpected = true;
- LOG.debug("Response available");
- } else {
- responseExpected = false;
- // Something is wrong.
- if (getRequestHeader("Expect") != null) {
- // Most probably Expect header is not recongnized
- // Remove the header to signal the method
- // that it's okay to go ahead with sending data
- removeRequestHeader("Expect");
- }
- LOG.debug("Response not available. Send the request body");
- }
- }
- if (responseExpected) {
- //try to do the read
- try {
- readResponse(state, connection);
- } catch (HttpRecoverableException httpre) {
- LOG.warn("Recoverable exception caught when reading response");
- if (LOG.isDebugEnabled()) {
- LOG.debug("Closing the connection.");
- }
-
- connection.close();
- throw httpre;
- }
- }
- //everything should be OK at this point
- }
-
- /**
- * On a [EMAIL PROTECTED] HttpStatus#SC_CONTINUE continue}, if there are more request
- * bytes to be sent, write them to the connection
- *
- * @param state the current state
- * @param connection the connection for communication
- *
- * @throws HttpException when errors occur as part of the HTTP protocol
- * conversation
- * @throws IOException when an I/O error occurs communicating with the
- * server
- */
- private void writeRemainingRequestBody(HttpState state,
- HttpConnection connection)
- throws HttpException, IOException {
- LOG.trace("enter writeRemainingRequestBody(HttpState, HttpConnection)");
-
- boolean writeRemaining = false;
- boolean continueReceived = false;
- if (statusLine == null) {
- writeRemaining = true;
- } else {
- if (statusLine.getStatusCode() == HttpStatus.SC_CONTINUE) {
- writeRemaining = true;
- continueReceived = true;
+ //try to do the read
+ try {
+ readResponse(state, connection);
+ } catch (HttpRecoverableException httpre) {
+ LOG.warn("Recoverable exception caught when reading response");
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Closing the connection.");
}
- }
- if (writeRemaining) {
- if (!bodySent) {
- bodySent = writeRequestBody(state, connection);
- } else {
- if (continueReceived) {
- LOG.warn("Received status CONTINUE but the body has already been sent");
- // According to RFC 2616 this respose should be ignored
- }
- }
- readResponse(state, connection);
+ connection.close();
+ throw httpre;
}
+ //everything should be OK at this point
}
-
/**
* Return the character set from the header.
Index: java/org/apache/commons/httpclient/methods/EntityEnclosingMethod.java
===================================================================
RCS file: /home/cvspublic/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/methods/EntityEnclosingMethod.java,v
retrieving revision 1.10
diff -u -r1.10 EntityEnclosingMethod.java
--- java/org/apache/commons/httpclient/methods/EntityEnclosingMethod.java 25 Feb 2003 23:33:48 -0000 1.10
+++ java/org/apache/commons/httpclient/methods/EntityEnclosingMethod.java 26 Feb 2003 21:13:34 -0000
@@ -392,17 +392,6 @@
LOG.trace(
"enter EntityEnclosingMethod.writeRequestBody(HttpState, HttpConnection)");
- if (getRequestHeader("Expect") != null) {
- if (getStatusLine() == null) {
- LOG.debug("Expecting response");
- return false;
- }
- if (getStatusLine().getStatusCode() != HttpStatus.SC_CONTINUE) {
- LOG.debug("Expecting 100-continue");
- return false;
- }
- }
-
int contentLength = getRequestContentLength();
if ((contentLength == CONTENT_LENGTH_CHUNKED) && !isHttp11()) {
Index: java/org/apache/commons/httpclient/methods/MultipartPostMethod.java
===================================================================
RCS file: /home/cvspublic/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/methods/MultipartPostMethod.java,v
retrieving revision 1.11
diff -u -r1.11 MultipartPostMethod.java
--- java/org/apache/commons/httpclient/methods/MultipartPostMethod.java 25 Feb 2003 23:33:48 -0000 1.11
+++ java/org/apache/commons/httpclient/methods/MultipartPostMethod.java 26 Feb 2003 21:13:34 -0000
@@ -276,16 +276,6 @@
protected boolean writeRequestBody(HttpState state, HttpConnection conn)
throws IOException, HttpException {
LOG.trace("enter MultipartPostMethod.writeRequestBody(HttpState state, HttpConnection conn)");
- if (getRequestHeader("Expect") != null) {
- if (getStatusLine() == null) {
- LOG.debug("Expecting response");
- return false;
- }
- if (getStatusLine().getStatusCode() != HttpStatus.SC_CONTINUE) {
- LOG.debug("Expecting 100-continue");
- return false;
- }
- }
OutputStream out = conn.getRequestOutputStream();
Part.sendParts(out, getParts());
return true;
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]