commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jsde...@apache.org
Subject cvs commit: jakarta-commons/httpclient/src/java/org/apache/commons/httpclient HttpConnection.java HttpMethodBase.java URIUtil.java
Date Wed, 07 Aug 2002 02:13:23 GMT
jsdever     2002/08/06 19:13:22

  Modified:    httpclient/src/java/org/apache/commons/httpclient
                        HttpConnection.java HttpMethodBase.java
                        URIUtil.java
  Log:
  HttpMethodBase.execute refactoring.
  Reviewed by dIon over discussions on the mailing list.
  
  Revision  Changes    Path
  1.16      +24 -15    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.15
  retrieving revision 1.16
  diff -u -r1.15 -r1.16
  --- HttpConnection.java	6 Aug 2002 19:47:29 -0000	1.15
  +++ HttpConnection.java	7 Aug 2002 02:13:22 -0000	1.16
  @@ -190,10 +190,17 @@
       /**
        * Return my port.
        *
  +     * If the port is -1 (or less than 0) the default port for
  +     * the current protocol is returned.
  +     *
        * @return my port.
        */
       public int getPort() {
  -        return _port;
  +        if (_port < 0) {
  +            return isSecure() ? 443 : 80;
  +        } else {
  +            return _port;
  +        }
       }
   
       /**
  @@ -470,15 +477,16 @@
   
           assertOpen();
           if(wireLog.isDebugEnabled() && (data.length > 0)) {
  -            wireLog.debug(">> \"" + new String(data) + "\"");
  +            String data_str =  new String(data);
  +            wireLog.debug(">> \"" + data_str.trim() + "\" [\\r\\n]" );
           }
           try {
               _output.write(data);
           } catch(SocketException se){
  -	    log.debug("HttpConnection: Socket exception while writing data", 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);
  +            log.debug("HttpConnection: Exception while writing data", ioe);
               throw ioe;
           }
       }
  @@ -513,16 +521,17 @@
   
           assertOpen();
           if(wireLog.isDebugEnabled() && (data.length > 0)) {
  -            wireLog.debug(">> \"" + new String(data) + "\"");
  +	    String data_str =  new String(data);
  +            wireLog.debug(">> \"" + data_str.trim() + "\" [\\r\\n]" );
           }
           try{
               _output.write(data);
               writeLine();
           } catch(SocketException se){
  -	    log.info("SocketException while writing data to output", se);
  +            log.info("SocketException while writing data to output", se);
               throw new HttpRecoverableException(se.toString());
           } catch(IOException ioe){
  -	    log.info("IOException while writing data to output", ioe);
  +            log.info("IOException while writing data to output", ioe);
               throw ioe;
           }
       }
  @@ -539,14 +548,14 @@
       throws IOException, IllegalStateException, HttpRecoverableException {
           log.trace("enter HttpConnection.writeLine()");
   
  -        wireLog.debug(">> \\r\\n");
  +        wireLog.debug(">> [\\r\\n]");
           try{
               _output.write(CRLF);
           } catch(SocketException se){
  -	    log.warn("HttpConnection: Socket exception while writing data", se);
  +            log.warn("HttpConnection: Socket exception while writing data", se);
               throw new HttpRecoverableException(se.toString());
           } catch(IOException ioe){
  -	    log.warn("HttpConnection: IO exception while writing data", ioe);
  +            log.warn("HttpConnection: IO exception while writing data", ioe);
               throw ioe;
           }
       }
  @@ -648,7 +657,7 @@
               Object[] params = new Object[0];
               shutdownOutput.invoke(_socket, params);
           } catch (Exception ex) {
  -	    log.debug("Unexpected Exception caught", ex);
  +            log.debug("Unexpected Exception caught", ex);
               // Ignore, and hope everything goes right
           }
           // close output stream?
  
  
  
  1.44      +110 -74   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.43
  retrieving revision 1.44
  diff -u -r1.43 -r1.44
  --- HttpMethodBase.java	6 Aug 2002 19:47:29 -0000	1.43
  +++ HttpMethodBase.java	7 Aug 2002 02:13:22 -0000	1.44
  @@ -163,8 +163,9 @@
       /**
        * Get the HTTP version.
        * @return HTTP/1.1 if http11, HTTP/1.0 otherwise
  +     * @since 2.0
        */
  -    public String getHttpVersion() {
  +    private String getHttpVersion() {
           return (http11 ? "HTTP/1.1" : "HTTP/1.0");
       }
   
  @@ -246,6 +247,7 @@
        * one "field-name: field-value" pair, without changing the
        * semantics of the message, by appending each subsequent field-value
        * to the first, each separated by a comma."
  +     * //TODO: This method is trying to make up for deficiencies in Header.
        */
       private String getNewHeaderValue(Header existingHeader, String value) {
           String existingValue = existingHeader.getValue();
  @@ -465,8 +467,13 @@
       }
   
       /**
  -     * Write a request and read the response. Retry, if needed, up to 
  -     * {@link #maxRetries} times.
  +     * Write a request and read the response.
  +     *
  +     * Both the write to the server will be retried {@link #maxRetries}
  +     * times if the operation fails with a HttpRecoverableException.
  +     * The write will only be attempted if the read has succeeded.
  +     * <p>
  +     * The <i>used</i> is set to true if the write succeeds.
        *
        * @param state the current state
        * @param connection the connection for communication
  @@ -474,14 +481,17 @@
        *      conversation
        * @throws IOException when an I/O error occurs communicating with the 
        *      server
  +     * @see writeRequest(HttpState,HttpConnection)
  +     * @see readResponse(HttpState,HttpConnection)
        */
       private void processRequest(HttpState state, HttpConnection connection)
       throws HttpException, IOException {
           log.trace("enter HttpMethodBase.processRequest(HttpState, HttpConnection)");
   
  +        //try to do the write
           for (int retryCount = 1; retryCount <= maxRetries; retryCount++) {
               if (log.isTraceEnabled()){
  -                log.trace("Attempt number " + retryCount + " to write request and read
response");
  +                log.trace("Attempt number " + retryCount + " to write request");
               }
               try {
                   if (!connection.isOpen()) {
  @@ -489,19 +499,34 @@
                       connection.open();
                   }
                   writeRequest(state, connection);
  -                used = true;
  -                readResponse(state,connection);
  -                return;
               } catch (HttpRecoverableException httpre) {
  +                log.debug("Closing the connection.");
                   connection.close();
  -                log.debug("Recoverable exception caught when writing/reading request",
httpre);
  +                log.info("Recoverable exception caught when writing request");
  +                if (retryCount == maxRetries) {
  +                    log.warn("Attempt to write request has reached max retries: " +
  +                            maxRetries);
  +                    throw httpre;
  +                }
               }
           }
   
  -        //exceeded max number of retries
  -        throw new HttpException("Unable to process request: maximum number of retries ("
+ 
  -                maxRetries + ") exceeded.");
  +        //at least the write has succeeded
  +        used = true;
  + 
  +        //try to do the read
  +        try {
  +            readResponse(state,connection);
  +        } catch (HttpRecoverableException httpre) {
  +            log.warn("Recoverable exception caught when reading response");
  +            log.debug("Closing the connection.");
  +            connection.close();
  +            throw httpre;
  +        }
  +
  +        //everything should be OK at this point
       }
  +
       
       /**
        * On a {@link HttpStatus#SC_CONTINUE continue}, if there are more request
  @@ -522,8 +547,7 @@
               if (!bodySent) {
                   bodySent = writeRequestBody(state, connection);
               } else {
  -                log.warn("HttpMethodBase.execute(): received 100 response, "
  -                    + "but I've already sent the response. Ignoring.");
  +                log.warn("Received status CONTINUE but he body has already been sent");
                   // According to RFC 2616 this respose should be ignored
               }
               readResponse(state,connection);
  @@ -541,10 +565,12 @@
           log.trace("enter closeConnection(HttpConnection)");
   
           if (!http11) {
  -            if (getName().equals(ConnectMethod.NAME) && (statusCode == HttpStatus.SC_OK))
{
  +            if (getName().equals(ConnectMethod.NAME) && 
  +                    (statusCode == HttpStatus.SC_OK)) {
                   log.debug("Leaving connection open for tunneling");
               } else {
  -                log.debug("Closing connection since using HTTP/1.0, using ConnectMethod
and status is OK");
  +                log.debug("Closing connection since using HTTP/1.0, " + 
  +                        "ConnectMethod and status is OK");
                   connection.close();
               }
           } else {
  @@ -568,6 +594,7 @@
        */
       private boolean processAuthenticationResponse(HttpState state, 
       HttpConnection connection) {
  +        log.trace("enter  HttpMethodBase.processAuthenticationResponse(HttpState, HttpConnection)");
   
           Set realms = new HashSet();
           Set proxyRealms = new HashSet();
  @@ -614,11 +641,11 @@
                           break;
                   }
               } catch (HttpException httpe) {
  -                log.warn("Exception thrown authenticating", httpe);
  +                log.warn("Exception thrown authenticating: " + httpe.getMessage());
                   return true; // finished request
               } catch (UnsupportedOperationException uoe) {
  -                log.warn("Exception thrown authenticating", uoe);
  -                return true; // finished request
  +                log.warn("Exception thrown authenticating: " + uoe.getMessage());
  +                //FIXME: should this return true?
               }
   
               if (!authenticated) {
  @@ -637,7 +664,6 @@
           return !authenticated; // finished processing if we aren't authenticated
       }
       
  -    // --------------------------------------------------------- Action Methods
   
       /** 
        * Generates a key used for idenifying visited URLs.
  @@ -651,37 +677,42 @@
       /**
        * Execute this method.
        *
  +     * Note that we cannot currently support redirects that change 
  +     * the connection parameters (host, port, protocol) because we 
  +     * don't yet have a good way to get the new connection.  For 
  +     * the time being, we just return the redirect response code, 
  +     * and allow the user agent to resubmit if desired.
  +     *
        * @param state {@link HttpState} information to associate with this 
        *      request. Must be non-null.
        * @param connection the {@link HttpConnection} to write to/read from.
        *      Must be non-null.
  -
  -                     //
  -                    // Note that we cannot currently support
  -                    // redirects that change the HttpConnection
  -                    // parameters (host, port, protocol)
  -                    // because we don't yet have a good way to
  -                    // get the new connection.
  -                    //
  -                    // For the time being, we just return
  -                    // the 302 response, and allow the user
  -                    // agent to resubmit if desired.
  -                    //
  +     *
  +     * Note that we cannot currently support
  +     * redirects that change the HttpConnection
  +     * parameters (host, port, protocol)
  +     * because we don't yet have a good way to
  +     * get the new connection.
  +     *   
  +     * For the time being, we just return
  +     * the 302 response, and allow the user
  +     * agent to resubmit if desired.
        *
        * @throws IOException if an I/O error occurs
        * @throws HttpException  if an protocol exception occurs
        *
        * @return the integer status code if one was obtained, or <tt>-1</tt>
        */
  -    public int execute(HttpState state, HttpConnection connection) 
  +    public int execute(HttpState state, HttpConnection conn) 
       throws HttpException, IOException {
           log.trace("enter HttpMethodBase.execute(HttpState, HttpConnection)");
  +        //TODO: This method is too large 
   
           //check some error conditions
  -        if (null == state) { //is there a point to this?
  +        if (null == state) {
               throw new NullPointerException("HttpState parameter");
           }
  -        if (null == connection) { //is there a point to this?
  +        if (null == conn) {
               throw new NullPointerException("HttpConnection parameter");
           }
           if (hasBeenUsed()) {
  @@ -693,48 +724,43 @@
   
           //pre-emptively add the authorization header, if required.
           Authenticator.authenticate(this, state);
  -        if (connection.isProxied()) {
  +        if (conn.isProxied()) {
               Authenticator.authenticateProxy(this, state);
           }
   
           Set visited = new HashSet();
  -        boolean repeat = true;
  -        int retryCount = 0; //protect from an infinite loop
  +        int forwardCount = 0; //protect from an infinite loop
   
  -        while(retryCount++ < 100) {
  +        while(forwardCount++ < maxForwards) {
   
  -            log.debug("Execute loop try " + retryCount);
  -
  -            //check to see if we have visited this url before
  -            if (visited.contains(generateVisitedKey(connection)) ) {
  -                log.error("Link " + generateVisitedKey(connection) + "' revisited");
  -                return statusCode;
  -            }
  -            visited.add(generateVisitedKey(connection));
  +            log.debug("Execute loop try " + forwardCount);
   
               //write the request and read the response, will retry
  -            processRequest(state, connection);
  +            processRequest(state, conn);
               
               //if SC_CONTINUE write the request body
  -            writeRemainingRequestBody(state, connection);
  +            writeRemainingRequestBody(state, conn);
   
               //close connection if required
  -            closeConnection(connection);
  +            closeConnection(conn);
   
               switch(statusCode){
                   case HttpStatus.SC_UNAUTHORIZED:
                   case HttpStatus.SC_PROXY_AUTHENTICATION_REQUIRED:
                       // process authentication response
  -                    boolean authenticated = processAuthenticationResponse(state, connection);
  +                    boolean authenticated = processAuthenticationResponse(state, conn);
                       if (authenticated){
                           return statusCode;
                       }
                       break;
  +
                   case HttpStatus.SC_MOVED_TEMPORARILY:
                   case HttpStatus.SC_MOVED_PERMANENTLY:
                   case HttpStatus.SC_TEMPORARY_REDIRECT:
  +                    //TODO: This block should be factored into a new
  +                    //method called processRedirectResponse
                       if (!getFollowRedirects()){
  -                        log.warn("Redirect requested but followRedirects is disabled");
  +                        log.info("Redirect requested but followRedirects is disabled");
                           return statusCode;
                       }
                       
  @@ -765,9 +791,8 @@
                       } else { //not strict mode
                           //try to construct the new url based on the current url
                           try {
  -                            URL currentUrl = new URL(connection.getProtocol(), connection.getHost(),

  -                                    connection.getPort(), getPath());
  -                            //TODO: rename connection to conn
  +                            URL currentUrl = new URL(conn.getProtocol(), conn.getHost(),

  +                                    conn.getPort(), getPath());
                               url = new URL(currentUrl, location);
                           } catch (Exception ex) {
                               log.error("Redirected location '" + locationHeader.getValue()
+ "' is malformed");
  @@ -778,16 +803,16 @@
                       
                       //check for redirect to a different protocol, host or port
                       String error = null;
  -                    if (! connection.getProtocol().equalsIgnoreCase(url.getProtocol()))
{
  -                        error = "Redirect from protocol " + connection.getProtocol() +

  +                    if (! conn.getProtocol().equalsIgnoreCase(url.getProtocol())) {
  +                        error = "Redirect from protocol " + conn.getProtocol() + 
                                   " to " + url.getProtocol() + " is not supported";
                       }
  -                    if (! connection.getHost().equalsIgnoreCase(url.getHost())) {
  -                        error = "Redirect from host " + connection.getHost() + 
  +                    if (! conn.getHost().equalsIgnoreCase(url.getHost())) {
  +                        error = "Redirect from host " + conn.getHost() + 
                                   " to " + url.getHost() + " is not supported";
                       }
  -                    if (connection.getPort() != url.getPort()) {
  -                        error = "Redirect from host " + connection.getHost() + 
  +                    if (conn.getPort() != url.getPort()) {
  +                        error = "Redirect from host " + conn.getHost() + 
                                   " to " + url.getHost() + " is not supported";
                       }
                       if (error != null){
  @@ -808,6 +833,7 @@
                           log.debug("Changing query string from \"" + getQueryString() +
"\" to \"" + 
                               qs + "\" in response to " + statusCode + " response.");
                       }
  +                    break;
   
                   default:
                       // neither an unauthorized nor a redirect response
  @@ -815,6 +841,13 @@
   
               }//end of switch
   
  +            //check to see if we have visited this url before
  +            if (visited.contains(generateVisitedKey(conn)) ) {
  +                log.error("Link " + generateVisitedKey(conn) + "' revisited");
  +                return statusCode;
  +            }
  +            visited.add(generateVisitedKey(conn));
  +
           }//end of loop
   
           log.error("Narrowly avoided an infinite loop in execute");
  @@ -877,7 +910,7 @@
       throws IOException, HttpException {
           log.trace("enter HttpMethodBase.writeRequestLine(HttpState, HttpConnection)");
           String requestLine = HttpMethodBase.generateRequestLine(conn, getName(),
  -            getPath(),getQueryString(),(http11 ? "HTTP/1.1" : "HTTP/1.0"));
  +            getPath(),getQueryString(), getHttpVersion());
           conn.print(requestLine);
       }
   
  @@ -960,7 +993,7 @@
        */
       protected void addHostRequestHeader(HttpState state, HttpConnection conn) 
       throws IOException, HttpException {
  -        log.trace("enter HttpMethodBase.addUserAgentRequestHeaders(HttpState, HttpConnection)");
  +        log.trace("enter HttpMethodBase.addHostRequestHeader(HttpState, HttpConnection)");
           // Per 19.6.1.1 of RFC 2616, it is legal for HTTP/1.0 based 
           // applications to send the Host request-header.
           // TODO: Add the ability to disable the sending of this header for HTTP/1.0 requests.
  @@ -974,7 +1007,8 @@
           }
           
           if (isIpAddress(host)) {
  -            log.debug("Request to add Host header ignored: host is an ipaddress");
  +            log.debug("Adding empty Host request header: host is an ipaddress");
  +            setRequestHeader("Host", null);
               return;
           }
   
  @@ -992,7 +1026,7 @@
        */
       protected void addCookieRequestHeader(HttpState state, HttpConnection conn)
       throws IOException, HttpException {
  -        log.trace("enter HttpMethodBase.addCookieRequestHeaders(HttpState, HttpConnection)");
  +        log.trace("enter HttpMethodBase.addCookieRequestHeader(HttpState, HttpConnection)");
   
           Header cookieHeader = Cookie.createCookieHeader(conn.getHost(), 
               conn.getPort(), getPath(), conn.isSecure(), new Date(), 
  @@ -1240,6 +1274,12 @@
        * <p>
        * Subclasses may want to override this method to
        * to customize the processing.
  +     * <p>
  +     * "It must be possible to combine the multiple header fields into
  +     * one "field-name: field-value" pair, without changing the
  +     * semantics of the message, by appending each subsequent field-value
  +     * to the first, each separated by a comma."
  +     *   - HTTP/1.0 (4.3)
        *
        * @see #readResponse
        * @see #processResponseHeaders
  @@ -1249,12 +1289,6 @@
        */
       protected void readResponseHeaders(HttpState state, HttpConnection conn)
       throws IOException, HttpException {
  -        // "It must be possible to combine the multiple header fields into
  -        // one "field-name: field-value" pair, without changing the
  -        // semantics of the message, by appending each subsequent field-value
  -        // to the first, each separated by a comma."
  -        //   - HTTP/1.0 (4.3)
  -
           log.trace("enter HttpMethodBase.readResponseHeaders(HttpState,HttpConnection)");
   
           responseHeaders.clear();
  @@ -1666,6 +1700,8 @@
       private byte[] responseBody = null;
       /** The maximum number of attempts to attempt recovery from an HttpRecoverableException.
*/
       private int maxRetries = 3;
  +    /** Maximum number of redirects and authentications that will be followed. */
  +    private static int maxForwards = 100;
       /** True if we're in strict mode. */
       private boolean strictMode = true;
   
  
  
  
  1.11      +4 -5      jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/URIUtil.java
  
  Index: URIUtil.java
  ===================================================================
  RCS file: /home/cvs/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/URIUtil.java,v
  retrieving revision 1.10
  retrieving revision 1.11
  diff -u -r1.10 -r1.11
  --- URIUtil.java	30 Jul 2002 03:01:47 -0000	1.10
  +++ URIUtil.java	7 Aug 2002 02:13:22 -0000	1.11
  @@ -536,7 +536,6 @@
               return encode(str.getBytes(enc), safe, spaceAsPlus);
           } catch (UnsupportedEncodingException e) {
   	    log.warn("Unsupported encoding " + enc + " requested, using default");
  -            e.printStackTrace();
               return encode(str.getBytes(), safe, spaceAsPlus);
           }
       }
  
  
  

--
To unsubscribe, e-mail:   <mailto:commons-dev-unsubscribe@jakarta.apache.org>
For additional commands, e-mail: <mailto:commons-dev-help@jakarta.apache.org>


Mime
View raw message