You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by ol...@apache.org on 2003/11/05 21:45:34 UTC

cvs commit: jakarta-commons/httpclient/src/test/org/apache/commons/httpclient TestHttpConnection.java

olegk       2003/11/05 12:45:34

  Modified:    httpclient/src/java/org/apache/commons/httpclient
                        HttpConnection.java HttpMethodBase.java
               httpclient/src/test/org/apache/commons/httpclient
                        TestHttpConnection.java
  Log:
  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) changed to throw IllegalArgumentException in case of invalid input. Used to throw HttpRecoverableException.
  - 'Used' connection logic improved.
  
  Contributed by Oleg Kalnichevski
  Reviewed by Michael Becke
  
  Revision  Changes    Path
  1.79      +105 -84   jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/HttpConnection.java
  
  Index: HttpConnection.java
  ===================================================================
  RCS file: /home/cvs/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/HttpConnection.java,v
  retrieving revision 1.78
  retrieving revision 1.79
  diff -u -r1.78 -r1.79
  --- HttpConnection.java	1 Nov 2003 21:17:25 -0000	1.78
  +++ HttpConnection.java	5 Nov 2003 20:45:34 -0000	1.79
  @@ -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,7 +1298,8 @@
   
           public void write(int b) throws IOException {
               try {
  -                out.write(b);            
  +                out.write(b);
  +                HttpConnection.this.used = true;            
               } catch (IOException ioe) {
                   throw handleException(ioe);
               }
  @@ -1365,6 +1324,7 @@
           public void write(byte[] b, int off, int len) throws IOException {
               try {
                   out.write(b, off, len);
  +                HttpConnection.this.used = true;            
               } catch (IOException ioe) {
                   throw handleException(ioe);
               }
  @@ -1373,6 +1333,67 @@
           public void write(byte[] b) throws IOException {
               try {
                   out.write(b);            
  +                HttpConnection.this.used = true;            
  +            } catch (IOException ioe) {
  +                throw handleException(ioe);
  +            }
  +        }
  +
  +    }
  +
  +    /**
  +     * 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 {
  +                return in.read();
  +            } catch (IOException ioe) {
  +                throw handleException(ioe);
  +            }
  +        }
  +        
  +        public void close() throws IOException {
  +            in.close();            
  +        }
  +
  +        public int read(byte[] b, int off, int len) throws IOException {
  +            try {
  +                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);
               }
  
  
  
  1.189     +5 -6      jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/HttpMethodBase.java
  
  Index: HttpMethodBase.java
  ===================================================================
  RCS file: /home/cvs/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/HttpMethodBase.java,v
  retrieving revision 1.188
  retrieving revision 1.189
  diff -u -r1.188 -r1.189
  --- HttpMethodBase.java	3 Nov 2003 23:03:02 -0000	1.188
  +++ HttpMethodBase.java	5 Nov 2003 20:45:34 -0000	1.189
  @@ -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;
  @@ -1949,7 +1948,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
  
  
  
  1.14      +2 -2      jakarta-commons/httpclient/src/test/org/apache/commons/httpclient/TestHttpConnection.java
  
  Index: TestHttpConnection.java
  ===================================================================
  RCS file: /home/cvs/jakarta-commons/httpclient/src/test/org/apache/commons/httpclient/TestHttpConnection.java,v
  retrieving revision 1.13
  retrieving revision 1.14
  diff -u -r1.13 -r1.14
  --- TestHttpConnection.java	26 Oct 2003 09:49:16 -0000	1.13
  +++ TestHttpConnection.java	5 Nov 2003 20:45:34 -0000	1.14
  @@ -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: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org