You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by gh...@apache.org on 2020/09/30 18:36:40 UTC

[sling-org-apache-sling-api] branch feature/SLING-9777-SlingUri-better-encoding-handling created (now f54999c)

This is an automated email from the ASF dual-hosted git repository.

ghenzler pushed a change to branch feature/SLING-9777-SlingUri-better-encoding-handling
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-api.git.


      at f54999c  SLING-9777 Better handling for invalid URIs when using SlingUri (special characters, spaces)

This branch includes the following new commits:

     new f54999c  SLING-9777 Better handling for invalid URIs when using SlingUri (special characters, spaces)

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



[sling-org-apache-sling-api] 01/01: SLING-9777 Better handling for invalid URIs when using SlingUri (special characters, spaces)

Posted by gh...@apache.org.
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 <ge...@netcentric.biz>
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&param2=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&param2%5B*%5D=value2&param3=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%3F%23@example.com/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:pw@example.com/path/with/spaces/A name with spaces.sel1.pdf/suffix?par1=val1&par2=val2#frag",
+                        "http://user:pw@example.com/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:jon.doe@example.com";
 
@@ -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();