You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cxf.apache.org by se...@apache.org on 2013/07/19 18:28:33 UTC
svn commit: r1504932 - in /cxf/branches/2.7.x-fixes: ./
rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/client/
rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/impl/
rt/transports/http/src/main/java/org/apache/cxf/transport/http/
systests/jax...
Author: sergeyb
Date: Fri Jul 19 16:28:33 2013
New Revision: 1504932
URL: http://svn.apache.org/r1504932
Log:
Merged revisions 1504921 via svnmerge from
https://svn.apache.org/repos/asf/cxf/trunk
........
r1504921 | sergeyb | 2013-07-19 16:37:39 +0100 (Fri, 19 Jul 2013) | 1 line
[CXF-5122] Optional support for restricting redirects to same host and for relative redirects
........
Modified:
cxf/branches/2.7.x-fixes/ (props changed)
cxf/branches/2.7.x-fixes/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/client/AbstractClient.java
cxf/branches/2.7.x-fixes/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/impl/ResponseBuilderImpl.java
cxf/branches/2.7.x-fixes/rt/transports/http/src/main/java/org/apache/cxf/transport/http/HTTPConduit.java
cxf/branches/2.7.x-fixes/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/BookStore.java
cxf/branches/2.7.x-fixes/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRSClientServerBookTest.java
Propchange: cxf/branches/2.7.x-fixes/
------------------------------------------------------------------------------
Merged /cxf/trunk:r1504921
Propchange: cxf/branches/2.7.x-fixes/
------------------------------------------------------------------------------
Binary property 'svnmerge-integrated' - no diff available.
Modified: cxf/branches/2.7.x-fixes/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/client/AbstractClient.java
URL: http://svn.apache.org/viewvc/cxf/branches/2.7.x-fixes/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/client/AbstractClient.java?rev=1504932&r1=1504931&r2=1504932&view=diff
==============================================================================
--- cxf/branches/2.7.x-fixes/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/client/AbstractClient.java (original)
+++ cxf/branches/2.7.x-fixes/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/client/AbstractClient.java Fri Jul 19 16:28:33 2013
@@ -549,12 +549,16 @@ public abstract class AbstractClient imp
}
protected void checkClientException(Message outMessage, Exception ex) throws Exception {
+ Throwable actualEx = ex instanceof Fault ? ((Fault)ex).getCause() : ex;
+
Integer responseCode = getResponseCode(outMessage.getExchange());
- if (responseCode == null) {
- if (ex instanceof ClientException) {
+ if (responseCode == null
+ || actualEx instanceof IOException
+ && outMessage.getExchange().get("client.redirect.exception") != null) {
+ if (actualEx instanceof ClientException) {
throw ex;
- } else if (ex != null) {
- throw new ClientException(ex);
+ } else if (actualEx != null) {
+ throw new ClientException(actualEx);
} else if (!outMessage.getExchange().isOneWay() || cfg.isResponseExpectedForOneway()) {
waitForResponseCode(outMessage.getExchange());
}
Modified: cxf/branches/2.7.x-fixes/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/impl/ResponseBuilderImpl.java
URL: http://svn.apache.org/viewvc/cxf/branches/2.7.x-fixes/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/impl/ResponseBuilderImpl.java?rev=1504932&r1=1504931&r2=1504932&view=diff
==============================================================================
--- cxf/branches/2.7.x-fixes/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/impl/ResponseBuilderImpl.java (original)
+++ cxf/branches/2.7.x-fixes/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/impl/ResponseBuilderImpl.java Fri Jul 19 16:28:33 2013
@@ -157,8 +157,6 @@ public final class ResponseBuilderImpl e
if (HttpUtils.isDateRelatedHeader(name)) {
Object theValue = value instanceof Date ? toHttpDate((Date)value) : value;
return setHeader(name, theValue);
- } else if (HttpHeaders.LOCATION.equals(name)) {
- return location(URI.create(value.toString()));
} else {
return addHeader(name, value);
}
Modified: cxf/branches/2.7.x-fixes/rt/transports/http/src/main/java/org/apache/cxf/transport/http/HTTPConduit.java
URL: http://svn.apache.org/viewvc/cxf/branches/2.7.x-fixes/rt/transports/http/src/main/java/org/apache/cxf/transport/http/HTTPConduit.java?rev=1504932&r1=1504931&r2=1504932&view=diff
==============================================================================
--- cxf/branches/2.7.x-fixes/rt/transports/http/src/main/java/org/apache/cxf/transport/http/HTTPConduit.java (original)
+++ cxf/branches/2.7.x-fixes/rt/transports/http/src/main/java/org/apache/cxf/transport/http/HTTPConduit.java Fri Jul 19 16:28:33 2013
@@ -163,8 +163,12 @@ public abstract class HTTPConduit
* Endpoint Qname to give the configuration name of this conduit.
*/
private static final String SC_HTTP_CONDUIT_SUFFIX = ".http-conduit";
-
- private static final String HTTP_POST_METHOD = "POST";
+
+ private static final String AUTO_REDIRECT_SAME_HOST_ONLY = "http.redirect.same.host.only";
+ private static final String AUTO_REDIRECT_ALLOW_REL_URI = "http.redirect.relative.uri";
+
+
+ private static final String HTTP_POST_METHOD = "POST";
private static final String HTTP_PUT_METHOD = "PUT";
private static final Set<String> KNOWN_HTTP_VERBS_WITH_NO_CONTENT =
new HashSet<String>(Arrays.asList(new String[]{"GET", "DELETE", "HEAD", "OPTIONS", "TRACE"}));
@@ -1402,9 +1406,21 @@ public abstract class HTTPConduit
}
Message m = new MessageImpl();
updateResponseHeaders(m);
+
String newURL = extractLocation(Headers.getSetProtocolHeaders(m));
-
- detectRedirectLoop(getConduitName(), url.toString(), newURL, outMessage);
+ String urlString = url.toString();
+
+ try {
+ detectRedirectLoop(conduitName, urlString, newURL, outMessage);
+ newURL = convertToAbsoluteUrlIfNeeded(conduitName, urlString, newURL, outMessage);
+ checkSameBaseUriRedirect(conduitName, urlString, newURL, outMessage);
+ } catch (IOException ex) {
+ // Consider introducing ClientRedirectException instead - it will require
+ // those client runtimes which want to check for it have a direct link to it
+ outMessage.getExchange().put("client.redirect.exception", "true");
+ throw ex;
+ }
+
if (newURL != null) {
new Headers(outMessage).removeAuthorizationHeaders();
@@ -1710,6 +1726,64 @@ public abstract class HTTPConduit
}
}
+ private static void checkSameBaseUriRedirect(String conduitName,
+ String lastURL,
+ String newURL,
+ Message message) throws IOException {
+ if (newURL != null
+ && MessageUtils.isTrue(message.getContextualProperty(AUTO_REDIRECT_SAME_HOST_ONLY))) {
+ URI newUri = URI.create(newURL);
+ URI lastUri = URI.create(lastURL);
+ // This can be further restricted to make sure newURL completely contains lastURL
+ // though making sure the same HTTP scheme and host are preserved should be enough
+
+ if (!newUri.getScheme().equals(lastUri.getScheme())
+ || !newUri.getHost().equals(lastUri.getHost())) {
+ String msg = "Different HTTP Scheme or Host Redirect detected on Conduit '"
+ + conduitName + "' on '" + newURL + "'";
+ LOG.log(Level.INFO, msg);
+ throw new IOException(msg);
+ }
+ }
+ }
+
+ // http://tools.ietf.org/html/draft-ietf-httpbis-p2-semantics-23#section-7.1.2
+ // Relative Location values are also supported
+ private static String convertToAbsoluteUrlIfNeeded(String conduitName,
+ String lastURL,
+ String newURL,
+ Message message) throws IOException {
+ if (newURL != null && !newURL.startsWith("http")) {
+
+ if (MessageUtils.isTrue(message.getContextualProperty(AUTO_REDIRECT_ALLOW_REL_URI))) {
+
+ int queryInd = lastURL.lastIndexOf('?');
+ String query = queryInd == -1 ? null : lastURL.substring(queryInd);
+ String newAbsURL = queryInd == -1 ? lastURL : lastURL.substring(0, queryInd);
+ if (newAbsURL.endsWith("/")) {
+ newAbsURL = newAbsURL.substring(0, newAbsURL.length() - 1);
+ }
+ newAbsURL = newAbsURL + newURL;
+ if (query != null) {
+ if (newAbsURL.lastIndexOf("?") != -1) {
+ newAbsURL += "&";
+ query = query.substring(1);
+ }
+ newAbsURL += query;
+ }
+ return newAbsURL;
+ } else {
+ String msg = "Relative Redirect detected on Conduit '"
+ + conduitName + "' on '" + newURL + "'";
+ LOG.log(Level.INFO, msg);
+ throw new IOException(msg);
+ }
+ } else {
+ return newURL;
+ }
+
+ }
+
private static void detectRedirectLoop(String conduitName,
String lastURL,
String newURL,
@@ -1721,14 +1795,20 @@ public abstract class HTTPConduit
message.put(KEY_VISITED_URLS, visitedURLs);
}
visitedURLs.add(lastURL);
- if (newURL != null && visitedURLs.contains(newURL)) {
- // See if we are being redirected in a loop as best we can,
- // using string equality on URL.
- // We are in a redirect loop; -- bail
- String msg = "Redirect loop detected on Conduit '"
- + conduitName + "' on '" + newURL + "'";
- LOG.log(Level.INFO, msg);
- throw new IOException(msg);
+ if (newURL != null) {
+ if (visitedURLs.contains(newURL)) {
+ // See if we are being redirected in a loop as best we can,
+ // using string equality on URL.
+ // We are in a redirect loop; -- bail
+ String msg = "Redirect loop detected on Conduit '"
+ + conduitName + "' on '" + newURL + "'";
+ LOG.log(Level.INFO, msg);
+ throw new IOException(msg);
+ }
+ // Important to prevent looping on relative URIs
+ if (!newURL.startsWith("http")) {
+ visitedURLs.add(newURL);
+ }
}
}
private static void detectAuthorizationLoop(String conduitName, Message message,
Modified: cxf/branches/2.7.x-fixes/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/BookStore.java
URL: http://svn.apache.org/viewvc/cxf/branches/2.7.x-fixes/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/BookStore.java?rev=1504932&r1=1504931&r2=1504932&view=diff
==============================================================================
--- cxf/branches/2.7.x-fixes/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/BookStore.java (original)
+++ cxf/branches/2.7.x-fixes/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/BookStore.java Fri Jul 19 16:28:33 2013
@@ -147,6 +147,35 @@ public class BookStore {
}
@GET
+ @Path("/redirect")
+ public Response getBookRedirect(@QueryParam("redirect") Boolean done,
+ @QueryParam("sameuri") Boolean sameuri) {
+ if (done == null) {
+ String uri = sameuri.equals(Boolean.TRUE)
+ ? ui.getAbsolutePathBuilder().queryParam("redirect", "true").build().toString()
+ : "http://otherhost/redirect";
+ return Response.status(303).header("Location", uri).build();
+ } else {
+ return Response.ok(new Book("CXF", 123L), "application/xml").build();
+ }
+ }
+
+ @GET
+ @Path("/redirect/relative")
+ public Response getBookRedirectRel(@QueryParam("redirect") Boolean done,
+ @QueryParam("loop") boolean loop) {
+ if (done == null) {
+ if (loop) {
+ return Response.status(303).header("Location", "/?a").build();
+ } else {
+ return Response.status(303).header("Location", "/?redirect=true").build();
+ }
+ } else {
+ return Response.ok(new Book("CXF", 124L), "application/xml").build();
+ }
+ }
+
+ @GET
@Path("/booklist")
public List<String> getBookListArray() {
return Collections.singletonList("Good book");
@@ -1416,7 +1445,7 @@ public class BookStore {
public void write(OutputStream output) throws IOException, WebApplicationException {
if (failEarly) {
throw new WebApplicationException(
- Response.status(410).type("text/plain")
+ Response.status(410).header("content-type", "text/plain")
.entity("This is supposed to go on the wire").build());
} else {
output.write("This is not supposed to go on the wire".getBytes());
Modified: cxf/branches/2.7.x-fixes/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRSClientServerBookTest.java
URL: http://svn.apache.org/viewvc/cxf/branches/2.7.x-fixes/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRSClientServerBookTest.java?rev=1504932&r1=1504931&r2=1504932&view=diff
==============================================================================
--- cxf/branches/2.7.x-fixes/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRSClientServerBookTest.java (original)
+++ cxf/branches/2.7.x-fixes/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRSClientServerBookTest.java Fri Jul 19 16:28:33 2013
@@ -92,6 +92,72 @@ public class JAXRSClientServerBookTest e
}
@Test
+ public void testGetBookSameUriAutoRedirect() throws Exception {
+ String address = "http://localhost:" + PORT + "/bookstore/redirect?sameuri=true";
+ WebClient wc = WebClient.create(address);
+ WebClient.getConfig(wc).getHttpConduit().getClient().setAutoRedirect(true);
+ Response r = wc.get();
+ Book book = r.readEntity(Book.class);
+ assertEquals(123L, book.getId());
+ }
+
+ @Test
+ public void testGetBookDiffUriAutoRedirect() throws Exception {
+ String address = "http://localhost:" + PORT + "/bookstore/redirect?sameuri=false";
+ WebClient wc = WebClient.create(address);
+ WebClient.getConfig(wc).getRequestContext().put("http.redirect.same.host.only", "true");
+ WebClient.getConfig(wc).getHttpConduit().getClient().setAutoRedirect(true);
+ try {
+ wc.get();
+ fail("Redirect to different host is not allowed");
+ } catch (ClientException ex) {
+ Throwable cause = ex.getCause();
+ assertTrue(cause.getMessage().contains("Different HTTP Scheme or Host Redirect detected on"));
+ }
+ }
+
+
+ @Test
+ public void testGetBookRelativeUriAutoRedirect() throws Exception {
+ String address = "http://localhost:" + PORT + "/bookstore/redirect/relative?loop=false";
+ WebClient wc = WebClient.create(address);
+ WebClient.getConfig(wc).getRequestContext().put("http.redirect.relative.uri", "true");
+ WebClient.getConfig(wc).getHttpConduit().getClient().setAutoRedirect(true);
+ Response r = wc.get();
+ Book book = r.readEntity(Book.class);
+ assertEquals(124L, book.getId());
+ }
+
+ @Test
+ public void testGetBookRelativeUriAutoRedirectLoop() throws Exception {
+ String address = "http://localhost:" + PORT + "/bookstore/redirect/relative?loop=true";
+ WebClient wc = WebClient.create(address);
+ WebClient.getConfig(wc).getRequestContext().put("http.redirect.relative.uri", "true");
+ WebClient.getConfig(wc).getHttpConduit().getClient().setAutoRedirect(true);
+ try {
+ wc.get();
+ fail("Redirect loop must be detected");
+ } catch (ClientException ex) {
+ Throwable cause = ex.getCause();
+ assertTrue(cause.getMessage().contains("Redirect loop detected on"));
+ }
+ }
+
+ @Test
+ public void testGetBookRelativeUriAutoRedirectNotAllowed() throws Exception {
+ String address = "http://localhost:" + PORT + "/bookstore/redirect/relative?loop=true";
+ WebClient wc = WebClient.create(address);
+ WebClient.getConfig(wc).getHttpConduit().getClient().setAutoRedirect(true);
+ try {
+ wc.get();
+ fail("relative Redirect is not allowed");
+ } catch (ClientException ex) {
+ Throwable cause = ex.getCause().getCause();
+ assertTrue(cause.getMessage().startsWith("Relative Redirect detected on"));
+ }
+ }
+
+ @Test
public void testPostEmptyForm() throws Exception {
String address = "http://localhost:" + PORT + "/bookstore/emptyform";
WebClient wc = WebClient.create(address);