This is an automated email from the ASF dual-hosted git repository.
hboutemy pushed a commit to branch WAGON-541
in repository https://gitbox.apache.org/repos/asf/maven-wagon.git
commit e6c8e915f4da48df10a67fd965990e5d7b7cf03f
Author: Peter Lynch <ply...@sonatype.com>
AuthorDate: Thu Oct 3 08:56:30 2019 -0300
WAGON-541 add consistency to exception messages for all wagons so that
clients ( ie. Maven ) reporting the Exceptions to users have good consistent
contextual information. This means also report any custom status line reason
phrases the servers involved might return.
---
.../apache/maven/wagon/http/HttpWagonTestCase.java | 133 ++++++++++--
.../wagon/providers/http/LightweightHttpWagon.java | 181 +++++++++++++----
.../providers/http/LightweightHttpWagonTest.java | 106 ++++++++++
.../wagon/shared/http/AbstractHttpClientWagon.java | 122 ++++-------
.../maven/wagon/shared/http/HttpMessageUtils.java | 222 +++++++++++++++++++++
.../maven/wagon/providers/http/HttpWagon.java | 22 +-
.../providers/http/ErrorWithMessageServlet.java | 4 +
.../wagon/providers/http/HttpWagonErrorTest.java | 69 ++++++-
.../wagon/providers/webdav/WebDavWagonTest.java | 7 +
.../apache/maven/wagon/tck/http/Assertions.java | 137 ++++++++++++-
.../apache/maven/wagon/tck/http/GetWagonTests.java | 35 +---
11 files changed, 858 insertions(+), 180 deletions(-)
diff --git
a/wagon-provider-test/src/main/java/org/apache/maven/wagon/http/HttpWagonTestCase.java
b/wagon-provider-test/src/main/java/org/apache/maven/wagon/http/HttpWagonTestCase.java
index 463431a..a90f8f4 100644
---
a/wagon-provider-test/src/main/java/org/apache/maven/wagon/http/HttpWagonTestCase.java
+++
b/wagon-provider-test/src/main/java/org/apache/maven/wagon/http/HttpWagonTestCase.java
@@ -25,6 +25,7 @@ import org.apache.maven.wagon.StreamingWagon;
import org.apache.maven.wagon.StreamingWagonTestCase;
import org.apache.maven.wagon.TransferFailedException;
import org.apache.maven.wagon.Wagon;
+import org.apache.maven.wagon.WagonException;
import org.apache.maven.wagon.authentication.AuthenticationInfo;
import org.apache.maven.wagon.authorization.AuthorizationException;
import org.apache.maven.wagon.proxy.ProxyInfo;
@@ -34,6 +35,7 @@ import org.apache.maven.wagon.resource.Resource;
import org.codehaus.plexus.util.FileUtils;
import org.codehaus.plexus.util.IOUtil;
import org.codehaus.plexus.util.StringUtils;
+import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.security.ConstraintMapping;
import org.eclipse.jetty.security.ConstraintSecurityHandler;
import org.eclipse.jetty.security.HashLoginService;
@@ -401,20 +403,25 @@ public abstract class HttpWagonTestCase
{
StreamingWagon wagon = (StreamingWagon) getWagon();
- Server server = new Server( );
- StatusHandler handler = new StatusHandler();
- handler.setStatusToReturn( status );
- server.setHandler( handler );
- addConnector( server );
+ Server server = createStatusServer( status );
server.start();
- wagon.connect( new Repository( "id", getRepositoryUrl( server ) ) );
+ String baseUrl = getRepositoryUrl( server );
+ String resourceName = "resource";
+ String serverReasonPhrase = HttpStatus.getCode( status ).getMessage();
+
+ wagon.connect( new Repository( "id", baseUrl ) );
try
{
wagon.getToStream( "resource", new ByteArrayOutputStream() );
fail();
}
+ catch ( Exception e )
+ {
+ verifyWagonExceptionMessage( e, status, baseUrl + "/" +
resourceName, serverReasonPhrase );
+ throw e;
+ }
finally
{
wagon.disconnect();
@@ -523,11 +530,7 @@ public abstract class HttpWagonTestCase
{
StreamingWagon wagon = (StreamingWagon) getWagon();
- Server server = new Server( );
- StatusHandler handler = new StatusHandler();
- handler.setStatusToReturn( status );
- server.setHandler( handler );
- addConnector( server );
+ Server server = createStatusServer( status );
server.start();
wagon.connect( new Repository( "id", getRepositoryUrl( server ) ) );
@@ -1593,6 +1596,16 @@ public abstract class HttpWagonTestCase
return server;
}
+ private Server createStatusServer( int status )
+ {
+ Server server = new Server( );
+ StatusHandler handler = new StatusHandler();
+ handler.setStatusToReturn( status );
+ server.setHandler( handler );
+ addConnector( server );
+ return server;
+ }
+
private String writeTestFile( File parent, String child, String
compressionType )
throws IOException
@@ -1756,11 +1769,7 @@ public abstract class HttpWagonTestCase
{
StreamingWagon wagon = (StreamingWagon) getWagon();
- Server server = new Server( );
- StatusHandler handler = new StatusHandler();
- handler.setStatusToReturn( status );
- server.setHandler( handler );
- addConnector( server );
+ Server server = createStatusServer( status );
server.start();
wagon.connect( new Repository( "id", getRepositoryUrl( server ) ) );
@@ -1769,11 +1778,20 @@ public abstract class HttpWagonTestCase
tempFile.deleteOnExit();
FileUtils.fileWrite( tempFile.getAbsolutePath(), "content" );
+ String baseUrl = getRepositoryUrl( server );
+ String resourceName = "resource";
+ String serverReasonPhrase = HttpStatus.getCode( status ).getMessage();
+
try
{
- wagon.put( tempFile, "resource" );
+ wagon.put( tempFile, resourceName );
fail();
}
+ catch ( Exception e )
+ {
+ verifyWagonExceptionMessage( e, status, baseUrl + "/" +
resourceName, serverReasonPhrase );
+ throw e;
+ }
finally
{
wagon.disconnect();
@@ -2281,4 +2299,85 @@ public abstract class HttpWagonTestCase
return sb.toString();
}
}
+
+ /**
+ * Verify a WagonException message contains required format and context
based on the status code we expected to
+ * trigger it in the first place.
+ * <p>
+ * This implementation represents the most desired assertions, but
HttpWagonTestCase sub-classes could override
+ * this method if a specific wagon representation makes it impossible to
meet these assertions.
+ *
+ * @param e an instance of {@link WagonException}
+ * @param forStatusCode the response status code that triggered the
exception
+ * @param forUrl the url that triggered the exception
+ * @param forReasonPhrase the optional status line reason phrase the
server returned
+ */
+ protected void verifyWagonExceptionMessage( Exception e, int
forStatusCode, String forUrl, String forReasonPhrase )
+ {
+ // TODO: handle AuthenticationException for Wagon.connect() calls
+ assertNotNull( e );
+ try
+ {
+ assertTrue( "only verify instances of WagonException", e
instanceof WagonException );
+
+ String reasonPhrase;
+ String assertMessageForBadMessage = "exception message not
described properly";
+ switch ( forStatusCode )
+ {
+ case HttpServletResponse.SC_NOT_FOUND:
+ // TODO: add test for 410: Gone?
+ assertTrue( "404 not found response should throw
ResourceDoesNotExistException",
+ e instanceof ResourceDoesNotExistException );
+ reasonPhrase = StringUtils.isEmpty( forReasonPhrase ) ? "
Not Found" : ( " " + forReasonPhrase );
+ assertEquals( assertMessageForBadMessage, "Resource
missing at " + forUrl + " 404"
+ + reasonPhrase, e.getMessage() );
+ break;
+
+ case HttpServletResponse.SC_UNAUTHORIZED:
+ // FIXME assumes Wagon.get()/put() returning 401 instead
of Wagon.connect()
+ assertTrue( "401 Unauthorized should throw
AuthorizationException since "
+ + " AuthenticationException is not
explicitly declared as thrown from wagon "
+ + "methods",
+ e instanceof AuthorizationException );
+ reasonPhrase = StringUtils.isEmpty( forReasonPhrase ) ? "
Unauthorized" : ( " " + forReasonPhrase );
+ assertEquals( assertMessageForBadMessage, "Authentication
failed for " + forUrl + " 401"
+ + reasonPhrase, e.getMessage() );
+ break;
+
+ case HttpServletResponse.SC_PROXY_AUTHENTICATION_REQUIRED:
+ assertTrue( "407 Proxy authentication required should
throw AuthorizationException",
+ e instanceof AuthorizationException );
+ reasonPhrase = StringUtils.isEmpty( forReasonPhrase ) ? "
Proxy Authentication Required"
+ : ( " " + forReasonPhrase );
+ assertEquals( assertMessageForBadMessage, "HTTP proxy
server authentication failed for "
+ + forUrl + " 407" + reasonPhrase, e.getMessage() );
+ break;
+
+ case HttpServletResponse.SC_FORBIDDEN:
+ assertTrue( "403 Forbidden should throw
AuthorizationException",
+ e instanceof AuthorizationException );
+ reasonPhrase = StringUtils.isEmpty( forReasonPhrase ) ? "
Forbidden" : ( " " + forReasonPhrase );
+ assertEquals( assertMessageForBadMessage, "Authorization
failed for " + forUrl + " 403"
+ + reasonPhrase, e.getMessage() );
+ break;
+
+ default:
+ assertTrue( "transfer failures should at least be wrapped
in a TransferFailedException", e
+ instanceof TransferFailedException );
+ assertTrue( "expected status code for transfer failures
should be >= 400",
+ forStatusCode >=
HttpServletResponse.SC_BAD_REQUEST );
+ reasonPhrase = forReasonPhrase == null ? "" : " " +
forReasonPhrase;
+ assertEquals( assertMessageForBadMessage, "Transfer failed
for " + forUrl + " "
+ + forStatusCode + reasonPhrase, e.getMessage() );
+ break;
+ }
+ }
+ catch ( AssertionError assertionError )
+ {
+ logger.error( "Exception which failed assertions: ", e );
+ throw assertionError;
+ }
+
+ }
+
}
diff --git
a/wagon-providers/wagon-http-lightweight/src/main/java/org/apache/maven/wagon/providers/http/LightweightHttpWagon.java
b/wagon-providers/wagon-http-lightweight/src/main/java/org/apache/maven/wagon/providers/http/LightweightHttpWagon.java
index 50fc28a..d0f32eb 100644
---
a/wagon-providers/wagon-http-lightweight/src/main/java/org/apache/maven/wagon/providers/http/LightweightHttpWagon.java
+++
b/wagon-providers/wagon-http-lightweight/src/main/java/org/apache/maven/wagon/providers/http/LightweightHttpWagon.java
@@ -50,9 +50,17 @@ import java.net.URL;
import java.util.ArrayList;
import java.util.List;
import java.util.Properties;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
import java.util.zip.DeflaterInputStream;
import java.util.zip.GZIPInputStream;
+import static java.lang.Integer.parseInt;
+import static
org.apache.maven.wagon.shared.http.HttpMessageUtils.UNKNOWN_STATUS_CODE;
+import static
org.apache.maven.wagon.shared.http.HttpMessageUtils.formatAuthorizationMessage;
+import static
org.apache.maven.wagon.shared.http.HttpMessageUtils.formatResourceDoesNotExistMessage;
+import static
org.apache.maven.wagon.shared.http.HttpMessageUtils.formatTransferFailedMessage;
+
/**
* LightweightHttpWagon, using JDK's HttpURLConnection.
*
@@ -69,6 +77,9 @@ public class LightweightHttpWagon
private Proxy proxy = Proxy.NO_PROXY;
+ private static final Pattern IOEXCEPTION_MESSAGE_PATTERN =
Pattern.compile( "Server returned HTTP response code: "
+ + "(\\d\\d\\d) for URL: (.*)" );
+
public static final int MAX_REDIRECTS = 10;
/**
@@ -105,20 +116,46 @@ public class LightweightHttpWagon
Resource resource = inputData.getResource();
String visitingUrl = buildUrl( resource );
- try
+
+ List<String> visitedUrls = new ArrayList<>();
+
+ for ( int redirectCount = 0; redirectCount < MAX_REDIRECTS;
redirectCount++ )
{
- List<String> visitedUrls = new ArrayList<String>();
+ if ( visitedUrls.contains( visitingUrl ) )
+ {
+ // TODO add a test for this message
+ throw new TransferFailedException( "Cyclic http redirect
detected. Aborting! " + visitingUrl );
+ }
+ visitedUrls.add( visitingUrl );
- for ( int redirectCount = 0; redirectCount < MAX_REDIRECTS;
redirectCount++ )
+ URL url = null;
+ try
{
- if ( visitedUrls.contains( visitingUrl ) )
- {
- throw new TransferFailedException( "Cyclic http redirect
detected. Aborting! " + visitingUrl );
- }
- visitedUrls.add( visitingUrl );
+ url = new URL( visitingUrl );
+ }
+ catch ( MalformedURLException e )
+ {
+ // TODO add test for this
+ throw new ResourceDoesNotExistException( "Invalid repository
URL: " + e.getMessage(), e );
+ }
+
+ HttpURLConnection urlConnection = null;
+
+ try
+ {
+ urlConnection = ( HttpURLConnection ) url.openConnection(
this.proxy );
+ }
+ catch ( IOException e )
+ {
+ // TODO: add test for this
+ String message = formatTransferFailedMessage( visitingUrl,
UNKNOWN_STATUS_CODE,
+ null, getProxyInfo() );
+ // TODO include e.getMessage appended to main message?
+ throw new TransferFailedException( message, e );
+ }
- URL url = new URL( visitingUrl );
- HttpURLConnection urlConnection = (HttpURLConnection)
url.openConnection( this.proxy );
+ try
+ {
urlConnection.setRequestProperty( "Accept-Encoding",
"gzip,deflate" );
if ( !useCache )
@@ -130,13 +167,16 @@ public class LightweightHttpWagon
// TODO: handle all response codes
int responseCode = urlConnection.getResponseCode();
+ String reasonPhrase = urlConnection.getResponseMessage();
+
if ( responseCode == HttpURLConnection.HTTP_FORBIDDEN
- || responseCode == HttpURLConnection.HTTP_UNAUTHORIZED )
+ || responseCode == HttpURLConnection.HTTP_UNAUTHORIZED
)
{
- throw new AuthorizationException( "Access denied to: " +
buildUrl( resource ) );
+ throw new AuthorizationException(
formatAuthorizationMessage( buildUrl( resource ),
+ responseCode, reasonPhrase, getProxyInfo() ) );
}
if ( responseCode == HttpURLConnection.HTTP_MOVED_PERM
- || responseCode == HttpURLConnection.HTTP_MOVED_TEMP )
+ || responseCode == HttpURLConnection.HTTP_MOVED_TEMP )
{
visitingUrl = urlConnection.getHeaderField( "Location" );
continue;
@@ -158,27 +198,22 @@ public class LightweightHttpWagon
resource.setLastModified( urlConnection.getLastModified() );
resource.setContentLength( urlConnection.getContentLength() );
break;
+
}
- }
- catch ( MalformedURLException e )
- {
- throw new ResourceDoesNotExistException( "Invalid repository URL:
" + e.getMessage(), e );
- }
- catch ( FileNotFoundException e )
- {
- throw new ResourceDoesNotExistException( "Unable to locate
resource in repository", e );
- }
- catch ( IOException e )
- {
- StringBuilder message = new StringBuilder( "Error transferring
file: " );
- message.append( e.getMessage() );
- message.append( " from " + visitingUrl );
- if ( getProxyInfo() != null && getProxyInfo().getHost() != null )
+ catch ( FileNotFoundException e )
+ {
+ // this could be 404 Not Found or 410 Gone - we don't have
access to which it was.
+ // TODO: 2019-10-03 url used should list all
visited/redirected urls, not just the original
+ throw new ResourceDoesNotExistException(
formatResourceDoesNotExistMessage( buildUrl( resource ),
+ UNKNOWN_STATUS_CODE, null, getProxyInfo() ), e );
+ }
+ catch ( IOException originalIOException )
{
- message.append( " with proxyInfo " ).append(
getProxyInfo().toString() );
+ throw convertHttpUrlConnectionException( originalIOException,
urlConnection, buildUrl( resource ) );
}
- throw new TransferFailedException( message.toString(), e );
+
}
+
}
private void addHeaders( HttpURLConnection urlConnection )
@@ -229,6 +264,7 @@ public class LightweightHttpWagon
{
try
{
+ String reasonPhrase = putConnection.getResponseMessage();
int statusCode = putConnection.getResponseCode();
switch ( statusCode )
@@ -240,23 +276,25 @@ public class LightweightHttpWagon
case HttpURLConnection.HTTP_NO_CONTENT: // 204
break;
+ // TODO: handle 401 explicitly?
case HttpURLConnection.HTTP_FORBIDDEN:
- throw new AuthorizationException( "Access denied to: " +
buildUrl( resource ) );
+ throw new AuthorizationException(
formatAuthorizationMessage( buildUrl( resource ), statusCode,
+ reasonPhrase, getProxyInfo() ) );
case HttpURLConnection.HTTP_NOT_FOUND:
- throw new ResourceDoesNotExistException( "File: " +
buildUrl( resource ) + " does not exist" );
+ throw new ResourceDoesNotExistException(
formatResourceDoesNotExistMessage( buildUrl( resource ),
+ statusCode, reasonPhrase, getProxyInfo() ) );
- // add more entries here
+ // add more entries here
default:
- throw new TransferFailedException(
- "Failed to transfer file: " + buildUrl( resource ) +
". Return code is: " + statusCode );
+ throw new TransferFailedException(
formatTransferFailedMessage( buildUrl( resource ),
+ statusCode, reasonPhrase, getProxyInfo() ) ) ;
}
}
catch ( IOException e )
{
fireTransferError( resource, e, TransferEvent.REQUEST_PUT );
-
- throw new TransferFailedException( "Error transferring file: " +
e.getMessage(), e );
+ throw convertHttpUrlConnectionException( e, putConnection,
buildUrl( resource ) );
}
}
@@ -474,4 +512,73 @@ public class LightweightHttpWagon
{
this.authenticator = authenticator;
}
+
+ /**
+ * Convert the IOException that is thrown for most transfer errors that
HttpURLConnection encounters to the
+ * equivalent {@link TransferFailedException}.
+ * <p>
+ * Details are extracted from the error stream if possible, either
directly or indirectly by way of supporting
+ * accessors. The returned exception will include the passed IOException
as a cause and a message that is as
+ * descriptive as possible.
+ *
+ * @param originalIOException an IOException thrown from an
HttpURLConnection operation
+ * @param urlConnection instance that triggered the IOException
+ * @param url originating url that triggered the
IOException
+ * @return exception that is representative of the original cause
+ */
+ private TransferFailedException convertHttpUrlConnectionException(
IOException originalIOException,
+
HttpURLConnection urlConnection,
+ String
url )
+ {
+ // javadoc of HttpUrlConnection, HTTP transfer errors throw IOException
+ // In that case, one may attempt to get the status code and reason
phrase
+ // from the errorstream. We do this, but by way of the following code
path
+ // getResponseCode()/getResponseMessage() - calls -> getHeaderFields()
+ // getHeaderFields() - calls -> getErrorStream()
+ try
+ {
+ // call getResponseMessage first since impl calls getResponseCode
as part of that anyways
+ String errorResponseMessage = urlConnection.getResponseMessage();
// may be null
+ int errorResponseCode = urlConnection.getResponseCode(); // may be
-1 if the code cannot be discerned
+ String message = formatTransferFailedMessage( url,
errorResponseCode, errorResponseMessage,
+ getProxyInfo() );
+ return new TransferFailedException( message, originalIOException );
+
+ }
+ catch ( IOException errorStreamException )
+ {
+ // there was a problem using the standard methods, need to fall
back to other options
+ }
+
+ // Attempt to parse the status code and URL which can be included in
an IOException message
+ //
https://github.com/AdoptOpenJDK/openjdk-jdk11/blame/999dbd4192d0f819cb5224f26e9e7fa75ca6f289/src/java
+ //
.base/share/classes/sun/net/www/protocol/http/HttpURLConnection.java#L1911L1913
+ String ioMsg = originalIOException.getMessage();
+ if ( ioMsg != null )
+ {
+ Matcher matcher = IOEXCEPTION_MESSAGE_PATTERN.matcher( ioMsg );
+ if ( matcher.matches() )
+ {
+ String codeStr = matcher.group( 1 );
+ String urlStr = matcher.group( 2 );
+
+ int code = UNKNOWN_STATUS_CODE;
+ try
+ {
+ code = parseInt( codeStr );
+ }
+ catch ( NumberFormatException nfe )
+ {
+ // if here there is a regex problem
+ }
+
+ String message = formatTransferFailedMessage( urlStr, code,
null, getProxyInfo() );
+ return new TransferFailedException( message,
originalIOException );
+ }
+ }
+
+ String message = formatTransferFailedMessage( url,
UNKNOWN_STATUS_CODE, null, getProxyInfo() );
+ return new TransferFailedException( message, originalIOException );
+ }
+
}
diff --git
a/wagon-providers/wagon-http-lightweight/src/test/java/org/apache/maven/wagon/providers/http/LightweightHttpWagonTest.java
b/wagon-providers/wagon-http-lightweight/src/test/java/org/apache/maven/wagon/providers/http/LightweightHttpWagonTest.java
index 06929c4..1a7de5e 100644
---
a/wagon-providers/wagon-http-lightweight/src/test/java/org/apache/maven/wagon/providers/http/LightweightHttpWagonTest.java
+++
b/wagon-providers/wagon-http-lightweight/src/test/java/org/apache/maven/wagon/providers/http/LightweightHttpWagonTest.java
@@ -19,9 +19,17 @@ package org.apache.maven.wagon.providers.http;
* under the License.
*/
+import org.apache.maven.wagon.ResourceDoesNotExistException;
import org.apache.maven.wagon.StreamingWagon;
+import org.apache.maven.wagon.TransferFailedException;
+import org.apache.maven.wagon.WagonException;
+import org.apache.maven.wagon.authorization.AuthorizationException;
import org.apache.maven.wagon.http.HttpWagonTestCase;
+import org.codehaus.plexus.util.StringUtils;
+import javax.servlet.http.HttpServletResponse;
+import java.io.FileNotFoundException;
+import java.io.IOException;
import java.util.Properties;
/**
@@ -63,4 +71,102 @@ public class LightweightHttpWagonTest
{
return false;
}
+
+ @Override
+ protected void verifyWagonExceptionMessage(Exception e, int forStatusCode,
String forUrl, String forReasonPhrase)
+ {
+
+ // HttpUrlConnection prevents direct API access to the response code
or reasonPhrase for any
+ // status code >= 400. So all we can do is check WagonException wraps
the HttpUrlConnection
+ // thrown IOException / FileNotFoundException as a cause, if cause is
not null
+
+ assertNotNull( e );
+ try
+ {
+ assertTrue( "only verify instances of WagonException", e
instanceof WagonException );
+
+ String assertMessageForBadMessage = "exception message not
described properly: ";
+ switch ( forStatusCode )
+ {
+ case HttpServletResponse.SC_GONE:
+ case HttpServletResponse.SC_NOT_FOUND:
+ assertTrue( "404 or 410 should throw
ResourceDoesNotExistException",
+ e instanceof ResourceDoesNotExistException );
+
+ if ( e.getCause() != null )
+ {
+ assertTrue( "ResourceDoesNotExistException should have
the expected cause",
+ e.getCause() instanceof FileNotFoundException
);
+ // the status code and reason phrase cannot always be
learned due to implementation limitations
+ // which means the message may not include them
+ assertEquals( assertMessageForBadMessage, "Resource
missing at " + forUrl, e.getMessage() );
+ }
+ else
+ {
+ assertEquals( assertMessageForBadMessage, "Resource
missing at " + forUrl
+ + " " + forStatusCode + " " + forReasonPhrase,
e.getMessage() );
+ }
+
+ break;
+
+ case HttpServletResponse.SC_FORBIDDEN:
+ assertTrue( "403 Forbidden throws AuthorizationException",
+ e instanceof AuthorizationException );
+
+ assertEquals( assertMessageForBadMessage, "Authorization
failed for " + forUrl + " 403"
+ + ( StringUtils.isEmpty( forReasonPhrase ) ? "
Forbidden" : ( " " + forReasonPhrase ) ),
+ e.getMessage() );
+ break;
+
+ case HttpServletResponse.SC_UNAUTHORIZED:
+ assertTrue( "401 Unauthorized throws
AuthorizationException",
+ e instanceof AuthorizationException );
+
+ assertEquals( assertMessageForBadMessage, "Authentication
failed for " + forUrl + " 401"
+ + ( StringUtils.isEmpty( forReasonPhrase )
? " Unauthorized" :
+ ( " " + forReasonPhrase ) ),
+ e.getMessage() );
+ break;
+
+ default:
+ assertTrue( "general exception must be
TransferFailedException",
+ e instanceof TransferFailedException );
+ assertTrue( "expected status code for transfer failures
should be >= 400, but none of "
+ + " the already handled codes",
+ forStatusCode >=
HttpServletResponse.SC_BAD_REQUEST );
+
+ if( e.getCause() != null ){
+ assertTrue("TransferFailedException should have the
original cause for diagnosis",
+ e.getCause() instanceof IOException );
+ }
+
+ // the status code and reason phrase cannot always be
learned due to implementation limitations
+ // so the message may not include them, but the
implementation should use a consistent format
+ assertTrue( "message should always include url",
+ e.getMessage().startsWith( "Transfer failed for "
+ forUrl ) );
+
+ if( e.getMessage().length() > ("Transfer failed for " +
forUrl).length() )
+ {
+ assertTrue( "message should include url and status
code",
+ e.getMessage().startsWith( "Transfer failed
for " + forUrl + " " + forStatusCode ) );
+ }
+
+ if( e.getMessage().length() > ("Transfer failed for " +
forUrl + " " + forStatusCode).length() )
+ {
+ assertEquals( "message should include url and status
code and reason phrase",
+ "Transfer failed for " + forUrl + " " +
forStatusCode + " " + forReasonPhrase,
+ e.getMessage() );
+ }
+
+ break;
+ }
+
+ }
+ catch ( AssertionError assertionError )
+ {
+ logger.error( "Exception which failed assertions: ", e );
+ throw assertionError;
+ }
+ }
+
}
diff --git
a/wagon-providers/wagon-http-shared/src/main/java/org/apache/maven/wagon/shared/http/AbstractHttpClientWagon.java
b/wagon-providers/wagon-http-shared/src/main/java/org/apache/maven/wagon/shared/http/AbstractHttpClientWagon.java
index 1d25664..9d3630b 100755
---
a/wagon-providers/wagon-http-shared/src/main/java/org/apache/maven/wagon/shared/http/AbstractHttpClientWagon.java
+++
b/wagon-providers/wagon-http-shared/src/main/java/org/apache/maven/wagon/shared/http/AbstractHttpClientWagon.java
@@ -105,6 +105,11 @@ import java.util.Properties;
import java.util.TimeZone;
import java.util.concurrent.TimeUnit;
+import static
org.apache.maven.wagon.shared.http.HttpMessageUtils.formatAuthorizationMessage;
+import static
org.apache.maven.wagon.shared.http.HttpMessageUtils.formatResourceDoesNotExistMessage;
+import static
org.apache.maven.wagon.shared.http.HttpMessageUtils.formatTransferDebugMessage;
+import static
org.apache.maven.wagon.shared.http.HttpMessageUtils.formatTransferFailedMessage;
+
/**
* @author <a href="michal.mac...@dimatics.com">Michal Maczka</a>
* @author <a href="mailto:ja...@atlassian.com";>James William Dumay</a>
@@ -713,17 +718,9 @@ public abstract class AbstractHttpClientWagon
CloseableHttpResponse response = execute( putMethod );
try
{
+ fireTransferDebug( formatTransferDebugMessage( url,
response.getStatusLine().getStatusCode(),
+ response.getStatusLine().getReasonPhrase(),
getProxyInfo() ) );
int statusCode = response.getStatusLine().getStatusCode();
- String reasonPhrase =
response.getStatusLine().getReasonPhrase();
- StringBuilder debugMessage = new StringBuilder();
- debugMessage.append( url );
- debugMessage.append( " -- " );
- debugMessage.append( "status code: " ).append( statusCode );
- if ( StringUtils.isNotEmpty( reasonPhrase ) )
- {
- debugMessage.append( ", reason phrase: " ).append(
reasonPhrase );
- }
- fireTransferDebug( debugMessage.toString() );
// Check that we didn't run out of retries.
switch ( statusCode )
@@ -741,20 +738,26 @@ public abstract class AbstractHttpClientWagon
case HttpStatus.SC_SEE_OTHER: // 303
put( resource, source, httpEntity,
calculateRelocatedUrl( response ) );
return;
+ //case HttpStatus.SC_UNAUTHORIZED:
case HttpStatus.SC_FORBIDDEN:
fireSessionConnectionRefused();
- throw new AuthorizationException( "Access denied to: "
+ url );
+ throw new AuthorizationException(
formatAuthorizationMessage( url,
+ response.getStatusLine().getStatusCode(),
+ response.getStatusLine().getReasonPhrase(),
getProxyInfo() ) );
case HttpStatus.SC_NOT_FOUND:
- throw new ResourceDoesNotExistException( "File " + url
+ " does not exist" );
+ throw new ResourceDoesNotExistException(
formatResourceDoesNotExistMessage( url,
+ response.getStatusLine().getStatusCode(),
+ response.getStatusLine().getReasonPhrase(),
getProxyInfo() ) );
case SC_TOO_MANY_REQUESTS:
put( backoff( wait, url ), resource, source,
httpEntity, url );
break;
//add more entries here
default:
- TransferFailedException e = new
TransferFailedException(
- "Failed to transfer file " + url + " with status
code " + statusCode );
+ TransferFailedException e = new
TransferFailedException( formatTransferFailedMessage( url,
+ response.getStatusLine().getStatusCode(),
+ response.getStatusLine().getReasonPhrase(),
getProxyInfo() ) );
fireTransferError( resource, e,
TransferEvent.REQUEST_PUT );
throw e;
}
@@ -768,23 +771,11 @@ public abstract class AbstractHttpClientWagon
response.close();
}
}
- catch ( IOException e )
- {
- fireTransferError( resource, e, TransferEvent.REQUEST_PUT );
-
- throw new TransferFailedException( e.getMessage(), e );
- }
- catch ( HttpException e )
+ catch ( IOException | HttpException | InterruptedException e )
{
fireTransferError( resource, e, TransferEvent.REQUEST_PUT );
- throw new TransferFailedException( e.getMessage(), e );
- }
- catch ( InterruptedException e )
- {
- fireTransferError( resource, e, TransferEvent.REQUEST_PUT );
-
- throw new TransferFailedException( e.getMessage(), e );
+ throw new TransferFailedException( formatTransferFailedMessage(
url, getProxyInfo() ), e );
}
}
@@ -831,14 +822,13 @@ public abstract class AbstractHttpClientWagon
case HttpStatus.SC_NOT_MODIFIED:
result = true;
break;
- case HttpStatus.SC_FORBIDDEN:
- throw new AuthorizationException( "Access denied to: "
+ url );
+ case HttpStatus.SC_FORBIDDEN:
case HttpStatus.SC_UNAUTHORIZED:
- throw new AuthorizationException( "Not authorized" );
-
case HttpStatus.SC_PROXY_AUTHENTICATION_REQUIRED:
- throw new AuthorizationException( "Not authorized by
proxy" );
+ throw new AuthorizationException(
formatAuthorizationMessage( url,
+ response.getStatusLine().getStatusCode(),
+ response.getStatusLine().getReasonPhrase(),
getProxyInfo() ) );
case HttpStatus.SC_NOT_FOUND:
result = false;
@@ -849,8 +839,9 @@ public abstract class AbstractHttpClientWagon
//add more entries here
default:
- throw new TransferFailedException(
- "Failed to transfer file " + url + " with status
code " + statusCode );
+ throw new TransferFailedException(
formatTransferFailedMessage( url,
+ response.getStatusLine().getStatusCode(),
+ response.getStatusLine().getReasonPhrase(),
getProxyInfo() ) );
}
EntityUtils.consume( response.getEntity() );
@@ -861,17 +852,9 @@ public abstract class AbstractHttpClientWagon
response.close();
}
}
- catch ( IOException e )
- {
- throw new TransferFailedException( e.getMessage(), e );
- }
- catch ( HttpException e )
+ catch ( IOException | HttpException | InterruptedException e )
{
- throw new TransferFailedException( e.getMessage(), e );
- }
- catch ( InterruptedException e )
- {
- throw new TransferFailedException( e.getMessage(), e );
+ throw new TransferFailedException( formatTransferFailedMessage(
url, getProxyInfo() ), e );
}
}
@@ -1116,17 +1099,10 @@ public abstract class AbstractHttpClientWagon
{
CloseableHttpResponse response = execute( getMethod );
closeable = response;
+
+ fireTransferDebug( formatTransferDebugMessage( url,
response.getStatusLine().getStatusCode(),
+ response.getStatusLine().getReasonPhrase(), getProxyInfo()
) );
int statusCode = response.getStatusLine().getStatusCode();
- String reasonPhrase = response.getStatusLine().getReasonPhrase();
- StringBuilder debugMessage = new StringBuilder();
- debugMessage.append( url );
- debugMessage.append( " -- " );
- debugMessage.append( "status code: " ).append( statusCode );
- if ( StringUtils.isNotEmpty( reasonPhrase ) )
- {
- debugMessage.append( ", reason phrase: " ).append(
reasonPhrase );
- }
- fireTransferDebug( debugMessage.toString() );
switch ( statusCode )
{
@@ -1136,20 +1112,19 @@ public abstract class AbstractHttpClientWagon
case HttpStatus.SC_NOT_MODIFIED:
// return, leaving last modified set to original value so
getIfNewer should return unmodified
return;
- case HttpStatus.SC_FORBIDDEN:
- fireSessionConnectionRefused();
- throw new AuthorizationException( "Access denied to: " +
url );
+ case HttpStatus.SC_FORBIDDEN:
case HttpStatus.SC_UNAUTHORIZED:
- fireSessionConnectionRefused();
- throw new AuthorizationException( "Not authorized" );
-
case HttpStatus.SC_PROXY_AUTHENTICATION_REQUIRED:
fireSessionConnectionRefused();
- throw new AuthorizationException( "Not authorized by
proxy" );
+ throw new AuthorizationException(
formatAuthorizationMessage( url,
+ response.getStatusLine().getStatusCode(),
response.getStatusLine().getReasonPhrase(),
+ getProxyInfo() ) );
case HttpStatus.SC_NOT_FOUND:
- throw new ResourceDoesNotExistException( "File " + url + "
does not exist" );
+ throw new ResourceDoesNotExistException(
formatResourceDoesNotExistMessage( url,
+ response.getStatusLine().getStatusCode(),
+ response.getStatusLine().getReasonPhrase(),
getProxyInfo() ) );
case SC_TOO_MANY_REQUESTS:
fillInputData( backoff( wait, url ), inputData );
@@ -1158,8 +1133,9 @@ public abstract class AbstractHttpClientWagon
// add more entries here
default:
cleanupGetTransfer( resource );
- TransferFailedException e = new TransferFailedException(
- "Failed to transfer file " + url + " with status code
" + statusCode );
+ TransferFailedException e = new TransferFailedException(
formatTransferFailedMessage( url,
+ response.getStatusLine().getStatusCode(),
response.getStatusLine().getReasonPhrase(),
+ getProxyInfo() ) );
fireTransferError( resource, e, TransferEvent.REQUEST_GET
);
throw e;
}
@@ -1199,23 +1175,11 @@ public abstract class AbstractHttpClientWagon
inputData.setInputStream( entity.getContent() );
}
}
- catch ( IOException e )
- {
- fireTransferError( resource, e, TransferEvent.REQUEST_GET );
-
- throw new TransferFailedException( e.getMessage(), e );
- }
- catch ( HttpException e )
- {
- fireTransferError( resource, e, TransferEvent.REQUEST_GET );
-
- throw new TransferFailedException( e.getMessage(), e );
- }
- catch ( InterruptedException e )
+ catch ( IOException | HttpException | InterruptedException e )
{
fireTransferError( resource, e, TransferEvent.REQUEST_GET );
- throw new TransferFailedException( e.getMessage(), e );
+ throw new TransferFailedException( formatTransferFailedMessage(
url, getProxyInfo() ), e );
}
}
diff --git
a/wagon-providers/wagon-http-shared/src/main/java/org/apache/maven/wagon/shared/http/HttpMessageUtils.java
b/wagon-providers/wagon-http-shared/src/main/java/org/apache/maven/wagon/shared/http/HttpMessageUtils.java
new file mode 100644
index 0000000..2b490d0
--- /dev/null
+++
b/wagon-providers/wagon-http-shared/src/main/java/org/apache/maven/wagon/shared/http/HttpMessageUtils.java
@@ -0,0 +1,222 @@
+package org.apache.maven.wagon.shared.http;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.apache.maven.wagon.ResourceDoesNotExistException;
+import org.apache.maven.wagon.TransferFailedException;
+import org.apache.maven.wagon.authorization.AuthorizationException;
+import org.apache.maven.wagon.proxy.ProxyInfo;
+import org.codehaus.plexus.util.StringUtils;
+
+/**
+ * Helper for HTTP related messages.
+ *
+ * @since 3.3.4
+ */
+public class HttpMessageUtils
+{
+ // status codes here to avoid checkstyle magic number and not have have
hard depend on non-wagon classes
+ private static final int SC_UNAUTHORIZED = 401;
+ private static final int SC_FORBIDDEN = 403;
+ private static final int SC_NOT_FOUND = 404;
+ private static final int SC_PROXY_AUTH_REQUIRED = 407;
+ private static final int SC_GONE = 410;
+
+ /**
+ * A HTTP status code used to indicate that the actual response status
code is not known at time of message
+ * generation.
+ */
+ public static final int UNKNOWN_STATUS_CODE = -1;
+
+ /**
+ * Format a consistent HTTP transfer debug message combining url, status
code, status line reason phrase and HTTP
+ * proxy server info.
+ * <p>
+ * Url will always be included in the message. A status code other than
{@link #UNKNOWN_STATUS_CODE} will be
+ * included. A reason phrase will only be included if non-empty and status
code is not {@link #UNKNOWN_STATUS_CODE}.
+ * Proxy information will only be included if not null.
+ *
+ * @param url the required non-null URL associated with the
message
+ * @param statusCode an HTTP response status code
+ * @param reasonPhrase an HTTP status line reason phrase
+ * @param proxyInfo proxy server used during the transfer, may be null
if none used
+ * @return a formatted debug message combining the parameters of this
method
+ * @throws NullPointerException if url is null
+ */
+ public static String formatTransferDebugMessage( String url, int
statusCode, String reasonPhrase,
+ ProxyInfo proxyInfo )
+ {
+ String msg = url;
+ if ( statusCode != UNKNOWN_STATUS_CODE )
+ {
+ msg += " -- status code: " + statusCode;
+ if ( StringUtils.isNotEmpty( reasonPhrase ) )
+ {
+ msg += ", reason phrase: " + reasonPhrase;
+ }
+ }
+ if ( proxyInfo != null )
+ {
+ msg += " -- " + proxyInfo.toString();
+ }
+ return msg;
+ }
+
+ /**
+ * Format a consistent message for HTTP related {@link
TransferFailedException}.
+ * <p>
+ * This variation typically used in cases where there is no HTTP transfer
response data to extract status code and
+ * reason phrase from. Equivalent to calling {@link
#formatTransferFailedMessage(String, int, String, ProxyInfo)}
+ * with {@link #UNKNOWN_STATUS_CODE} and null reason phrase.
+ *
+ * @param url the URL associated with the message
+ * @param proxyInfo proxy server used during the transfer, may be null if
none used
+ * @return a formatted failure message combining the parameters of this
method
+ */
+ public static String formatTransferFailedMessage( String url, ProxyInfo
proxyInfo )
+ {
+ return formatTransferFailedMessage( url, UNKNOWN_STATUS_CODE, null,
+ proxyInfo );
+ }
+
+ /**
+ * Format a consistent message for HTTP related {@link
TransferFailedException}.
+ *
+ * @param url the URL associated with the message
+ * @param statusCode an HTTP response status code or {@link
#UNKNOWN_STATUS_CODE}
+ * @param reasonPhrase an HTTP status line reason phrase or null if the
reason phrase unknown
+ * @param proxyInfo proxy server used during the transfer, may be null
if none used
+ * @return a formatted failure message combining the parameters of this
method
+ */
+ public static String formatTransferFailedMessage( String url, int
statusCode, String reasonPhrase,
+ ProxyInfo proxyInfo )
+ {
+ String msg = "Transfer failed for " + url;
+ if ( statusCode != UNKNOWN_STATUS_CODE )
+ {
+ msg += " " + statusCode;
+ // deliberately a null check instead of empty check so that we
avoid having to handle
+ // all conceivable default status code messages
+ if ( reasonPhrase != null )
+ {
+ msg += " " + reasonPhrase;
+ }
+ }
+ if ( proxyInfo != null )
+ {
+ msg += " " + proxyInfo.toString();
+ }
+ return msg;
+ }
+
+ /**
+ * Format a consistent message for HTTP related {@link
AuthorizationException}.
+ * <p>
+ * The message will always include the URL and status code provided. If
empty, the reason phrase is substituted with
+ * a common reason based on status code. {@link ProxyInfo} is only
included in the message if not null.
+ *
+ * @param url the URL associated with the message
+ * @param statusCode an HTTP response status code related to auth
+ * @param reasonPhrase an HTTP status line reason phrase
+ * @param proxyInfo proxy server used during the transfer, may be null
if none used
+ * @return a consistent message for a HTTP related {@link
AuthorizationException}
+ */
+ public static String formatAuthorizationMessage( String url, int
statusCode, String reasonPhrase,
+ ProxyInfo proxyInfo )
+ {
+ switch ( statusCode )
+ {
+ case SC_UNAUTHORIZED: // no credentials or auth was not valid
+ return "Authentication failed for " + url + " " + statusCode
+ + ( StringUtils.isEmpty( reasonPhrase ) ? "
Unauthorized" : " " + reasonPhrase );
+
+ case SC_FORBIDDEN: // forbidden based on permissions usually
+ return "Authorization failed for " + url + " " + statusCode
+ + ( StringUtils.isEmpty( reasonPhrase ) ? " Forbidden"
: " " + reasonPhrase );
+
+ case SC_PROXY_AUTH_REQUIRED:
+ return "HTTP proxy server authentication failed for " + url +
" " + statusCode
+ + ( StringUtils.isEmpty( reasonPhrase ) ? " Proxy
Authentication Required"
+ : " " + reasonPhrase );
+ default:
+ break;
+ }
+ String msg = "Authorization failed for " + url;
+ if ( statusCode != UNKNOWN_STATUS_CODE )
+ {
+ msg += " " + statusCode;
+ if ( StringUtils.isNotEmpty( reasonPhrase ) )
+ {
+ msg += " " + reasonPhrase;
+ }
+ }
+ if ( proxyInfo != null )
+ {
+ msg += " " + proxyInfo.toString();
+ }
+ return msg;
+
+ }
+
+ /**
+ * Format a consistent message for HTTP related {@link
ResourceDoesNotExistException}.
+ * <p>
+ * The message will always include the URL and status code provided. If
empty, the reason phrase is substituted with
+ * the commonly used reason phrases per status code. {@link ProxyInfo} is
only included if not null.
+ *
+ * @param url the URL associated with the message
+ * @param statusCode an HTTP response status code related to resources
not being found
+ * @param reasonPhrase an HTTP status line reason phrase
+ * @param proxyInfo proxy server used during the transfer, may be null
if none used
+ * @return a consistent message for a HTTP related {@link
ResourceDoesNotExistException}
+ */
+ public static String formatResourceDoesNotExistMessage( String url, int
statusCode, String reasonPhrase,
+ ProxyInfo
proxyInfo )
+ {
+ String msg = "Resource missing at " + url;
+
+ if ( statusCode != UNKNOWN_STATUS_CODE )
+ {
+ msg += " " + statusCode;
+
+ if ( StringUtils.isNotEmpty( reasonPhrase ) )
+ {
+ msg += " " + reasonPhrase;
+ }
+ else
+ {
+ if ( statusCode == SC_NOT_FOUND )
+ {
+ msg += " Not Found";
+ }
+ else if ( statusCode == SC_GONE )
+ {
+ msg += " Gone";
+ }
+ }
+ }
+ if ( proxyInfo != null )
+ {
+ msg += " " + proxyInfo.toString();
+ }
+ return msg;
+ }
+
+}
diff --git
a/wagon-providers/wagon-http/src/main/java/org/apache/maven/wagon/providers/http/HttpWagon.java
b/wagon-providers/wagon-http/src/main/java/org/apache/maven/wagon/providers/http/HttpWagon.java
index 6d2f5a1..9d53729 100755
---
a/wagon-providers/wagon-http/src/main/java/org/apache/maven/wagon/providers/http/HttpWagon.java
+++
b/wagon-providers/wagon-http/src/main/java/org/apache/maven/wagon/providers/http/HttpWagon.java
@@ -29,11 +29,16 @@ import org.apache.maven.wagon.TransferFailedException;
import org.apache.maven.wagon.authorization.AuthorizationException;
import org.apache.maven.wagon.shared.http.AbstractHttpClientWagon;
import org.apache.maven.wagon.shared.http.HtmlFileListParser;
+import org.apache.maven.wagon.shared.http.HttpMessageUtils;
import java.io.IOException;
import java.util.Collections;
import java.util.List;
+import static
org.apache.maven.wagon.shared.http.HttpMessageUtils.formatResourceDoesNotExistMessage;
+import static
org.apache.maven.wagon.shared.http.HttpMessageUtils.formatTransferDebugMessage;
+import static
org.apache.maven.wagon.shared.http.HttpMessageUtils.formatTransferFailedMessage;
+
/**
* @author <a href="michal.mac...@dimatics.com">Michal Maczka</a>
*/
@@ -64,9 +69,10 @@ public class HttpWagon
CloseableHttpResponse response = execute( getMethod );
try
{
+ String reasonPhrase =
response.getStatusLine().getReasonPhrase();
int statusCode = response.getStatusLine().getStatusCode();
- fireTransferDebug( url + " - Status code: " + statusCode );
+ fireTransferDebug( formatTransferDebugMessage( url,
statusCode, reasonPhrase, getProxyInfo() ) );
switch ( statusCode )
{
@@ -74,24 +80,22 @@ public class HttpWagon
break;
case HttpStatus.SC_FORBIDDEN:
- throw new AuthorizationException( "Access denied to: "
+ url );
-
case HttpStatus.SC_UNAUTHORIZED:
- throw new AuthorizationException( "Not authorized." );
-
case HttpStatus.SC_PROXY_AUTHENTICATION_REQUIRED:
- throw new AuthorizationException( "Not authorized by
proxy." );
+ throw new AuthorizationException(
HttpMessageUtils.formatAuthorizationMessage( url, statusCode,
+ reasonPhrase, getProxyInfo() ) );
case HttpStatus.SC_NOT_FOUND:
- throw new ResourceDoesNotExistException( "File: " +
url + " does not exist" );
+ throw new ResourceDoesNotExistException(
formatResourceDoesNotExistMessage( url, statusCode,
+ reasonPhrase, getProxyInfo() ) );
case SC_TOO_MANY_REQUESTS:
return getFileList( backoff( wait, url ),
destinationDirectory );
//add more entries here
default:
- throw new TransferFailedException(
- "Failed to transfer file: " + url + ". Return code
is: " + statusCode );
+ throw new TransferFailedException(
formatTransferFailedMessage( url, statusCode, reasonPhrase,
+ getProxyInfo() ) );
}
HttpEntity entity = response.getEntity();
if ( entity != null )
diff --git
a/wagon-providers/wagon-http/src/test/java/org/apache/maven/wagon/providers/http/ErrorWithMessageServlet.java
b/wagon-providers/wagon-http/src/test/java/org/apache/maven/wagon/providers/http/ErrorWithMessageServlet.java
index 95a86c1..bf74118 100644
---
a/wagon-providers/wagon-http/src/test/java/org/apache/maven/wagon/providers/http/ErrorWithMessageServlet.java
+++
b/wagon-providers/wagon-http/src/test/java/org/apache/maven/wagon/providers/http/ErrorWithMessageServlet.java
@@ -48,6 +48,10 @@ public class ErrorWithMessageServlet
{
response.sendError( 403, MESSAGE );
}
+ else if ( request.getRequestURL().toString().contains( "404" ) )
+ {
+ response.sendError( 404, MESSAGE );
+ }
else if ( request.getRequestURL().toString().contains( "407" ) )
{
response.sendError( 407, MESSAGE );
diff --git
a/wagon-providers/wagon-http/src/test/java/org/apache/maven/wagon/providers/http/HttpWagonErrorTest.java
b/wagon-providers/wagon-http/src/test/java/org/apache/maven/wagon/providers/http/HttpWagonErrorTest.java
index 85df40d..83d4387 100644
---
a/wagon-providers/wagon-http/src/test/java/org/apache/maven/wagon/providers/http/HttpWagonErrorTest.java
+++
b/wagon-providers/wagon-http/src/test/java/org/apache/maven/wagon/providers/http/HttpWagonErrorTest.java
@@ -20,6 +20,7 @@ package org.apache.maven.wagon.providers.http;
*/
import org.apache.maven.wagon.FileTestUtils;
+import org.apache.maven.wagon.ResourceDoesNotExistException;
import org.apache.maven.wagon.TransferFailedException;
import org.apache.maven.wagon.Wagon;
import org.apache.maven.wagon.authorization.AuthorizationException;
@@ -34,6 +35,8 @@ import java.io.File;
public class HttpWagonErrorTest
extends HttpWagonHttpServerTestCase
{
+ private int serverPort;
+
protected void setUp()
throws Exception
{
@@ -41,9 +44,10 @@ public class HttpWagonErrorTest
ServletHolder servlets = new ServletHolder( new
ErrorWithMessageServlet() );
context.addServlet( servlets, "/*" );
startServer();
+ serverPort = getPort();
}
- public void testGetReasonPhase401()
+ public void testGet401()
throws Exception
{
Exception thrown = null;
@@ -53,14 +57,14 @@ public class HttpWagonErrorTest
Wagon wagon = getWagon();
Repository testRepository = new Repository();
- testRepository.setUrl( "http://localhost:"; + getPort() );
+ testRepository.setUrl( "http://localhost:"; + serverPort );
wagon.connect( testRepository );
File destFile = FileTestUtils.createUniqueFile( getName(),
getName() );
destFile.deleteOnExit();
- wagon.get( "/401", destFile );
+ wagon.get( "401", destFile );
wagon.disconnect();
}
@@ -75,9 +79,11 @@ public class HttpWagonErrorTest
assertNotNull( thrown );
assertEquals( AuthorizationException.class, thrown.getClass() );
+ assertEquals("Authentication failed for http://localhost:"; + serverPort
+ + "/401 401 " + ErrorWithMessageServlet.MESSAGE,
thrown.getMessage());
}
- public void testGetReasonPhase403()
+ public void testGet403()
throws Exception
{
Exception thrown = null;
@@ -87,14 +93,14 @@ public class HttpWagonErrorTest
Wagon wagon = getWagon();
Repository testRepository = new Repository();
- testRepository.setUrl( "http://localhost:"; + getPort() );
+ testRepository.setUrl( "http://localhost:"; + serverPort );
wagon.connect( testRepository );
File destFile = FileTestUtils.createUniqueFile( getName(),
getName() );
destFile.deleteOnExit();
- wagon.get( "/403", destFile );
+ wagon.get( "403", destFile );
wagon.disconnect();
}
@@ -109,10 +115,47 @@ public class HttpWagonErrorTest
assertNotNull( thrown );
assertEquals( AuthorizationException.class, thrown.getClass() );
+ assertEquals("Authorization failed for http://localhost:"; + serverPort
+ + "/403 403 " + ErrorWithMessageServlet.MESSAGE,
thrown.getMessage());
}
+ public void testGet404()
+ throws Exception
+ {
+ Exception thrown = null;
+
+ try
+ {
+ Wagon wagon = getWagon();
+
+ Repository testRepository = new Repository();
+ testRepository.setUrl( "http://localhost:"; + serverPort );
+
+ wagon.connect( testRepository );
+
+ File destFile = FileTestUtils.createUniqueFile( getName(),
getName() );
+ destFile.deleteOnExit();
+
+ wagon.get( "404", destFile );
+
+ wagon.disconnect();
+ }
+ catch ( Exception e )
+ {
+ thrown = e;
+ }
+ finally
+ {
+ stopServer();
+ }
+
+ assertNotNull( thrown );
+ assertEquals( ResourceDoesNotExistException.class, thrown.getClass() );
+ assertEquals("Resource missing at http://localhost:"; + serverPort +
"/404 404 "
+ + ErrorWithMessageServlet.MESSAGE, thrown.getMessage());
+ }
- public void testGetReasonPhase407()
+ public void testGet407()
throws Exception
{
Exception thrown = null;
@@ -129,7 +172,7 @@ public class HttpWagonErrorTest
File destFile = FileTestUtils.createUniqueFile( getName(),
getName() );
destFile.deleteOnExit();
- wagon.get( "/407", destFile );
+ wagon.get( "407", destFile );
wagon.disconnect();
}
@@ -144,9 +187,11 @@ public class HttpWagonErrorTest
assertNotNull( thrown );
assertEquals( AuthorizationException.class, thrown.getClass() );
+ assertEquals("HTTP proxy server authentication failed for
http://localhost:"; + serverPort
+ + "/407 407 " + ErrorWithMessageServlet.MESSAGE,
thrown.getMessage());
}
- public void testGetReasonPhase500()
+ public void testGet500()
throws Exception
{
Exception thrown = null;
@@ -156,14 +201,14 @@ public class HttpWagonErrorTest
Wagon wagon = getWagon();
Repository testRepository = new Repository();
- testRepository.setUrl( "http://localhost:"; + getPort() );
+ testRepository.setUrl( "http://localhost:"; + serverPort );
wagon.connect( testRepository );
File destFile = FileTestUtils.createUniqueFile( getName(),
getName() );
destFile.deleteOnExit();
- wagon.get( "/500", destFile );
+ wagon.get( "500", destFile );
wagon.disconnect();
}
@@ -178,5 +223,7 @@ public class HttpWagonErrorTest
assertNotNull( thrown );
assertEquals( TransferFailedException.class, thrown.getClass() );
+ assertEquals("Transfer failed for http://localhost:"; + serverPort +
"/500 500 "
+ + ErrorWithMessageServlet.MESSAGE, thrown.getMessage());
}
}
diff --git
a/wagon-providers/wagon-webdav-jackrabbit/src/test/java/org/apache/maven/wagon/providers/webdav/WebDavWagonTest.java
b/wagon-providers/wagon-webdav-jackrabbit/src/test/java/org/apache/maven/wagon/providers/webdav/WebDavWagonTest.java
index c8a3692..5c07d87 100644
---
a/wagon-providers/wagon-webdav-jackrabbit/src/test/java/org/apache/maven/wagon/providers/webdav/WebDavWagonTest.java
+++
b/wagon-providers/wagon-webdav-jackrabbit/src/test/java/org/apache/maven/wagon/providers/webdav/WebDavWagonTest.java
@@ -523,4 +523,11 @@ public class WebDavWagonTest
}
+ @Override
+ protected void verifyWagonExceptionMessage( Exception e, int
forStatusCode, String forUrl, String forReasonPhrase )
+ {
+ Repository repo = new Repository( "test-geturl", forUrl );
+ String expectedMessageUrl = ( new WebDavWagon() ).getURL( repo );
+ super.verifyWagonExceptionMessage( e, forStatusCode,
expectedMessageUrl, forReasonPhrase );
+ }
}
diff --git
a/wagon-tcks/wagon-tck-http/src/main/java/org/apache/maven/wagon/tck/http/Assertions.java
b/wagon-tcks/wagon-tck-http/src/main/java/org/apache/maven/wagon/tck/http/Assertions.java
index 9a10944..eb7cfa8 100644
---
a/wagon-tcks/wagon-tck-http/src/main/java/org/apache/maven/wagon/tck/http/Assertions.java
+++
b/wagon-tcks/wagon-tck-http/src/main/java/org/apache/maven/wagon/tck/http/Assertions.java
@@ -19,21 +19,36 @@ package org.apache.maven.wagon.tck.http;
* under the License.
*/
-import static junit.framework.Assert.assertEquals;
-import static org.codehaus.plexus.util.FileUtils.fileRead;
-
+import org.apache.maven.wagon.ResourceDoesNotExistException;
+import org.apache.maven.wagon.TransferFailedException;
+import org.apache.maven.wagon.WagonException;
+import org.apache.maven.wagon.authorization.AuthorizationException;
+import org.apache.maven.wagon.proxy.ProxyInfo;
import org.codehaus.plexus.util.IOUtil;
+import org.codehaus.plexus.util.StringUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import javax.servlet.http.HttpServletResponse;
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
+import static org.codehaus.plexus.util.FileUtils.fileRead;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
/**
*
*/
public final class Assertions
{
+ public static final int NO_RESPONSE_STATUS_CODE = -1;
+
+ protected static final Logger LOGGER = LoggerFactory.getLogger(
Assertions.class );
+
public static void assertFileContentsFromResource( final String
resourceBase, final String resourceName,
final File output,
final String whyWouldItFail )
throws IOException
@@ -67,4 +82,120 @@ public final class Assertions
return resource;
}
+ /**
+ * Assert a WagonException message contains required format and context
based on the status code we expected to
+ * trigger it in the first place.
+ * <p>
+ * This implementation represents the most desired assertions, but
HttpWagonTestCase sub-classes could override
+ * this method if a specific wagon representation makes it impossible to
meet these assertions.
+ *
+ * @param e an instance of {@link WagonException}
+ * @param forStatusCode the response status code that triggered the
exception
+ * @param forUrl the url that triggered the exception
+ * @param forReasonPhrase the optional status line reason phrase the
server returned
+ */
+ public static void assertWagonExceptionMessage( Exception e, int
forStatusCode, String forUrl,
+ String forReasonPhrase,
ProxyInfo proxyInfo )
+ {
+ // TODO: handle AuthenticationException for Wagon.connect() calls
+ assertNotNull( e );
+ try
+ {
+ assertTrue( "only verify instances of WagonException", e
instanceof WagonException );
+
+ String reasonPhrase;
+ String assertMessageForBadMessage = "exception message not
described properly";
+
+ if ( proxyInfo != null )
+ {
+ assertTrue( "message should end with proxy information if
proxy was used",
+ e.getMessage().endsWith( proxyInfo.toString() ) );
+ }
+
+ switch ( forStatusCode )
+ {
+ case HttpServletResponse.SC_NOT_FOUND:
+ // TODO: add test for 410: Gone?
+ assertTrue( "404 not found response should throw
ResourceDoesNotExistException",
+ e instanceof ResourceDoesNotExistException );
+ reasonPhrase = StringUtils.isEmpty( forReasonPhrase ) ? "
Not Found" : ( " " + forReasonPhrase );
+ assertEquals( assertMessageForBadMessage, "Resource
missing at " + forUrl + " 404"
+ + reasonPhrase, e.getMessage() );
+ break;
+
+ case HttpServletResponse.SC_UNAUTHORIZED:
+ // FIXME assumes Wagon.get()/put() returning 401 instead
of Wagon.connect()
+ assertTrue( "401 Unauthorized should throw
AuthorizationException since "
+ + " AuthenticationException is not
explicitly declared as thrown from wagon "
+ + "methods",
+ e instanceof AuthorizationException );
+ reasonPhrase = StringUtils.isEmpty( forReasonPhrase ) ? "
Unauthorized" : ( " " + forReasonPhrase );
+ assertEquals( assertMessageForBadMessage, "Authentication
failed for " + forUrl + " 401"
+ + reasonPhrase, e.getMessage() );
+ break;
+
+ case HttpServletResponse.SC_PROXY_AUTHENTICATION_REQUIRED:
+ assertTrue( "407 Proxy authentication required should
throw AuthorizationException",
+ e instanceof AuthorizationException );
+ reasonPhrase = StringUtils.isEmpty( forReasonPhrase ) ? "
Proxy Authentication Required"
+ : ( " " + forReasonPhrase );
+ assertEquals( assertMessageForBadMessage, "HTTP proxy
server authentication failed for "
+ + forUrl + " 407" + reasonPhrase, e.getMessage() );
+ break;
+
+ case HttpServletResponse.SC_FORBIDDEN:
+ assertTrue( "403 Forbidden should throw
AuthorizationException",
+ e instanceof AuthorizationException );
+ reasonPhrase = StringUtils.isEmpty( forReasonPhrase ) ? "
Forbidden" : ( " " + forReasonPhrase );
+ assertEquals( assertMessageForBadMessage, "Authorization
failed for " + forUrl + " 403"
+ + reasonPhrase, e.getMessage() );
+ break;
+
+ default:
+ assertTrue( "transfer failures should at least be wrapped
in a TransferFailedException", e
+ instanceof TransferFailedException );
+
+ // the status code and reason phrase cannot always be
learned due to implementation limitations
+ // so the message may not include them, but the
implementation should use a consistent format
+ assertTrue( "message should always include url tried: " +
e.getMessage(),
+ e.getMessage().startsWith( "Transfer failed for "
+ forUrl ) );
+
+ String statusCodeStr = forStatusCode ==
NO_RESPONSE_STATUS_CODE ? ""
+ : String.valueOf( forStatusCode );
+ if ( forStatusCode != NO_RESPONSE_STATUS_CODE )
+ {
+
+ assertTrue( "if there was a response status line, the
status code should be >= 400",
+ forStatusCode >=
HttpServletResponse.SC_BAD_REQUEST );
+
+ if ( e.getMessage().length() > ( "Transfer failed for
" + forUrl ).length() )
+ {
+ assertTrue( "message should include url and status
code: " + e.getMessage(),
+ e.getMessage().startsWith( "Transfer
failed for " + forUrl + statusCodeStr ) );
+ }
+
+ reasonPhrase = forReasonPhrase == null ? "" : " " +
forReasonPhrase;
+
+ if ( reasonPhrase.length() > 0 &&
e.getMessage().length() > ( "Transfer failed for " + forUrl
+ + statusCodeStr ).length() )
+ {
+ assertTrue( "message should include url and status
code and reason phrase: "
+ + e.getMessage(),
e.getMessage().startsWith( "Transfer failed for "
+ + forUrl + statusCodeStr
+ + reasonPhrase ) );
+ }
+
+ }
+
+ break;
+ }
+ }
+ catch ( AssertionError assertionError )
+ {
+ LOGGER.error( "Exception which failed assertions: ", e );
+ throw assertionError;
+ }
+
+ }
+
}
diff --git
a/wagon-tcks/wagon-tck-http/src/main/java/org/apache/maven/wagon/tck/http/GetWagonTests.java
b/wagon-tcks/wagon-tck-http/src/main/java/org/apache/maven/wagon/tck/http/GetWagonTests.java
index 1562369..ac48636 100644
---
a/wagon-tcks/wagon-tck-http/src/main/java/org/apache/maven/wagon/tck/http/GetWagonTests.java
+++
b/wagon-tcks/wagon-tck-http/src/main/java/org/apache/maven/wagon/tck/http/GetWagonTests.java
@@ -45,7 +45,9 @@ import java.io.IOException;
import static junit.framework.Assert.assertTrue;
import static junit.framework.Assert.fail;
+import static
org.apache.maven.wagon.tck.http.Assertions.NO_RESPONSE_STATUS_CODE;
import static
org.apache.maven.wagon.tck.http.Assertions.assertFileContentsFromResource;
+import static
org.apache.maven.wagon.tck.http.Assertions.assertWagonExceptionMessage;
/**
*
@@ -114,7 +116,7 @@ public class GetWagonTests
return;
}
- final ValueHolder<Boolean> holder = new ValueHolder<Boolean>( false );
+ final ValueHolder<TransferFailedException> holder = new ValueHolder<>(
null );
Runnable r = new Runnable()
{
@@ -141,34 +143,15 @@ public class GetWagonTests
fail( "Should have failed to transfer due to transaction
timeout." );
}
- catch ( ConnectionException e )
- {
- throw new IllegalStateException( e );
- }
- catch ( AuthenticationException e )
+ catch ( ConnectionException | AuthenticationException |
ResourceDoesNotExistException
+ | AuthorizationException |
ComponentConfigurationException | IOException e )
{
throw new IllegalStateException( e );
}
catch ( TransferFailedException e )
{
// expected
- holder.setValue( true );
- }
- catch ( ResourceDoesNotExistException e )
- {
- throw new IllegalStateException( e );
- }
- catch ( AuthorizationException e )
- {
- throw new IllegalStateException( e );
- }
- catch ( ComponentConfigurationException e )
- {
- throw new IllegalStateException( e );
- }
- catch ( IOException e )
- {
- throw new IllegalStateException( e );
+ holder.setValue( e );
}
}
};
@@ -189,7 +172,9 @@ public class GetWagonTests
logger.info( "Interrupting thread." );
t.interrupt();
- assertTrue( "TransferFailedException should have been thrown.",
holder.getValue() );
+ assertTrue( "TransferFailedException should have been thrown.",
holder.getValue() != null );
+ assertWagonExceptionMessage( holder.getValue(),
NO_RESPONSE_STATUS_CODE, getBaseUrl() + "infinite/",
+ "", null );
}
@Test
@@ -214,6 +199,8 @@ public class GetWagonTests
catch ( TransferFailedException e )
{
// expected
+ assertWagonExceptionMessage( e, NO_RESPONSE_STATUS_CODE,
"http://localhost:65520/base.txt";,
+ null, null );
}
}
