You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by js...@apache.org on 2002/08/07 04:13:23 UTC
cvs commit: jakarta-commons/httpclient/src/java/org/apache/commons/httpclient HttpConnection.java HttpMethodBase.java URIUtil.java
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: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>