This is an automated email from the ASF dual-hosted git repository. ghenzler pushed a commit to branch feature/SLING-9777-SlingUri-better-encoding-handling in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-api.git
commit f54999cb4c7313cdc871dd8b7788b3a69b0dce2b Author: georg.henzler <[email protected]> AuthorDate: Wed Sep 30 20:36:17 2020 +0200 SLING-9777 Better handling for invalid URIs when using SlingUri (special characters, spaces) --- .../java/org/apache/sling/api/uri/SlingUri.java | 7 +- .../org/apache/sling/api/uri/SlingUriBuilder.java | 358 +++++++++++++++++---- .../apache/sling/api/uri/SlingUriBuilderTest.java | 38 +++ .../apache/sling/api/uri/SlingUriEncodingTest.java | 109 +++++++ .../sling/api/uri/SlingUriInvalidUrisTest.java | 37 ++- .../org/apache/sling/api/uri/SlingUriTest.java | 60 +++- 6 files changed, 518 insertions(+), 91 deletions(-) diff --git a/src/main/java/org/apache/sling/api/uri/SlingUri.java b/src/main/java/org/apache/sling/api/uri/SlingUri.java index 2ad757e..2a83417 100644 --- a/src/main/java/org/apache/sling/api/uri/SlingUri.java +++ b/src/main/java/org/apache/sling/api/uri/SlingUri.java @@ -29,9 +29,14 @@ import org.jetbrains.annotations.Nullable; import org.osgi.annotation.versioning.ProviderType; /** + * <p> * Represents an immutable URI in the same way as java.net.URI but is extended with Sling-specific URI parts (e.g. selectors). A SlingUri * usually points to a resource but alternatively, can also contain an opaque URI like {@code mailto:} or {@code javascript:}. Use * {@link SlingUri#adjust(Consumer)} or {@link SlingUriBuilder} to create new or modified Sling URIs. + * </p> + * <p> + * Opposed to {@link URI}, {@code SlingUri} does not make any assumptions regarding URI encoding, a SlingUri can hold URL-encoded or + * non-URL-encoded values for user info, path, query and fragment. * * @since 1.0.0 (Sling API Bundle 2.23.0) */ @@ -97,7 +102,7 @@ public interface SlingUri extends RequestPathInfo { /** * Returns the selector string. * - * @return returns the selector string or null if the URI does not contain selector(s) + * @return returns the selector string or null if the URI does not contain selector(s) (in line with {@link RequestPathInfo}) */ @Override @Nullable diff --git a/src/main/java/org/apache/sling/api/uri/SlingUriBuilder.java b/src/main/java/org/apache/sling/api/uri/SlingUriBuilder.java index 7088672..5522800 100644 --- a/src/main/java/org/apache/sling/api/uri/SlingUriBuilder.java +++ b/src/main/java/org/apache/sling/api/uri/SlingUriBuilder.java @@ -18,8 +18,11 @@ */ package org.apache.sling.api.uri; +import java.io.UnsupportedEncodingException; import java.net.URI; import java.net.URISyntaxException; +import java.net.URLEncoder; +import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.Collections; import java.util.Iterator; @@ -68,8 +71,9 @@ public class SlingUriBuilder { private static final String HTTP_SCHEME = "http"; private static final int HTTP_DEFAULT_PORT = 80; - static final char CHAR_HASH = '#'; - static final char CHAR_QM = '?'; + static final String CHAR_HASH = "#"; + static final String CHAR_QM = "?"; + static final char CHAR_AMP = '&'; static final char CHAR_AT = '@'; static final char CHAR_SEMICOLON = ';'; static final char CHAR_EQUALS = '='; @@ -79,6 +83,7 @@ public class SlingUriBuilder { static final String CHAR_SLASH = "/"; static final String SELECTOR_DOT_REGEX = "\\.(?!\\.?/)"; // (?!\\.?/) to avoid matching ./ and ../ static final String PATH_PARAMETERS_REGEX = ";([a-zA-z0-9]+)=(?:\\'([^']*)\\'|([^/]+))"; + static final String BEST_EFFORT_INVALID_URI_MATCHER = "^(?:([^:#@]+):)?(?://(?:([^@#]+)@)?([^/#:]+)(?::([0-9]+))?)?(?:([^?#]+))?(?:\\?([^#]*))?(?:#(.*))?$"; /** * Creates a builder without any URI parameters set. @@ -112,7 +117,7 @@ public class SlingUriBuilder { .setFragment(slingUri.getFragment()) .setSchemeSpecificPart(slingUri.isOpaque() ? slingUri.getSchemeSpecificPart() : null) .setResourceResolver(slingUri instanceof ImmutableSlingUri - ? ((ImmutableSlingUri) slingUri).getBuilder().resourceResolver + ? ((ImmutableSlingUri) slingUri).getData().resourceResolver : null); } @@ -172,19 +177,20 @@ public class SlingUriBuilder { */ @NotNull public static SlingUriBuilder createFrom(@NotNull URI uri, @Nullable ResourceResolver resourceResolver) { - String path = uri.getPath(); + String path = uri.getRawPath(); boolean pathExists = isNotBlank(path); - boolean schemeSpecificRelevant = !pathExists && uri.getQuery() == null; + String uriQuery = uri.getRawQuery(); + boolean schemeSpecificRelevant = !pathExists && uriQuery == null; return create() .setResourceResolver(resourceResolver) .setScheme(uri.getScheme()) - .setUserInfo(uri.getUserInfo()) + .setUserInfo(uri.getRawUserInfo()) .setHost(uri.getHost()) .setPort(uri.getPort()) .setPath(pathExists ? path : null) - .setQuery(uri.getQuery()) - .setFragment(uri.getFragment()) - .setSchemeSpecificPart(schemeSpecificRelevant ? uri.getSchemeSpecificPart() : null); + .setQuery(uriQuery) + .setFragment(uri.getRawFragment()) + .setSchemeSpecificPart(schemeSpecificRelevant ? uri.getRawSchemeSpecificPart() : null); } /** @@ -203,14 +209,34 @@ public class SlingUriBuilder { return createFrom(uri, resourceResolver); } catch (URISyntaxException e) { LOG.debug("Invalid URI {}: {}", uriStr, e.getMessage(), e); - // best effort - String[] invalidUriParts = uriStr.split(CHAR_COLON, 2); - if (invalidUriParts.length == 1) { - return create().setSchemeSpecificPart(invalidUriParts[0]); - } else { + // best effort to match input, see SlingUriInvalidUrisTest + Matcher matcher = Pattern.compile(BEST_EFFORT_INVALID_URI_MATCHER).matcher(uriStr); + matcher.find(); + + String scheme = matcher.groupCount() >= 1 ? matcher.group(1) : null; + String userInfo = matcher.groupCount() >= 2 ? matcher.group(2) : null; + String host = matcher.groupCount() >= 3 ? matcher.group(3) : null; + String port = matcher.groupCount() >= 4 ? matcher.group(4) : null; + String path = matcher.groupCount() >= 5 ? matcher.group(5) : null; + String query = matcher.groupCount() >= 6 ? matcher.group(6) : null; + String fragment = matcher.groupCount() >= 7 ? matcher.group(7) : null; + if (!isBlank(scheme) && isBlank(host)) { + // opaque case + return create() + .setScheme(scheme) + .setSchemeSpecificPart(path) + .setFragment(fragment); + } else if (!isBlank(host) || !isBlank(path)) { return create() - .setScheme(invalidUriParts[0]) - .setSchemeSpecificPart(invalidUriParts[1]); + .setScheme(scheme) + .setUserInfo(userInfo) + .setHost(host) + .setPort(port != null ? Integer.parseInt(port) : -1) + .setPath(path) + .setQuery(query) + .setFragment(fragment); + } else { + return create().setSchemeSpecificPart(uriStr); } } } @@ -495,16 +521,58 @@ public class SlingUriBuilder { } /** - * Set the fragment of the URI. + * Add a query parameter to the query of the URI. Key and value are URL-encoded before adding the parameter to the query string. * - * @param fragment the fragment + * @param parameterName the parameter name + * @param value the parameter value * @return the builder for method chaining */ @NotNull - public SlingUriBuilder setFragment(@Nullable String fragment) { + public SlingUriBuilder addQueryParameter(@NotNull String parameterName, @NotNull String value) { if (schemeSpecificPart != null) { return this; } + try { + this.query = (this.query == null ? "" : this.query + CHAR_AMP) + + URLEncoder.encode(parameterName, StandardCharsets.UTF_8.name()) + + "=" + URLEncoder.encode(value, StandardCharsets.UTF_8.name()); + } catch (UnsupportedEncodingException e) { + throw new IllegalStateException("Encoding not supported: " + StandardCharsets.UTF_8, e); + } + return this; + } + + /** + * <p> + * Replace all query parameters of the URL. Both keys and values are URL-encoded before adding them to the query string. + * </p> + * <p> + * For adding multiple query parameters with the same name prefer to use {@link #addQueryParameter(String, String)}. + * </p> + * + * @param queryParameters the map with the query parameters + * @return the builder for method chaining + */ + @NotNull + public SlingUriBuilder setQueryParameters(@NotNull Map<String, String> queryParameters) { + if (schemeSpecificPart != null) { + return this; + } + setQuery(null); // reset first + for (Map.Entry<String, String> parameter : queryParameters.entrySet()) { + addQueryParameter(parameter.getKey(), parameter.getValue()); + } + return this; + } + + /** + * Set the fragment of the URI. + * + * @param fragment the fragment + * @return the builder for method chaining + */ + @NotNull + public SlingUriBuilder setFragment(@Nullable String fragment) { this.fragment = fragment; return this; } @@ -563,6 +631,168 @@ public class SlingUriBuilder { } /** + * Returns the resource path. + * + * @return returns the resource path or null if the URI does not contain a path. + */ + @Nullable + public String getResourcePath() { + return resourcePath; + } + + /** + * Returns the selector string + * + * @return returns the selector string or null if the URI does not contain selector(s) (in line with {@link RequestPathInfo}) + */ + @Nullable + public String getSelectorString() { + return !selectors.isEmpty() ? String.join(CHAR_DOT, selectors) : null; + } + + /** + * Returns the selectors array. + * + * @return the selectors array (empty if the URI does not contain selector(s), never null) + */ + @NotNull + public String[] getSelectors() { + return selectors.toArray(new String[selectors.size()]); + } + + /** + * Returns the extension. + * + * @return the extension or null if the URI does not contain an extension + */ + @Nullable + public String getExtension() { + return extension; + } + + /** + * Returns the path parameters. + * + * @return the path parameters or an empty Map if the URI does not contain any + */ + @Nullable + public Map<String, String> getPathParameters() { + return pathParameters; + } + + /** + * Returns the suffix part of the URI + * + * @return the suffix string or null if the URI does not contain a suffix + */ + @Nullable + public String getSuffix() { + return suffix; + } + + /** + * Returns the corresponding suffix resource or null if + * <ul> + * <li>no resource resolver is available (depends on the create method used in SlingUriBuilder)</li> + * <li>the URI does not contain a suffix</li> + * <li>if the suffix resource could not be found</li> + * </ul> + * + * @return the suffix resource if available or null + */ + @Nullable + public Resource getSuffixResource() { + if (isNotBlank(suffix) && resourceResolver != null) { + return resourceResolver.resolve(suffix); + } else { + return null; + } + } + + /** + * Returns the joint path of resource path, selectors, extension and suffix. + * + * @return the path or null if no path is set + */ + @Nullable + public String getPath() { + return assemblePath(true); + } + + /** + * Returns the scheme-specific part of the URI, compare with Javadoc of {@link URI}. + * + * @return scheme specific part of the URI + */ + @Nullable + public String getSchemeSpecificPart() { + if (isOpaque()) { + return schemeSpecificPart; + } else { + return toStringInternal(false, false); + } + } + + /** + * Returns the query. + * + * @return the query part of the URI or null if the URI does not contain a query + */ + @Nullable + public String getQuery() { + return query; + } + + /** + * Returns the fragment. + * + * @return the fragment or null if the URI does not contain a fragment + */ + @Nullable + public String getFragment() { + return fragment; + } + + /** + * Returns the scheme. + * + * @return the scheme or null if not set + */ + @Nullable + public String getScheme() { + return scheme; + } + + /** + * Returns the host. + * + * @return returns the host of the SlingUri or null if not set + */ + @Nullable + public String getHost() { + return host; + } + + /** + * Returns the port. + * + * @return returns the port of the SlingUri or -1 if not set + */ + public int getPort() { + return port; + } + + /** + * Returns the user info. + * + * @return the user info of the SlingUri or null if not set + */ + @Nullable + public String getUserInfo() { + return userInfo; + } + + /** * Will take over scheme and authority (user info, host and port) from provided URI. * * @param uri the URI @@ -770,36 +1000,34 @@ public class SlingUriBuilder { return resourcePath; } - // returns null in line with - // https://sling.apache.org/apidocs/sling11/org/apache/sling/api/request/RequestPathInfo.html#getSelectorString-- @Override public String getSelectorString() { - return !selectors.isEmpty() ? String.join(CHAR_DOT, selectors) : null; + return getData().getSelectorString(); } @Override public String[] getSelectors() { - return selectors.toArray(new String[selectors.size()]); + return getData().getSelectors(); } @Override public String getExtension() { - return extension; + return getData().getExtension(); } @Override public Map<String, String> getPathParameters() { - return Collections.unmodifiableMap(pathParameters); + return Collections.unmodifiableMap(getData().getPathParameters()); } @Override public String getSuffix() { - return suffix; + return getData().getSuffix(); } @Override public String getPath() { - return assemblePath(true); + return getData().getPath(); } @Override @@ -813,66 +1041,62 @@ public class SlingUriBuilder { @Override public String getQuery() { - return query; + return getData().getQuery(); } @Override public String getFragment() { - return fragment; + return getData().getFragment(); } @Override public String getScheme() { - return scheme; + return getData().getScheme(); } @Override public String getHost() { - return host; + return getData().getHost(); } @Override public int getPort() { - return port; + return getData().getPort(); } @Override public Resource getSuffixResource() { - if (isNotBlank(suffix) && resourceResolver != null) { - return resourceResolver.resolve(suffix); - } else { - return null; - } + return getData().getSuffixResource(); } @Override public String getUserInfo() { - return userInfo; + return getData().getUserInfo(); } @Override public boolean isOpaque() { - return getBuilder().isOpaque(); + return getData().isOpaque(); } @Override public boolean isPath() { - return getBuilder().isPath(); + return getData().isPath(); } @Override public boolean isAbsolutePath() { - return getBuilder().isAbsolutePath(); + return getData().isAbsolutePath(); } @Override public boolean isRelativePath() { - return getBuilder().isRelativePath(); + return getData().isRelativePath(); } @Override public boolean isAbsolute() { - return getBuilder().isAbsolute(); + return getData().isAbsolute(); } @Override @@ -890,7 +1114,7 @@ public class SlingUriBuilder { } } - private SlingUriBuilder getBuilder() { + private SlingUriBuilder getData() { return SlingUriBuilder.this; } @@ -924,61 +1148,61 @@ public class SlingUriBuilder { return false; ImmutableSlingUri other = (ImmutableSlingUri) obj; if (extension == null) { - if (other.getBuilder().extension != null) + if (other.getData().extension != null) return false; - } else if (!extension.equals(other.getBuilder().extension)) + } else if (!extension.equals(other.getData().extension)) return false; if (fragment == null) { - if (other.getBuilder().fragment != null) + if (other.getData().fragment != null) return false; - } else if (!fragment.equals(other.getBuilder().fragment)) + } else if (!fragment.equals(other.getData().fragment)) return false; if (host == null) { - if (other.getBuilder().host != null) + if (other.getData().host != null) return false; - } else if (!host.equals(other.getBuilder().host)) + } else if (!host.equals(other.getData().host)) return false; if (pathParameters == null) { - if (other.getBuilder().pathParameters != null) + if (other.getData().pathParameters != null) return false; - } else if (!pathParameters.equals(other.getBuilder().pathParameters)) + } else if (!pathParameters.equals(other.getData().pathParameters)) return false; - if (port != other.getBuilder().port) + if (port != other.getData().port) return false; if (query == null) { - if (other.getBuilder().query != null) + if (other.getData().query != null) return false; - } else if (!query.equals(other.getBuilder().query)) + } else if (!query.equals(other.getData().query)) return false; if (resourcePath == null) { - if (other.getBuilder().resourcePath != null) + if (other.getData().resourcePath != null) return false; - } else if (!resourcePath.equals(other.getBuilder().resourcePath)) + } else if (!resourcePath.equals(other.getData().resourcePath)) return false; if (scheme == null) { - if (other.getBuilder().scheme != null) + if (other.getData().scheme != null) return false; - } else if (!scheme.equals(other.getBuilder().scheme)) + } else if (!scheme.equals(other.getData().scheme)) return false; if (schemeSpecificPart == null) { - if (other.getBuilder().schemeSpecificPart != null) + if (other.getData().schemeSpecificPart != null) return false; - } else if (!schemeSpecificPart.equals(other.getBuilder().schemeSpecificPart)) + } else if (!schemeSpecificPart.equals(other.getData().schemeSpecificPart)) return false; if (selectors == null) { - if (other.getBuilder().selectors != null) + if (other.getData().selectors != null) return false; - } else if (!selectors.equals(other.getBuilder().selectors)) + } else if (!selectors.equals(other.getData().selectors)) return false; if (suffix == null) { - if (other.getBuilder().suffix != null) + if (other.getData().suffix != null) return false; - } else if (!suffix.equals(other.getBuilder().suffix)) + } else if (!suffix.equals(other.getData().suffix)) return false; if (userInfo == null) { - if (other.getBuilder().userInfo != null) + if (other.getData().userInfo != null) return false; - } else if (!userInfo.equals(other.getBuilder().userInfo)) + } else if (!userInfo.equals(other.getData().userInfo)) return false; return true; } diff --git a/src/test/java/org/apache/sling/api/uri/SlingUriBuilderTest.java b/src/test/java/org/apache/sling/api/uri/SlingUriBuilderTest.java index 0f33031..5a59045 100644 --- a/src/test/java/org/apache/sling/api/uri/SlingUriBuilderTest.java +++ b/src/test/java/org/apache/sling/api/uri/SlingUriBuilderTest.java @@ -25,6 +25,8 @@ import static org.mockito.Mockito.when; import java.net.URI; import java.net.URISyntaxException; +import java.util.LinkedHashMap; +import java.util.Map; import org.apache.sling.api.SlingHttpServletRequest; import org.apache.sling.api.request.RequestPathInfo; @@ -150,6 +152,42 @@ public class SlingUriBuilderTest { assertEquals("https://example.com:8080/path/to/page.html";, testUri.toString()); } + @Test + public void testAddQueryParameter() throws URISyntaxException { + String testPath = "/path/to/page.html"; + SlingUri testUri = SlingUriBuilder.parse(testPath, null) + .addQueryParameter("param1", "val1") + .build(); + assertEquals("/path/to/page.html?param1=val1", testUri.toString()); + } + + @Test + public void testAddQueryParameterValueEncoded() throws URISyntaxException { + String testPath = "/path/to/page.html"; + SlingUri testUri = SlingUriBuilder.parse(testPath, null) + .addQueryParameter("redirect", "http://www.example.com/path/to/file.txt?q=3&test=3#test";) + .addQueryParameter("param2", "true") + .build(); + assertEquals( + "/path/to/page.html?redirect=http%3A%2F%2Fwww.example.com%2Fpath%2Fto%2Ffile.txt%3Fq%3D3%26test%3D3%23test¶m2=true", + testUri.toString()); + } + + @Test + public void testSetQueryParameterValueEncoded() throws URISyntaxException { + String testPath = "/path/to/page.html?param=value"; // existing query parameters are meant to be replaced + + Map<String, String> queryParams = new LinkedHashMap<>(); + queryParams.put("param1", "value1"); + queryParams.put("param2[*]", "value2"); + queryParams.put("param3", "value3%@"); + + SlingUri testUri = SlingUriBuilder.parse(testPath, null) + .setQueryParameters(queryParams) + .build(); + assertEquals("/path/to/page.html?param1=value1¶m2%5B*%5D=value2¶m3=value3%25%40", testUri.toString()); + } + @Test(expected = IllegalStateException.class) public void testBuilderMayOnlyBeUsedToBuildAnUri() { SlingUriBuilder builder = SlingUriBuilder.parse("/path/to/page.html", null); diff --git a/src/test/java/org/apache/sling/api/uri/SlingUriEncodingTest.java b/src/test/java/org/apache/sling/api/uri/SlingUriEncodingTest.java new file mode 100644 index 0000000..a5c1add --- /dev/null +++ b/src/test/java/org/apache/sling/api/uri/SlingUriEncodingTest.java @@ -0,0 +1,109 @@ +/* + * 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. + */ +package org.apache.sling.api.uri; + +import static org.junit.Assert.assertEquals; + +import java.io.UnsupportedEncodingException; +import java.net.URISyntaxException; +import java.net.URLDecoder; +import java.nio.charset.StandardCharsets; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.junit.MockitoJUnitRunner; + +@RunWith(MockitoJUnitRunner.class) +public class SlingUriEncodingTest { + + @Test + public void testUriWithEuroSign() throws URISyntaxException, UnsupportedEncodingException { + + testUriUnchangedForEncodedAndDecodedStr("/test-with-euro-sign-%E2%82%AC-suffix.pdf", true, true, false, false, false); + } + + @Test + public void testUriWithSpaces() throws URISyntaxException, UnsupportedEncodingException { + + testUriUnchangedForEncodedAndDecodedStr("/test+with+spaces%20in+different%20encodings", true, true, false, false, false); + } + + @Test + public void testUriWithSpecialCharacters() throws URISyntaxException, UnsupportedEncodingException { + + testUriUnchangedForEncodedAndDecodedStr( + "/path/with/many/special/chars/%2B%26%2B-%25%3D%2A%5E%3F%23%5E%21%24%3D%3D%5E_-_%24%2B%24%3D%5E%3F%3F%2B%26%40%3D%25%40-%24", + true, true, false, false, false); + } + + @Test + public void testUriWithSpecialCharactersInUserInfo() throws URISyntaxException, UnsupportedEncodingException { + + testUriUnchangedForEncodedAndDecodedStr( + "http://user:%2b%26%2b-%25%3d%2a%5e%[email protected]/path.txt";, + false, false, false, true, false); + } + + @Test + public void testUriWithSpecialCharactersInQuery() throws URISyntaxException, UnsupportedEncodingException { + + testUriUnchangedForEncodedAndDecodedStr( + "http://example.com/path.txt?testParam=%2B%26%2B-%25%3D%2A%5E%3F%23";, + false, false, false, true, false); + } + + @Test + public void testUriWithSpecialCharactersInFragment() throws URISyntaxException, UnsupportedEncodingException { + + testUriUnchangedForEncodedAndDecodedStr( + "http://example.com/path.txt?testParam=testVal#%2B%26%2B-%25%3D%2A%5E%3F%23";, + false, false, false, true, false); + } + + private void testUriUnchangedForEncodedAndDecodedStr(String testUriStrEncoded, boolean isPath, boolean isAbsolutePath, + boolean isRelativePath, boolean isAbsolute, boolean isOpaque) throws UnsupportedEncodingException { + + testUriUnchanged(testUriStrEncoded, isPath, isAbsolutePath, isRelativePath, isAbsolute, isOpaque); + + // decoded variant should also stay unchanged + String testUriStrDecoded = URLDecoder.decode(testUriStrEncoded, StandardCharsets.UTF_8.name()); + testUriUnchanged(testUriStrDecoded, isPath, isAbsolutePath, isRelativePath, isAbsolute, isOpaque); + } + + public static SlingUri testUriUnchanged(String testUri, boolean isPath, boolean isAbsolutePath, boolean isRelativePath, + boolean isAbsolute, boolean isOpaque) { + SlingUri slingUri = SlingUriBuilder.parse(testUri, null).build(); + + assertEquals("Uri toString() same as input", testUri, slingUri.toString()); + + assertEquals("isPath()", isPath, slingUri.isPath()); + assertEquals("isAbsolutePath()", isAbsolutePath, slingUri.isAbsolutePath()); + assertEquals("isRelativePath()", isRelativePath, slingUri.isRelativePath()); + assertEquals("isAbsolute()", isAbsolute, slingUri.isAbsolute()); + assertEquals("isOpaque()", isOpaque, slingUri.isOpaque()); + + SlingUri slingUriParsedFromSameInput = SlingUriBuilder.parse(testUri, null).build(); + assertEquals("uris parsed from same input are expected to be equal", slingUriParsedFromSameInput, slingUri); + assertEquals("uris parsed from same input are expected to have the same hash code", slingUriParsedFromSameInput.hashCode(), + slingUri.hashCode()); + + return slingUri; + } + +} diff --git a/src/test/java/org/apache/sling/api/uri/SlingUriInvalidUrisTest.java b/src/test/java/org/apache/sling/api/uri/SlingUriInvalidUrisTest.java index 48a9aff..7d6fb33 100644 --- a/src/test/java/org/apache/sling/api/uri/SlingUriInvalidUrisTest.java +++ b/src/test/java/org/apache/sling/api/uri/SlingUriInvalidUrisTest.java @@ -19,7 +19,6 @@ package org.apache.sling.api.uri; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNull; import static org.junit.Assert.fail; import java.net.URI; @@ -36,14 +35,28 @@ import org.junit.runners.Parameterized.Parameters; public class SlingUriInvalidUrisTest { @Parameters(name = "Invalid URI: {0}") - public static Collection<String> data() { - return Arrays.asList(":foo", "https://";, "https:", "@:", "://", "::::"); + public static Collection<String[]> data() { + return Arrays.asList( + // test fix URIs with spaces + new String[] { "/path/with/spaces/A name with spaces.pdf", "/test.pdf" }, + new String[] { "http://example.com/path/with/spaces/A name with spaces.pdf", "http://example.com/test.pdf"; }, + new String[] { "http://user:[email protected]/path/with/spaces/A name with spaces.sel1.pdf/suffix?par1=val1&par2=val2#frag", + "http://user:[email protected]/test.sel1.pdf/suffix?par1=val1&par2=val2#frag"; }, + // short invalid URIs + new String[] { "https://";, "https://"; }, + new String[] { "special:", "special:/test" }, + new String[] { "@:", "/test" }, + new String[] { ":foo", "/test" }, + new String[] { "://", "/test" }, + new String[] { "::::", "/test" }); } private final String invalidUri; + private final String invalidUriAdjustedAfterSetPath; - public SlingUriInvalidUrisTest(String invalidUri) { + public SlingUriInvalidUrisTest(String invalidUri, String invalidUriAdjustedAfterSetPath) { this.invalidUri = invalidUri; + this.invalidUriAdjustedAfterSetPath = invalidUriAdjustedAfterSetPath; } @Test @@ -59,20 +72,22 @@ public class SlingUriInvalidUrisTest { } @Test - public void testAdjustInvalidUriNoEffect() { + public void testAdjustInvalidUriToValidUri() { SlingUri slingUri = SlingUriBuilder.parse(invalidUri, null).build(); - SlingUri slingUriAdjusted = slingUri.adjust(b -> b.setResourcePath("/test")); - assertNull("setResourcePath() should have been ignored for uri " + invalidUri, slingUriAdjusted.getResourcePath()); + SlingUri slingUriAdjusted = slingUri.adjust(b -> b.setSchemeSpecificPart(null).setResourcePath("/test")); + assertEquals("Using setSchemeSpecificPart(null) should reset the invalid URI to be adjustable", "/test", + slingUriAdjusted.getResourcePath()); } @Test - public void testAdjustInvalidUriToValidUri() { + public void testAdjustInvalidUri() { SlingUri slingUri = SlingUriBuilder.parse(invalidUri, null).build(); - SlingUri slingUriAdjusted = slingUri.adjust(b -> b.setSchemeSpecificPart(null).setResourcePath("/test")); - assertEquals("Using setSchemeSpecificPart(null) should reset the invalid URI to be adjustable", "/test", - slingUriAdjusted.getResourcePath()); + SlingUri slingUriAdjusted = slingUri.adjust(b -> b.setResourcePath("/test")); + assertEquals("setPath('/test') to invalid URI '" + invalidUri + "' should result in: " + + invalidUriAdjustedAfterSetPath, invalidUriAdjustedAfterSetPath, slingUriAdjusted.toString()); } + } diff --git a/src/test/java/org/apache/sling/api/uri/SlingUriTest.java b/src/test/java/org/apache/sling/api/uri/SlingUriTest.java index 3e11d9a..8d7a84a 100644 --- a/src/test/java/org/apache/sling/api/uri/SlingUriTest.java +++ b/src/test/java/org/apache/sling/api/uri/SlingUriTest.java @@ -26,6 +26,7 @@ import java.net.URI; import java.util.List; import java.util.function.Consumer; +import org.apache.commons.lang3.StringUtils; import org.apache.sling.api.resource.ResourceResolver; import org.junit.Test; import org.junit.runner.RunWith; @@ -313,9 +314,7 @@ public class SlingUriTest { @Test public void testJavascriptUri() { - String testUriStr = "javascript:void(0)"; - - testUri(testUriStr, false, false, false, true, true, slingUri -> { + testUri("javascript:void(0)", false, false, false, true, true, slingUri -> { assertEquals("javascript", slingUri.getScheme()); assertEquals(null, slingUri.getUserInfo()); assertEquals(null, slingUri.getHost()); @@ -330,6 +329,24 @@ public class SlingUriTest { } @Test + public void testJavascriptUriLong() { + + testUri("javascript:console.log('long js'); return true;", false, false, false, true, true, slingUri -> { + assertEquals("javascript", slingUri.getScheme()); + assertEquals(null, slingUri.getUserInfo()); + assertEquals(null, slingUri.getHost()); + assertEquals(-1, slingUri.getPort()); + assertEquals(null, slingUri.getSelectorString()); + assertEquals(null, slingUri.getExtension()); + assertEquals(null, slingUri.getSuffix()); + assertEquals("console.log('long js'); return true;", slingUri.getSchemeSpecificPart()); + assertEquals(null, slingUri.getQuery()); + assertEquals(null, slingUri.getFragment()); + }, null, true); + + } + + @Test public void testMailtotUri() { String testUriStr = "mailto:[email protected]";; @@ -427,6 +444,26 @@ public class SlingUriTest { }, asList(resolver, null)); } + @Test + public void testDataUrl() { + + // data url according to https://tools.ietf.org/html/rfc2397 + String dataUrl = "data:image/gif;base64,R0lGODdhMAAwAPAAAAAAAP///ywAAAAAMAAw" + + "AAAC8IyPqcvt3wCcDkiLc7C0qwyGHhSWpjQu5yqmCYsapyuvUUlvONmOZtfzgFz" + + "ByTB10QgxOR0TqBQejhRNzOfkVJ+5YiUqrXF5Y5lKh/DeuNcP5yLWGsEbtLiOSp" + + "a/TPg7JpJHxyendzWTBfX0cxOnKPjgBzi4diinWGdkF8kjdfnycQZXZeYGejmJl" + + "ZeGl9i2icVqaNVailT6F5iJ90m6mvuTS4OK05M0vDk0Q4XUtwvKOzrcd3iq9uis" + + "F81M1OIcR7lEewwcLp7tuNNkM3uNna3F2JQFo97Vriy/Xl4/f1cf5VWzXyym7PH" + + "hhx4dbgYKAAA7"; + + testUri(dataUrl, false, false, false, true, true, slingUri -> { + assertEquals("data", slingUri.getScheme()); + assertEquals(StringUtils.substringAfter(dataUrl, ":"), slingUri.getSchemeSpecificPart()); + assertEquals(null, slingUri.getQuery()); + assertEquals(null, slingUri.getFragment()); + }, asList(resolver, null)); + } + // -- helper methods public static void testUri(String testUri, boolean isPath, boolean isAbsolutePath, boolean isRelativePath, boolean isAbsolute, boolean isOpaque, Consumer<SlingUri> additionalAssertions, List<ResourceResolver> resourceResolvers) { @@ -452,8 +489,15 @@ public class SlingUriTest { SlingUri slingUri = SlingUriBuilder.parse(testUri, resourceResolver).build(); if (!urlIsRestructured) { + URI javaUri = slingUri.toUri(); assertEquals("Uri toString() same as input", testUri, slingUri.toString()); - assertEquals("Uri toUri().toString() same as input", testUri, slingUri.toUri().toString()); + assertEquals("Uri toUri().toString() same as input", testUri, javaUri.toString()); + assertEquals("isOpaque() matches to java URI impl", javaUri.isOpaque(), slingUri.isOpaque()); + assertEquals("getSchemeSpecificPart() matches to java URI impl", javaUri.getRawSchemeSpecificPart(), + slingUri.getSchemeSpecificPart()); + assertEquals("getFragment() matches to java URI impl", javaUri.getRawFragment(), slingUri.getFragment()); + assertEquals("getQuery() matches to java URI impl", javaUri.getRawQuery(), slingUri.getQuery()); + assertEquals("isAbsolute() matches to java URI impl", javaUri.isAbsolute(), slingUri.isAbsolute()); } assertEquals("isPath()", isPath, slingUri.isPath()); @@ -462,14 +506,6 @@ public class SlingUriTest { assertEquals("isAbsolute()", isAbsolute, slingUri.isAbsolute()); assertEquals("isOpaque()", isOpaque, slingUri.isOpaque()); - URI javaUri = slingUri.toUri(); - assertEquals("isOpaque() matches to java URI impl", javaUri.isOpaque(), slingUri.isOpaque()); - assertEquals("getSchemeSpecificPart() matches to java URI impl", javaUri.getSchemeSpecificPart(), - slingUri.getSchemeSpecificPart()); - assertEquals("getFragment() matches to java URI impl", javaUri.getFragment(), slingUri.getFragment()); - assertEquals("getQuery() matches to java URI impl", javaUri.getQuery(), slingUri.getQuery()); - assertEquals("isAbsolute() matches to java URI impl", javaUri.isAbsolute(), slingUri.isAbsolute()); - additionalAssertions.accept(slingUri); SlingUri slingUriParsedFromSameInput = SlingUriBuilder.parse(testUri, resourceResolver).build();
