Folks,
This patch has been prompted by the questions/concerns raised by Fabio
<linea at libero.it>. Let's face it, HttpConnection has got a bit messy
with all the numerous changes made recently. In particular the exception
handling especially with regards to handling recoverable exceptions and
timeouts got muddy. This patch attempts to address the problem
changelog:
- Socket input stream now wrapped with a wrapper class that re-throws
certain type of generic IO exceptions as HttpClient specific exceptions.
In particular java.io.InterruptedIOExceptions are re-thrown as
IOTimeoutException
- java.io.InterruptedIOExceptions in HttpConnection#open() re-thrown as
ConnectTimeoutException
- HttpConnection#write(byte[], int, int) used to throw
HttpRecoverableException in case of getting absolutely messed up
parameters. changed to throw IllegalArgumentException instead
- HttpConnection#write(byte[], int, int) does not do any exception
handling anymore, since this is clearly a prerogative of the
WrappedOutputStream
- 'used' flag in HttpConnection was set to true only upon the connection
release. Why? Was there any reason for this kind of behavior? That doest
not seem right to me. Changed to set 'used' flag upon first successful
write operation. Mike, please have a close look. Let me know if you
disagree with the change. It can well be that I missed something
important.
- close() and flush() in WrappedOutputStream no longer throw
HttpRecoverableExceptions. I believe this kind of exceptions are too
severe to be automatically recovered from
Let me know what you think
Oleg
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.78
diff -u -r1.78 HttpConnection.java
--- java/org/apache/commons/httpclient/HttpConnection.java 1 Nov 2003 21:17:25 -0000 1.78
+++ java/org/apache/commons/httpclient/HttpConnection.java 2 Nov 2003 15:27:05 -0000
@@ -1,5 +1,5 @@
/*
- * $Header: /home/cvspublic/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/HttpConnection.java,v 1.78 2003/11/01 21:17:25 olegk Exp $
+ * $Header: /home/cvs/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/HttpConnection.java,v 1.78 2003/11/01 21:17:25 olegk Exp $
* $Revision: 1.78 $
* $Date: 2003/11/01 21:17:25 $
*
@@ -495,7 +495,7 @@
socket.setSoTimeout(this.params.getSoTimeout());
}
}
- } catch (InterruptedIOException e) {
+ } catch (IOTimeoutException e) {
// aha - the connection is NOT stale - continue on!
} catch (IOException e) {
// oops - the connection is stale, the read or soTimeout failed.
@@ -659,12 +659,12 @@
public void open() throws IOException {
LOG.trace("enter HttpConnection.open()");
+ final String host = (proxyHostName == null) ? hostName : proxyHostName;
+ final int port = (proxyHostName == null) ? portNumber : proxyPortNumber;
assertNotOpen();
try {
if (null == socket) {
- final String host = (null == proxyHostName) ? hostName : proxyHostName;
- final int port = (null == proxyHostName) ? portNumber : proxyPortNumber;
usingSecureSocket = isSecure() && !isProxied();
@@ -718,35 +718,32 @@
if (rcvBufSize >= 0) {
socket.setReceiveBufferSize(rcvBufSize);
}
- inputStream = new PushbackInputStream(socket.getInputStream());
+ inputStream = new PushbackInputStream(
+ new WrappedInputStream(socket.getInputStream()));
outputStream = new BufferedOutputStream(
new WrappedOutputStream(socket.getOutputStream()),
socket.getSendBufferSize()
);
isOpen = true;
used = false;
+ } catch (InterruptedIOException e) {
+ closeSocketAndStreams();
+ throw new ConnectTimeoutException("Open connection interrupted", e);
} catch (IOException e) {
// Connection wasn't opened properly
// so close everything out
closeSocketAndStreams();
throw e;
} catch (TimeoutController.TimeoutException e) {
- if (LOG.isWarnEnabled()) {
- StringBuffer buffer = new StringBuffer();
- buffer.append("The host ");
- buffer.append(hostName);
- buffer.append(":");
- buffer.append(portNumber);
- buffer.append(" (or proxy ");
- buffer.append(proxyHostName);
- buffer.append(":");
- buffer.append(proxyPortNumber);
- buffer.append(") did not accept the connection within timeout of ");
- buffer.append(this.params.getConnectionTimeout());
- buffer.append(" milliseconds");
- LOG.warn(buffer.toString());
- }
- throw new ConnectionTimeoutException();
+ StringBuffer buffer = new StringBuffer();
+ buffer.append("The host ");
+ buffer.append(host);
+ buffer.append(":");
+ buffer.append(port);
+ buffer.append(" did not accept the connection within timeout of ");
+ buffer.append(this.params.getConnectionTimeout());
+ buffer.append(" milliseconds");
+ throw new ConnectTimeoutException(buffer.toString());
}
}
@@ -893,7 +890,7 @@
} else {
LOG.debug("Input data not available");
}
- } catch (InterruptedIOException e) {
+ } catch (IOTimeoutException e) {
if (LOG.isDebugEnabled()) {
LOG.debug("Input data not available after " + timeout + " ms");
}
@@ -915,13 +912,12 @@
* Writes the specified bytes to the output stream.
*
* @param data the data to be written
- * @throws HttpRecoverableException if a SocketException occurs
* @throws IllegalStateException if not connected
* @throws IOException if an I/O problem occurs
* @see #write(byte[],int,int)
*/
public void write(byte[] data)
- throws IOException, IllegalStateException, HttpRecoverableException {
+ throws IOException, IllegalStateException {
LOG.trace("enter HttpConnection.write(byte[])");
this.write(data, 0, data.length);
}
@@ -938,38 +934,24 @@
* @param data array containing the data to be written.
* @param offset the start offset in the data.
* @param length the number of bytes to write.
- * @throws HttpRecoverableException if a SocketException occurs
* @throws IllegalStateException if not connected
* @throws IOException if an I/O problem occurs
*/
public void write(byte[] data, int offset, int length)
- throws IOException, IllegalStateException, HttpRecoverableException {
+ throws IOException, IllegalStateException {
LOG.trace("enter HttpConnection.write(byte[], int, int)");
+ if (offset < 0) {
+ throw new IllegalArgumentException("Array offset may not be negative");
+ }
+ if (length < 0) {
+ throw new IllegalArgumentException("Array length may not be negative");
+ }
if (offset + length > data.length) {
- throw new HttpRecoverableException("Unable to write:"
- + " offset=" + offset + " length=" + length
- + " data.length=" + data.length);
- } else if (data.length <= 0) {
- throw new HttpRecoverableException(
- "Unable to write:" + " data.length=" + data.length);
+ throw new IllegalArgumentException("Given offset and length exceed the array length");
}
-
assertOpen();
-
- try {
- outputStream.write(data, offset, length);
- } catch (HttpRecoverableException hre) {
- throw hre;
- } catch (SocketException se) {
- LOG.debug(
- "HttpConnection: Socket exception while writing data",
- se);
- throw new HttpRecoverableException(se.toString());
- } catch (IOException ioe) {
- LOG.debug("HttpConnection: Exception while writing data", ioe);
- throw ioe;
- }
+ this.outputStream.write(data, offset, length);
}
/**
@@ -977,12 +959,11 @@
* output stream.
*
* @param data the bytes to be written
- * @throws HttpRecoverableException when socket exceptions occur writing data
* @throws IllegalStateException if the connection is not open
* @throws IOException if an I/O problem occurs
*/
public void writeLine(byte[] data)
- throws IOException, IllegalStateException, HttpRecoverableException {
+ throws IOException, IllegalStateException {
LOG.trace("enter HttpConnection.writeLine(byte[])");
write(data);
writeLine();
@@ -991,13 +972,11 @@
/**
* Writes <tt>"\r\n".getBytes()</tt> to the output stream.
*
- * @throws HttpRecoverableException when socket exceptions occur writing
- * data
* @throws IllegalStateException if the connection is not open
* @throws IOException if an I/O problem occurs
*/
public void writeLine()
- throws IOException, IllegalStateException, HttpRecoverableException {
+ throws IOException, IllegalStateException {
LOG.trace("enter HttpConnection.writeLine()");
write(CRLF);
}
@@ -1006,13 +985,11 @@
* Writes the specified String (as bytes) to the output stream.
*
* @param data the string to be written
- * @throws HttpRecoverableException when socket exceptions occur writing
- * data
* @throws IllegalStateException if the connection is not open
* @throws IOException if an I/O problem occurs
*/
public void print(String data)
- throws IOException, IllegalStateException, HttpRecoverableException {
+ throws IOException, IllegalStateException {
LOG.trace("enter HttpConnection.print(String)");
write(HttpConstants.getBytes(data));
}
@@ -1022,13 +999,11 @@
* <tt>"\r\n".getBytes()</tt> to the output stream.
*
* @param data the data to be written
- * @throws HttpRecoverableException when socket exceptions occur writing
- * data
* @throws IllegalStateException if the connection is not open
* @throws IOException if an I/O problem occurs
*/
public void printLine(String data)
- throws IOException, IllegalStateException, HttpRecoverableException {
+ throws IOException, IllegalStateException {
LOG.trace("enter HttpConnection.printLine(String)");
writeLine(HttpConstants.getBytes(data));
}
@@ -1036,13 +1011,11 @@
/**
* Writes <tt>"\r\n".getBytes()</tt> to the output stream.
*
- * @throws HttpRecoverableException when socket exceptions occur writing
- * data
* @throws IllegalStateException if the connection is not open
* @throws IOException if an I/O problem occurs
*/
public void printLine()
- throws IOException, IllegalStateException, HttpRecoverableException {
+ throws IOException, IllegalStateException {
LOG.trace("enter HttpConnection.printLine()");
writeLine();
}
@@ -1115,10 +1088,6 @@
*/
public void releaseConnection() {
LOG.trace("enter HttpConnection.releaseConnection()");
-
- // we are assuming that the connection will only be released once used
- used = true;
-
if (locked) {
LOG.debug("Connection is locked. Call to releaseConnection() ignored.");
} else if (httpConnectionManager != null) {
@@ -1248,17 +1217,6 @@
this.params.setSendBufferSize(sendBufferSize);
}
- // -- Timeout Exception
- /**
- * Signals that a timeout occured while opening the socket.
- */
- public class ConnectionTimeoutException extends IOException {
- /** Create an instance */
- public ConnectionTimeoutException() {
- }
- }
-
-
/**
* Helper class for wrapping socket based tasks.
*/
@@ -1301,8 +1259,8 @@
}
/**
- * A wrapper for output streams that closes the connection and converts to recoverable
- * exceptions as appropriable when an IOException occurs.
+ * A wrapper for output streams that closes the connection and converts
+ * to HttpClient specific exceptions as appropriable when an IOException occurs.
*/
private class WrappedOutputStream extends OutputStream {
@@ -1323,11 +1281,11 @@
*/
private IOException handleException(IOException ioe) {
// keep the original value of used, as it will be set to false by close().
- boolean tempUsed = used;
+ boolean isRecoverable = HttpConnection.this.used;
HttpConnection.this.close();
if (ioe instanceof InterruptedIOException) {
return new IOTimeoutException(ioe.getMessage());
- } else if (tempUsed) {
+ } else if (isRecoverable) {
LOG.debug(
"Output exception occurred on a used connection. Will treat as recoverable.",
ioe
@@ -1340,39 +1298,94 @@
public void write(int b) throws IOException {
try {
- out.write(b);
+ out.write(b);
+ HttpConnection.this.used = true;
} catch (IOException ioe) {
throw handleException(ioe);
}
}
public void flush() throws IOException {
+ out.flush();
+ }
+
+ public void close() throws IOException {
+ out.close();
+ }
+
+ public void write(byte[] b, int off, int len) throws IOException {
try {
- out.flush();
+ out.write(b, off, len);
+ HttpConnection.this.used = true;
} catch (IOException ioe) {
throw handleException(ioe);
}
}
- public void close() throws IOException {
+ public void write(byte[] b) throws IOException {
try {
- out.close();
+ out.write(b);
+ HttpConnection.this.used = true;
} catch (IOException ioe) {
throw handleException(ioe);
}
}
- public void write(byte[] b, int off, int len) throws IOException {
+ }
+
+ /**
+ * A wrapper for input streams that converts to HTTPClient
+ * specific exceptions as appropriable when an IOException occurs.
+ */
+ private class WrappedInputStream extends InputStream {
+
+ /** the actual inpuit stream */
+ private InputStream in;
+
+ /**
+ * @param in the input stream to wrap
+ */
+ public WrappedInputStream(InputStream in) {
+ this.in = in;
+ }
+
+ /**
+ * Conditionally converts exception to HttpClient specific
+ * exception.
+ * @param ioe the exception that occurred
+ * @return the exception to be thrown
+ */
+ private IOException handleException(IOException ioe) {
+ if (ioe instanceof InterruptedIOException) {
+ return new IOTimeoutException(ioe.getMessage());
+ } else {
+ return ioe;
+ }
+ }
+
+ public int read() throws IOException {
try {
- out.write(b, off, len);
+ return in.read();
} catch (IOException ioe) {
throw handleException(ioe);
}
}
+
+ public void close() throws IOException {
+ in.close();
+ }
- public void write(byte[] b) throws IOException {
+ public int read(byte[] b, int off, int len) throws IOException {
try {
- out.write(b);
+ return in.read(b, off, len);
+ } catch (IOException ioe) {
+ throw handleException(ioe);
+ }
+ }
+
+ public int read(byte[] b) throws IOException {
+ try {
+ return in.read(b);
} catch (IOException ioe) {
throw handleException(ioe);
}
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.187
diff -u -r1.187 HttpMethodBase.java
--- java/org/apache/commons/httpclient/HttpMethodBase.java 2 Nov 2003 12:10:28 -0000 1.187
+++ java/org/apache/commons/httpclient/HttpMethodBase.java 2 Nov 2003 15:27:18 -0000
@@ -1,5 +1,5 @@
/*
- * $Header: /home/cvspublic/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/HttpMethodBase.java,v 1.187 2003/11/02 12:10:28 olegk Exp $
+ * $Header: /home/cvs/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/HttpMethodBase.java,v 1.187 2003/11/02 12:10:28 olegk Exp $
* $Revision: 1.187 $
* $Date: 2003/11/02 12:10:28 $
*
@@ -67,7 +67,6 @@
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.InputStream;
-import java.io.InterruptedIOException;
import java.util.Iterator;
import org.apache.commons.httpclient.auth.AuthScheme;
@@ -1941,7 +1940,7 @@
} else {
return;
}
- } catch (InterruptedIOException e) {
+ } catch (IOTimeoutException e) {
// Most probably Expect header is not recongnized
// Remove the header to signal the method
// that it's okay to go ahead with sending data
Index: test/org/apache/commons/httpclient/TestHttpConnection.java
===================================================================
RCS file: /home/cvspublic/jakarta-commons/httpclient/src/test/org/apache/commons/httpclient/TestHttpConnection.java,v
retrieving revision 1.13
diff -u -r1.13 TestHttpConnection.java
--- test/org/apache/commons/httpclient/TestHttpConnection.java 26 Oct 2003 09:49:16 -0000 1.13
+++ test/org/apache/commons/httpclient/TestHttpConnection.java 2 Nov 2003 15:27:20 -0000
@@ -137,7 +137,7 @@
fail("Should have timed out");
} catch(IOException e) {
/* should fail */
- assertTrue(e instanceof HttpConnection.ConnectionTimeoutException);
+ assertTrue(e instanceof ConnectTimeoutException);
assertTrue(connectionManager.isConnectionReleased());
}
}
@@ -162,7 +162,7 @@
conn.open();
fail("Should have timed out");
} catch(IOException e) {
- assertTrue(e instanceof HttpConnection.ConnectionTimeoutException);
+ assertTrue(e instanceof ConnectTimeoutException);
/* should fail */
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
