From f816724db965b342dbb08b2ce946bc91e60db4ec Mon Sep 17 00:00:00 2001 From: Matt Fletcher Date: Wed, 12 Sep 2018 16:30:53 -0400 Subject: [PATCH 1/9] Deprecate URLBuilder in favor of AddressBuilder. These are the last two internal use cases for URLBuilder. And replacing them makes my IDE happy. Both are similar to the create case but different. InboundRewriteRuleAdaptor needs the encoding. RewriteViewHandler needs to merge two query strings. --- .../InboundRewriteRuleAdaptor.java | 40 +++++++++++++++---- .../rewrite/faces/RewriteViewHandler.java | 40 +++++++++++++++++-- 2 files changed, 69 insertions(+), 11 deletions(-) diff --git a/config-prettyfaces/src/main/java/org/ocpsoft/rewrite/prettyfaces/InboundRewriteRuleAdaptor.java b/config-prettyfaces/src/main/java/org/ocpsoft/rewrite/prettyfaces/InboundRewriteRuleAdaptor.java index c383151ec..65f194634 100644 --- a/config-prettyfaces/src/main/java/org/ocpsoft/rewrite/prettyfaces/InboundRewriteRuleAdaptor.java +++ b/config-prettyfaces/src/main/java/org/ocpsoft/rewrite/prettyfaces/InboundRewriteRuleAdaptor.java @@ -21,12 +21,17 @@ */ package org.ocpsoft.rewrite.prettyfaces; +import java.net.URI; +import java.net.URISyntaxException; + import org.ocpsoft.rewrite.config.Rule; import org.ocpsoft.rewrite.context.EvaluationContext; import org.ocpsoft.rewrite.event.Rewrite; import org.ocpsoft.rewrite.servlet.http.event.HttpInboundServletRewrite; import org.ocpsoft.rewrite.servlet.http.event.HttpServletRewrite; -import org.ocpsoft.rewrite.servlet.util.URLBuilder; +import org.ocpsoft.urlbuilder.Address; +import org.ocpsoft.urlbuilder.AddressBuilder; +import org.ocpsoft.urlbuilder.util.Encoder; import com.ocpsoft.pretty.faces.config.rewrite.Redirect; import com.ocpsoft.pretty.faces.config.rewrite.RewriteRule; @@ -88,8 +93,7 @@ public void perform(final Rewrite event, final EvaluationContext context) + QueryString.build(httpRewrite.getInboundAddress().getQuery()).toQueryString(); String contextPath = ((HttpServletRewrite) event).getContextPath(); - if (!contextPath.equals("/") && originalUrl.startsWith(contextPath)) - originalUrl = originalUrl.substring(contextPath.length()); + originalUrl = StringUtils.removePathPrefix(contextPath, originalUrl); String newUrl = engine.processInbound(((HttpServletRewrite) event).getRequest(), ((HttpServletRewrite) event).getResponse(), rule, originalUrl); @@ -110,14 +114,14 @@ public void perform(final Rewrite event, final EvaluationContext context) { /* - * Add context path and encode request using encodeRedirectURL(). + * Add context path. */ redirectURL = contextPath + newUrl; } else if (StringUtils.isNotBlank(rule.getUrl())) { /* - * This is a custom location - don't call encodeRedirectURL() and don't add context path, just + * This is a custom location - don't add context path, just * redirect to the encoded URL */ redirectURL = newUrl.trim(); @@ -126,8 +130,30 @@ else if (StringUtils.isNotBlank(rule.getUrl())) if (redirectURL != null) { - URLBuilder encodedRedirectUrl = URLBuilder.createFrom(redirectURL).encode(); - redirectURL = encodedRedirectUrl.toString(); + try + { + URI uri = new URI(redirectURL); + if (uri.getScheme() == null && uri.getHost() != null) { + Address encodedRedirectAddress + = AddressBuilder.begin() + .scheme(uri.getScheme()) + .domain(uri.getHost()) + .port(uri.getPort()) + .pathEncoded(uri.getPath()) + .queryLiteral(QueryString.build(uri.getQuery()).toQueryString()) + .anchor(uri.getRawFragment()) + .buildLiteral(); + redirectURL = encodedRedirectAddress.toString(); + } + } + catch (URISyntaxException e) + { + throw new IllegalArgumentException( + "[" + redirectURL + "] is not a valid URL fragment. Consider encoding relevant portions of the URL with [" + + Encoder.class + + "], or use the provided builder pattern to specify part encoding.", e); + } + if (Redirect.PERMANENT.equals(rule.getRedirect())) ((HttpInboundServletRewrite) event).redirectPermanent(redirectURL); if (Redirect.TEMPORARY.equals(rule.getRedirect())) diff --git a/integration-faces/src/main/java/org/ocpsoft/rewrite/faces/RewriteViewHandler.java b/integration-faces/src/main/java/org/ocpsoft/rewrite/faces/RewriteViewHandler.java index 5cb67773a..3eba5fdf7 100644 --- a/integration-faces/src/main/java/org/ocpsoft/rewrite/faces/RewriteViewHandler.java +++ b/integration-faces/src/main/java/org/ocpsoft/rewrite/faces/RewriteViewHandler.java @@ -16,6 +16,8 @@ package org.ocpsoft.rewrite.faces; import java.io.IOException; +import java.net.URI; +import java.net.URISyntaxException; import java.util.Collections; import java.util.List; import java.util.Locale; @@ -32,7 +34,9 @@ import org.ocpsoft.common.services.ServiceLoader; import org.ocpsoft.common.util.Iterators; import org.ocpsoft.rewrite.faces.spi.FacesActionUrlProvider; -import org.ocpsoft.rewrite.servlet.util.URLBuilder; +import org.ocpsoft.urlbuilder.Address; +import org.ocpsoft.urlbuilder.AddressBuilder; +import org.ocpsoft.urlbuilder.util.Encoder; /** * @author Lincoln Baxter, III @@ -131,9 +135,37 @@ public String getActionURL(final FacesContext context, final String viewId) String parentActionURL = parent.getActionURL(context, viewId); if (parentActionURL.contains("?")) { - URLBuilder builder = URLBuilder.createFrom(result); - builder.getQueryStringBuilder().addParameters(parentActionURL); - result = builder.toURL(); + try + { + URI uri = new URI(result); + URI action = new URI(parentActionURL); + StringBuilder query = new StringBuilder(uri.getQuery()); + if (query.length() > 0 && action.getQuery() != null) { + query.append('&'); + } + query.append(action.getQuery()); + + if (uri.getScheme() == null && uri.getHost() != null) { + Address address + = AddressBuilder.begin() + .scheme(uri.getScheme()) + .domain(uri.getHost()) + .port(uri.getPort()) + .pathEncoded(uri.getPath()) + .queryLiteral(query.toString()) + .anchor(uri.getRawFragment()) + .buildLiteral(); + + result = address.toString(); + } + } + catch (URISyntaxException e) + { + throw new IllegalArgumentException( + "[" + result + "] is not a valid URL fragment. Consider encoding relevant portions of the URL with [" + + Encoder.class + + "], or use the provided builder pattern to specify part encoding.", e); + } } } } From 07c526e7db960a40b65db8e43a4b4bb9b066382d Mon Sep 17 00:00:00 2001 From: Matt Fletcher Date: Thu, 13 Sep 2018 10:24:00 -0400 Subject: [PATCH 2/9] Fix path encoding. --- .../rewrite/prettyfaces/InboundRewriteRuleAdaptor.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/config-prettyfaces/src/main/java/org/ocpsoft/rewrite/prettyfaces/InboundRewriteRuleAdaptor.java b/config-prettyfaces/src/main/java/org/ocpsoft/rewrite/prettyfaces/InboundRewriteRuleAdaptor.java index 65f194634..1bf98ebd5 100644 --- a/config-prettyfaces/src/main/java/org/ocpsoft/rewrite/prettyfaces/InboundRewriteRuleAdaptor.java +++ b/config-prettyfaces/src/main/java/org/ocpsoft/rewrite/prettyfaces/InboundRewriteRuleAdaptor.java @@ -134,12 +134,19 @@ else if (StringUtils.isNotBlank(rule.getUrl())) { URI uri = new URI(redirectURL); if (uri.getScheme() == null && uri.getHost() != null) { + String path = uri.getPath(); + if (StringUtils.hasLeadingSlash(path)) + { + path = path.substring(1); + } + Address encodedRedirectAddress = AddressBuilder.begin() .scheme(uri.getScheme()) .domain(uri.getHost()) .port(uri.getPort()) - .pathEncoded(uri.getPath()) + .path("/{p}") + .setEncoded("p", path) .queryLiteral(QueryString.build(uri.getQuery()).toQueryString()) .anchor(uri.getRawFragment()) .buildLiteral(); From ad64af09932680c02a8c8abf5b7543456e074632 Mon Sep 17 00:00:00 2001 From: Matt Fletcher Date: Thu, 13 Sep 2018 14:59:51 -0400 Subject: [PATCH 3/9] Encode before parsing as URI If we send an invalid URL to the URI parser, it falls over and fails. But apparently we are supposed to accept and fix rules that produce invalid URLs. So encode URL before we try to parse it. --- .../prettyfaces/InboundRewriteRuleAdaptor.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/config-prettyfaces/src/main/java/org/ocpsoft/rewrite/prettyfaces/InboundRewriteRuleAdaptor.java b/config-prettyfaces/src/main/java/org/ocpsoft/rewrite/prettyfaces/InboundRewriteRuleAdaptor.java index 1bf98ebd5..1fde0cff6 100644 --- a/config-prettyfaces/src/main/java/org/ocpsoft/rewrite/prettyfaces/InboundRewriteRuleAdaptor.java +++ b/config-prettyfaces/src/main/java/org/ocpsoft/rewrite/prettyfaces/InboundRewriteRuleAdaptor.java @@ -130,6 +130,20 @@ else if (StringUtils.isNotBlank(rule.getUrl())) if (redirectURL != null) { + if (redirectURL.contains(" ")) { + String[] parts = redirectURL.split("?", 2); + String[] encodedParts = new String[parts.length]; + encodedParts[0] = Encoder.path(parts[0]); + if (parts.length > 1) { + encodedParts[1] = Encoder.query(parts[1]); + redirectURL = String.join("?", encodedParts); + } + else + { + redirectURL = encodedParts[1]; + } + } + try { URI uri = new URI(redirectURL); From 0cd3eda6c2aab4792438bf57d7b49ba300c7d78a Mon Sep 17 00:00:00 2001 From: Matt Fletcher Date: Thu, 13 Sep 2018 15:08:48 -0400 Subject: [PATCH 4/9] Join manually before Java8 String.join did not exist prior to Java 8. So implement manually with StringBuilder. --- .../rewrite/prettyfaces/InboundRewriteRuleAdaptor.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/config-prettyfaces/src/main/java/org/ocpsoft/rewrite/prettyfaces/InboundRewriteRuleAdaptor.java b/config-prettyfaces/src/main/java/org/ocpsoft/rewrite/prettyfaces/InboundRewriteRuleAdaptor.java index 1fde0cff6..58128222b 100644 --- a/config-prettyfaces/src/main/java/org/ocpsoft/rewrite/prettyfaces/InboundRewriteRuleAdaptor.java +++ b/config-prettyfaces/src/main/java/org/ocpsoft/rewrite/prettyfaces/InboundRewriteRuleAdaptor.java @@ -136,7 +136,11 @@ else if (StringUtils.isNotBlank(rule.getUrl())) encodedParts[0] = Encoder.path(parts[0]); if (parts.length > 1) { encodedParts[1] = Encoder.query(parts[1]); - redirectURL = String.join("?", encodedParts); + + StringBuilder joiner = new StringBuilder(encodedParts[0]); + joiner.append("?").append(encodedParts[1]); + + redirectURL = joiner.toString(); } else { From 647296a03dd68bc993198dfd4a22acd2d69ddd4e Mon Sep 17 00:00:00 2001 From: Matt Fletcher Date: Thu, 13 Sep 2018 15:13:08 -0400 Subject: [PATCH 5/9] Fix incorrect index --- .../ocpsoft/rewrite/prettyfaces/InboundRewriteRuleAdaptor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config-prettyfaces/src/main/java/org/ocpsoft/rewrite/prettyfaces/InboundRewriteRuleAdaptor.java b/config-prettyfaces/src/main/java/org/ocpsoft/rewrite/prettyfaces/InboundRewriteRuleAdaptor.java index 58128222b..5cbd795ac 100644 --- a/config-prettyfaces/src/main/java/org/ocpsoft/rewrite/prettyfaces/InboundRewriteRuleAdaptor.java +++ b/config-prettyfaces/src/main/java/org/ocpsoft/rewrite/prettyfaces/InboundRewriteRuleAdaptor.java @@ -144,7 +144,7 @@ else if (StringUtils.isNotBlank(rule.getUrl())) } else { - redirectURL = encodedParts[1]; + redirectURL = encodedParts[0]; } } From 216db82fc2429c1292f3f38116a890c9dff0f806 Mon Sep 17 00:00:00 2001 From: Matt Fletcher Date: Thu, 13 Sep 2018 16:03:42 -0400 Subject: [PATCH 6/9] Escape the question mark Use Pattern.quote to escape the question mark and use it literally. --- .../rewrite/prettyfaces/InboundRewriteRuleAdaptor.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/config-prettyfaces/src/main/java/org/ocpsoft/rewrite/prettyfaces/InboundRewriteRuleAdaptor.java b/config-prettyfaces/src/main/java/org/ocpsoft/rewrite/prettyfaces/InboundRewriteRuleAdaptor.java index 5cbd795ac..3be89e9f7 100644 --- a/config-prettyfaces/src/main/java/org/ocpsoft/rewrite/prettyfaces/InboundRewriteRuleAdaptor.java +++ b/config-prettyfaces/src/main/java/org/ocpsoft/rewrite/prettyfaces/InboundRewriteRuleAdaptor.java @@ -23,6 +23,7 @@ import java.net.URI; import java.net.URISyntaxException; +import java.util.regex.Pattern; import org.ocpsoft.rewrite.config.Rule; import org.ocpsoft.rewrite.context.EvaluationContext; @@ -131,9 +132,10 @@ else if (StringUtils.isNotBlank(rule.getUrl())) if (redirectURL != null) { if (redirectURL.contains(" ")) { - String[] parts = redirectURL.split("?", 2); + String[] parts = redirectURL.split(Pattern.quote("?"), 2); String[] encodedParts = new String[parts.length]; encodedParts[0] = Encoder.path(parts[0]); + if (parts.length > 1) { encodedParts[1] = Encoder.query(parts[1]); From ee978fd0f8f0aaa902f82a7b4dfb817aefdd7c5c Mon Sep 17 00:00:00 2001 From: Matt Fletcher Date: Thu, 13 Sep 2018 16:05:07 -0400 Subject: [PATCH 7/9] Make clearer what it is doing Since both pathEncoded and pathDecoded decode the string, use pathDecoded so people will know what's happening. --- .../main/java/org/ocpsoft/rewrite/faces/RewriteViewHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration-faces/src/main/java/org/ocpsoft/rewrite/faces/RewriteViewHandler.java b/integration-faces/src/main/java/org/ocpsoft/rewrite/faces/RewriteViewHandler.java index 3eba5fdf7..18f835447 100644 --- a/integration-faces/src/main/java/org/ocpsoft/rewrite/faces/RewriteViewHandler.java +++ b/integration-faces/src/main/java/org/ocpsoft/rewrite/faces/RewriteViewHandler.java @@ -151,7 +151,7 @@ public String getActionURL(final FacesContext context, final String viewId) .scheme(uri.getScheme()) .domain(uri.getHost()) .port(uri.getPort()) - .pathEncoded(uri.getPath()) + .pathDecoded(uri.getPath()) .queryLiteral(query.toString()) .anchor(uri.getRawFragment()) .buildLiteral(); From af17ced07b1308810c184f107e3b75f2485a322b Mon Sep 17 00:00:00 2001 From: Matt Fletcher Date: Sun, 16 Sep 2018 11:47:24 -0400 Subject: [PATCH 8/9] Add message to assertion As this test is not failing locally and not producing useful information on Travis, adding a message to the failing assertion. --- .../ocpsoft/rewrite/prettyfaces/encoding/URLEncodingTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/config-prettyfaces-tests/src/test/java/org/ocpsoft/rewrite/prettyfaces/encoding/URLEncodingTest.java b/config-prettyfaces-tests/src/test/java/org/ocpsoft/rewrite/prettyfaces/encoding/URLEncodingTest.java index 80eb85741..7b3190016 100644 --- a/config-prettyfaces-tests/src/test/java/org/ocpsoft/rewrite/prettyfaces/encoding/URLEncodingTest.java +++ b/config-prettyfaces-tests/src/test/java/org/ocpsoft/rewrite/prettyfaces/encoding/URLEncodingTest.java @@ -72,7 +72,8 @@ public void testRewriteEncodingUrl() throws Exception HttpAction action = get(target); - Assert.assertTrue(action.getCurrentURL().endsWith(expected)); + Assert.assertTrue(action.getCurrentURL() + " should end with " + expected, + action.getCurrentURL().endsWith(expected)); Assert.assertTrue(action.getResponseContent().contains(expected)); } From 6f66abfb5ac7e320d4847929d7c5073ff6ed9ecf Mon Sep 17 00:00:00 2001 From: Matt Fletcher Date: Tue, 18 Sep 2018 15:40:03 -0400 Subject: [PATCH 9/9] Remove duplicate casts We cast event to HttpServletRewrite before getting the inbound address. We do not need to keep doing it, as we have the cast version stored. --- .../rewrite/prettyfaces/InboundRewriteRuleAdaptor.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/config-prettyfaces/src/main/java/org/ocpsoft/rewrite/prettyfaces/InboundRewriteRuleAdaptor.java b/config-prettyfaces/src/main/java/org/ocpsoft/rewrite/prettyfaces/InboundRewriteRuleAdaptor.java index 3be89e9f7..5b4062647 100644 --- a/config-prettyfaces/src/main/java/org/ocpsoft/rewrite/prettyfaces/InboundRewriteRuleAdaptor.java +++ b/config-prettyfaces/src/main/java/org/ocpsoft/rewrite/prettyfaces/InboundRewriteRuleAdaptor.java @@ -93,11 +93,11 @@ public void perform(final Rewrite event, final EvaluationContext context) originalUrl = URL.build(originalUrl).decode().toURL() + QueryString.build(httpRewrite.getInboundAddress().getQuery()).toQueryString(); - String contextPath = ((HttpServletRewrite) event).getContextPath(); + String contextPath = httpRewrite.getContextPath(); originalUrl = StringUtils.removePathPrefix(contextPath, originalUrl); - String newUrl = engine.processInbound(((HttpServletRewrite) event).getRequest(), - ((HttpServletRewrite) event).getResponse(), rule, originalUrl); + String newUrl = engine.processInbound(httpRewrite.getRequest(), + httpRewrite.getResponse(), rule, originalUrl); if (!Redirect.CHAIN.equals(rule.getRedirect())) {