You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by GitBox <gi...@apache.org> on 2020/09/30 18:55:50 UTC

[GitHub] [sling-org-apache-sling-engine] ghenzler opened a new pull request #10: SLING-9662 Use SlingUri as provided in request from auth.core

ghenzler opened a new pull request #10:
URL: https://github.com/apache/sling-org-apache-sling-engine/pull/10


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [sling-org-apache-sling-engine] ghenzler commented on a change in pull request #10: SLING-9662 Use SlingUri as provided in request from auth.core

Posted by GitBox <gi...@apache.org>.
ghenzler commented on a change in pull request #10:
URL: https://github.com/apache/sling-org-apache-sling-engine/pull/10#discussion_r498743213



##########
File path: src/main/java/org/apache/sling/engine/impl/request/RequestData.java
##########
@@ -262,10 +260,14 @@ public Resource initResource(ResourceResolver resourceResolver) {
     public void initServlet(final Resource resource,
             final ServletResolver sr) {
         // the resource and the request path info, will never be null
-        RequestPathInfo requestPathInfo = new SlingRequestPathInfo(resource);
-        ContentData contentData = setContent(resource, requestPathInfo);
-
-	    requestProgressTracker.log("Resource Path Info: {0}", requestPathInfo);
+        SlingUri slingUri = (SlingUri) getSlingRequest().getAttribute(AuthenticationSupport.REQUEST_ATTRIBUTE_URI);

Review comment:
       The problem was a bit that initResource() already returns a resource and using `slingUri` as member var would put a bit duplication on the object (`currentContentData` is already a field and `slingUri` contains some of the same information) - but I agree using the request att again is not really nice either, I'll find a solution




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [sling-org-apache-sling-engine] cziegeler commented on a change in pull request #10: SLING-9662 Use SlingUri as provided in request from auth.core

Posted by GitBox <gi...@apache.org>.
cziegeler commented on a change in pull request #10:
URL: https://github.com/apache/sling-org-apache-sling-engine/pull/10#discussion_r498024945



##########
File path: src/main/java/org/apache/sling/engine/impl/request/RequestData.java
##########
@@ -237,19 +237,17 @@ public Resource initResource(ResourceResolver resourceResolver) {
         requestProgressTracker.startTimer("ResourceResolution");
         final SlingHttpServletRequest request = getSlingRequest();
 
-        StringBuffer requestURL = servletRequest.getRequestURL();
-        String path = request.getPathInfo();
-        if (requestURL.indexOf(";") > -1 && !path.contains(";")) {
-            final String decodedURL;
-            try {
-                decodedURL = URLDecoder.decode(requestURL.toString(), "UTF-8");
-            } catch (UnsupportedEncodingException e) {
-                throw new AssertionError("UTF-8 encoding is not supported");
-            }
-            path = path.concat(decodedURL.substring(decodedURL.indexOf(';')));
+        // Set by o.a.s.auth.core bundle
+        SlingUri slingUri = (SlingUri) request.getAttribute(AuthenticationSupport.REQUEST_ATTRIBUTE_URI);

Review comment:
       I think we should allow that the attribute is missing




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [sling-org-apache-sling-engine] ghenzler commented on a change in pull request #10: SLING-9662 Use SlingUri as provided in request from auth.core

Posted by GitBox <gi...@apache.org>.
ghenzler commented on a change in pull request #10:
URL: https://github.com/apache/sling-org-apache-sling-engine/pull/10#discussion_r498540869



##########
File path: src/main/java/org/apache/sling/engine/impl/request/RequestData.java
##########
@@ -237,19 +237,17 @@ public Resource initResource(ResourceResolver resourceResolver) {
         requestProgressTracker.startTimer("ResourceResolution");
         final SlingHttpServletRequest request = getSlingRequest();
 
-        StringBuffer requestURL = servletRequest.getRequestURL();
-        String path = request.getPathInfo();
-        if (requestURL.indexOf(";") > -1 && !path.contains(";")) {
-            final String decodedURL;
-            try {
-                decodedURL = URLDecoder.decode(requestURL.toString(), "UTF-8");

Review comment:
       Not in `SlingUri`,  idea is to allow `SlingUri` to contain both encoded and non-encoded values (non-encoded is unproblematic as long as the URI components are in separate fields, it only gets problematic when you serialize it to a String, see [SLING-9777](https://issues.apache.org/jira/browse/SLING-9777) for details.
   
   But the idea is that  [PathToUriMappingServiceImpl.getSlingUriFromRequest()](https://github.com/apache/sling-org-apache-sling-resourceresolver/blob/4ab43acf49dfd9cd25119b74054980b3c4144918/src/main/java/org/apache/sling/resourceresolver/impl/urimapping/PathToUriMappingServiceImpl.java#L201) takes care of it, it uses the methods `request.getServletPath()` and `request.getPathInfo()` that both return already decoded values according to [javadoc HttpServletRequest](https://docs.oracle.com/javaee/6/api/javax/servlet/http/HttpServletRequest.html#getServletPath())
   
   But now looking at [SLING-4804](https://issues.apache.org/jira/browse/SLING-4804) and @trekawek 's commit 83acbff6b08f52a50aa it looks the path parameter is not contained in getServletPath(), so the most likely current version needs a fix, however this should be done in [PathToUriMappingServiceImpl.getSlingUriFromRequest()](https://github.com/apache/sling-org-apache-sling-resourceresolver/blob/4ab43acf49dfd9cd25119b74054980b3c4144918/src/main/java/org/apache/sling/resourceresolver/impl/urimapping/PathToUriMappingServiceImpl.java#L201) then. I'll take care of it.
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [sling-org-apache-sling-engine] ghenzler commented on a change in pull request #10: SLING-9662 Use SlingUri as provided in request from auth.core

Posted by GitBox <gi...@apache.org>.
ghenzler commented on a change in pull request #10:
URL: https://github.com/apache/sling-org-apache-sling-engine/pull/10#discussion_r498737048



##########
File path: src/main/java/org/apache/sling/engine/impl/request/RequestData.java
##########
@@ -237,19 +237,17 @@ public Resource initResource(ResourceResolver resourceResolver) {
         requestProgressTracker.startTimer("ResourceResolution");
         final SlingHttpServletRequest request = getSlingRequest();
 
-        StringBuffer requestURL = servletRequest.getRequestURL();
-        String path = request.getPathInfo();
-        if (requestURL.indexOf(";") > -1 && !path.contains(";")) {
-            final String decodedURL;
-            try {
-                decodedURL = URLDecoder.decode(requestURL.toString(), "UTF-8");
-            } catch (UnsupportedEncodingException e) {
-                throw new AssertionError("UTF-8 encoding is not supported");
-            }
-            path = path.concat(decodedURL.substring(decodedURL.indexOf(';')));
+        // Set by o.a.s.auth.core bundle
+        SlingUri slingUri = (SlingUri) request.getAttribute(AuthenticationSupport.REQUEST_ATTRIBUTE_URI);
+        if (slingUri == null) {
+            throw new IllegalStateException(
+                    "SlingUri not available as attribute of request (expected to be set in bundle o.a.s.auth.core)");
         }
+        // ensure slingUri is bound to correct resource resolver
+        slingUri = slingUri.adjust(b -> b.setResourceResolver(resourceResolver));
+        request.setAttribute(AuthenticationSupport.REQUEST_ATTRIBUTE_URI, slingUri);
 
-        Resource resource = resourceResolver.resolve(request, path);
+        Resource resource = resourceResolver.resolve(request, slingUri.getPath());

Review comment:
       yes, so the version works because rr.resolve() is not yet using PathToUriMappingService (and hence potential SlingUriMapper services are not called when using rr.resolve()). Moving forward PathToUriMappingService.resolve()/map() should delegate to PathToUriMappingService and for that, I  agree we'll need an additional method in ResourceResolver - this should be able to stay privately in bundle as PathToUriMappingServiceImpl and ResourceResolverImpl are in same (this) bundle. I'll work on this and update the PR
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [sling-org-apache-sling-engine] ghenzler commented on a change in pull request #10: SLING-9662 Use SlingUri as provided in request from auth.core

Posted by GitBox <gi...@apache.org>.
ghenzler commented on a change in pull request #10:
URL: https://github.com/apache/sling-org-apache-sling-engine/pull/10#discussion_r498531644



##########
File path: src/main/java/org/apache/sling/engine/impl/request/RequestData.java
##########
@@ -237,19 +237,17 @@ public Resource initResource(ResourceResolver resourceResolver) {
         requestProgressTracker.startTimer("ResourceResolution");
         final SlingHttpServletRequest request = getSlingRequest();
 
-        StringBuffer requestURL = servletRequest.getRequestURL();
-        String path = request.getPathInfo();
-        if (requestURL.indexOf(";") > -1 && !path.contains(";")) {
-            final String decodedURL;
-            try {
-                decodedURL = URLDecoder.decode(requestURL.toString(), "UTF-8");
-            } catch (UnsupportedEncodingException e) {
-                throw new AssertionError("UTF-8 encoding is not supported");
-            }
-            path = path.concat(decodedURL.substring(decodedURL.indexOf(';')));
+        // Set by o.a.s.auth.core bundle
+        SlingUri slingUri = (SlingUri) request.getAttribute(AuthenticationSupport.REQUEST_ATTRIBUTE_URI);

Review comment:
       So in my tests it seemed like this case is impossible (since all requests pass through `auth.core`), hence implementing  a fallback here would produce "dead code" - can you think of a scenario when `auth.core` is not setting `REQUEST_ATTRIBUTE_URI`?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [sling-org-apache-sling-engine] cziegeler commented on a change in pull request #10: SLING-9662 Use SlingUri as provided in request from auth.core

Posted by GitBox <gi...@apache.org>.
cziegeler commented on a change in pull request #10:
URL: https://github.com/apache/sling-org-apache-sling-engine/pull/10#discussion_r498646696



##########
File path: src/main/java/org/apache/sling/engine/impl/request/RequestData.java
##########
@@ -237,19 +237,17 @@ public Resource initResource(ResourceResolver resourceResolver) {
         requestProgressTracker.startTimer("ResourceResolution");
         final SlingHttpServletRequest request = getSlingRequest();
 
-        StringBuffer requestURL = servletRequest.getRequestURL();
-        String path = request.getPathInfo();
-        if (requestURL.indexOf(";") > -1 && !path.contains(";")) {
-            final String decodedURL;
-            try {
-                decodedURL = URLDecoder.decode(requestURL.toString(), "UTF-8");
-            } catch (UnsupportedEncodingException e) {
-                throw new AssertionError("UTF-8 encoding is not supported");
-            }
-            path = path.concat(decodedURL.substring(decodedURL.indexOf(';')));
+        // Set by o.a.s.auth.core bundle
+        SlingUri slingUri = (SlingUri) request.getAttribute(AuthenticationSupport.REQUEST_ATTRIBUTE_URI);

Review comment:
       I don't like implicit contracts, it is not clear that when calling this method, the request attribute needs to be set. This might pose a problem over time. I guess the other situation is when an old auth.core bundle is used - I would assume that the engine does not have an import auth.core as its only using constants.
   We have dead code nevertheless, either the dead code is throwing an exception or creating a new SlingUri :)




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [sling-org-apache-sling-engine] ghenzler commented on a change in pull request #10: SLING-9662 Use SlingUri as provided in request from auth.core

Posted by GitBox <gi...@apache.org>.
ghenzler commented on a change in pull request #10:
URL: https://github.com/apache/sling-org-apache-sling-engine/pull/10#discussion_r498737048



##########
File path: src/main/java/org/apache/sling/engine/impl/request/RequestData.java
##########
@@ -237,19 +237,17 @@ public Resource initResource(ResourceResolver resourceResolver) {
         requestProgressTracker.startTimer("ResourceResolution");
         final SlingHttpServletRequest request = getSlingRequest();
 
-        StringBuffer requestURL = servletRequest.getRequestURL();
-        String path = request.getPathInfo();
-        if (requestURL.indexOf(";") > -1 && !path.contains(";")) {
-            final String decodedURL;
-            try {
-                decodedURL = URLDecoder.decode(requestURL.toString(), "UTF-8");
-            } catch (UnsupportedEncodingException e) {
-                throw new AssertionError("UTF-8 encoding is not supported");
-            }
-            path = path.concat(decodedURL.substring(decodedURL.indexOf(';')));
+        // Set by o.a.s.auth.core bundle
+        SlingUri slingUri = (SlingUri) request.getAttribute(AuthenticationSupport.REQUEST_ATTRIBUTE_URI);
+        if (slingUri == null) {
+            throw new IllegalStateException(
+                    "SlingUri not available as attribute of request (expected to be set in bundle o.a.s.auth.core)");
         }
+        // ensure slingUri is bound to correct resource resolver
+        slingUri = slingUri.adjust(b -> b.setResourceResolver(resourceResolver));
+        request.setAttribute(AuthenticationSupport.REQUEST_ATTRIBUTE_URI, slingUri);
 
-        Resource resource = resourceResolver.resolve(request, path);
+        Resource resource = resourceResolver.resolve(request, slingUri.getPath());

Review comment:
       yes, so the version works because rr.resolve() is not yet using PathToUriMappingService (and hence potential SlingUriMapper services are not called when using rr.resolve()). Moving forward PathToUriMappingService.resolve()/map() should delegate to PathToUriMappingService and for that, I  agree we'll need an additional method in ResourceResolver - this should be able to stay privately in bundle as PathToUriMappingServiceImpl and ResourceResolverImpl are in same bundle (resourceresolver). I'll work on this and update the PR
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [sling-org-apache-sling-engine] ghenzler commented on a change in pull request #10: SLING-9662 Use SlingUri as provided in request from auth.core

Posted by GitBox <gi...@apache.org>.
ghenzler commented on a change in pull request #10:
URL: https://github.com/apache/sling-org-apache-sling-engine/pull/10#discussion_r498743213



##########
File path: src/main/java/org/apache/sling/engine/impl/request/RequestData.java
##########
@@ -262,10 +260,14 @@ public Resource initResource(ResourceResolver resourceResolver) {
     public void initServlet(final Resource resource,
             final ServletResolver sr) {
         // the resource and the request path info, will never be null
-        RequestPathInfo requestPathInfo = new SlingRequestPathInfo(resource);
-        ContentData contentData = setContent(resource, requestPathInfo);
-
-	    requestProgressTracker.log("Resource Path Info: {0}", requestPathInfo);
+        SlingUri slingUri = (SlingUri) getSlingRequest().getAttribute(AuthenticationSupport.REQUEST_ATTRIBUTE_URI);

Review comment:
       The problem was a bit that initResource() already returns a resource and using `slingUri` as member var would put a bit duplication on the object (`currentContentData` is already a field and `slingUri` contains some of the same information) - but I agree using the request attribute again is not really nice either, I'll find a solution




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [sling-org-apache-sling-engine] ghenzler commented on a change in pull request #10: SLING-9662 Use SlingUri as provided in request from auth.core

Posted by GitBox <gi...@apache.org>.
ghenzler commented on a change in pull request #10:
URL: https://github.com/apache/sling-org-apache-sling-engine/pull/10#discussion_r498734704



##########
File path: src/main/java/org/apache/sling/engine/impl/request/RequestData.java
##########
@@ -237,19 +237,17 @@ public Resource initResource(ResourceResolver resourceResolver) {
         requestProgressTracker.startTimer("ResourceResolution");
         final SlingHttpServletRequest request = getSlingRequest();
 
-        StringBuffer requestURL = servletRequest.getRequestURL();
-        String path = request.getPathInfo();
-        if (requestURL.indexOf(";") > -1 && !path.contains(";")) {
-            final String decodedURL;
-            try {
-                decodedURL = URLDecoder.decode(requestURL.toString(), "UTF-8");
-            } catch (UnsupportedEncodingException e) {
-                throw new AssertionError("UTF-8 encoding is not supported");
-            }
-            path = path.concat(decodedURL.substring(decodedURL.indexOf(';')));
+        // Set by o.a.s.auth.core bundle
+        SlingUri slingUri = (SlingUri) request.getAttribute(AuthenticationSupport.REQUEST_ATTRIBUTE_URI);
+        if (slingUri == null) {
+            throw new IllegalStateException(
+                    "SlingUri not available as attribute of request (expected to be set in bundle o.a.s.auth.core)");
         }
+        // ensure slingUri is bound to correct resource resolver
+        slingUri = slingUri.adjust(b -> b.setResourceResolver(resourceResolver));
+        request.setAttribute(AuthenticationSupport.REQUEST_ATTRIBUTE_URI, slingUri);

Review comment:
       Makes sense.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [sling-org-apache-sling-engine] cziegeler commented on a change in pull request #10: SLING-9662 Use SlingUri as provided in request from auth.core

Posted by GitBox <gi...@apache.org>.
cziegeler commented on a change in pull request #10:
URL: https://github.com/apache/sling-org-apache-sling-engine/pull/10#discussion_r498025085



##########
File path: src/main/java/org/apache/sling/engine/impl/request/RequestData.java
##########
@@ -237,19 +237,17 @@ public Resource initResource(ResourceResolver resourceResolver) {
         requestProgressTracker.startTimer("ResourceResolution");
         final SlingHttpServletRequest request = getSlingRequest();
 
-        StringBuffer requestURL = servletRequest.getRequestURL();
-        String path = request.getPathInfo();
-        if (requestURL.indexOf(";") > -1 && !path.contains(";")) {
-            final String decodedURL;
-            try {
-                decodedURL = URLDecoder.decode(requestURL.toString(), "UTF-8");
-            } catch (UnsupportedEncodingException e) {
-                throw new AssertionError("UTF-8 encoding is not supported");
-            }
-            path = path.concat(decodedURL.substring(decodedURL.indexOf(';')));
+        // Set by o.a.s.auth.core bundle
+        SlingUri slingUri = (SlingUri) request.getAttribute(AuthenticationSupport.REQUEST_ATTRIBUTE_URI);
+        if (slingUri == null) {
+            throw new IllegalStateException(
+                    "SlingUri not available as attribute of request (expected to be set in bundle o.a.s.auth.core)");
         }
+        // ensure slingUri is bound to correct resource resolver
+        slingUri = slingUri.adjust(b -> b.setResourceResolver(resourceResolver));
+        request.setAttribute(AuthenticationSupport.REQUEST_ATTRIBUTE_URI, slingUri);

Review comment:
       I would rather remove the attribute, so its not leaking any further

##########
File path: src/main/java/org/apache/sling/engine/impl/request/RequestData.java
##########
@@ -237,19 +237,17 @@ public Resource initResource(ResourceResolver resourceResolver) {
         requestProgressTracker.startTimer("ResourceResolution");
         final SlingHttpServletRequest request = getSlingRequest();
 
-        StringBuffer requestURL = servletRequest.getRequestURL();
-        String path = request.getPathInfo();
-        if (requestURL.indexOf(";") > -1 && !path.contains(";")) {
-            final String decodedURL;
-            try {
-                decodedURL = URLDecoder.decode(requestURL.toString(), "UTF-8");
-            } catch (UnsupportedEncodingException e) {
-                throw new AssertionError("UTF-8 encoding is not supported");
-            }
-            path = path.concat(decodedURL.substring(decodedURL.indexOf(';')));
+        // Set by o.a.s.auth.core bundle
+        SlingUri slingUri = (SlingUri) request.getAttribute(AuthenticationSupport.REQUEST_ATTRIBUTE_URI);
+        if (slingUri == null) {
+            throw new IllegalStateException(
+                    "SlingUri not available as attribute of request (expected to be set in bundle o.a.s.auth.core)");
         }
+        // ensure slingUri is bound to correct resource resolver
+        slingUri = slingUri.adjust(b -> b.setResourceResolver(resourceResolver));
+        request.setAttribute(AuthenticationSupport.REQUEST_ATTRIBUTE_URI, slingUri);
 
-        Resource resource = resourceResolver.resolve(request, path);
+        Resource resource = resourceResolver.resolve(request, slingUri.getPath());

Review comment:
       I think this is where we need a new method in the resource resolver, uri mapping has already been applied.
   Or we can use getResource() - as slingUri has already done that part (using a service user) - only if null is returned, we can invoke resolve()

##########
File path: src/main/java/org/apache/sling/engine/impl/request/RequestData.java
##########
@@ -262,10 +260,14 @@ public Resource initResource(ResourceResolver resourceResolver) {
     public void initServlet(final Resource resource,
             final ServletResolver sr) {
         // the resource and the request path info, will never be null
-        RequestPathInfo requestPathInfo = new SlingRequestPathInfo(resource);
-        ContentData contentData = setContent(resource, requestPathInfo);
-
-	    requestProgressTracker.log("Resource Path Info: {0}", requestPathInfo);
+        SlingUri slingUri = (SlingUri) getSlingRequest().getAttribute(AuthenticationSupport.REQUEST_ATTRIBUTE_URI);

Review comment:
       I think the uri should rather be parsed in as an argument to this method than relying on request attributes




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [sling-org-apache-sling-engine] ghenzler commented on a change in pull request #10: SLING-9662 Use SlingUri as provided in request from auth.core

Posted by GitBox <gi...@apache.org>.
ghenzler commented on a change in pull request #10:
URL: https://github.com/apache/sling-org-apache-sling-engine/pull/10#discussion_r498734513



##########
File path: src/main/java/org/apache/sling/engine/impl/request/RequestData.java
##########
@@ -237,19 +237,17 @@ public Resource initResource(ResourceResolver resourceResolver) {
         requestProgressTracker.startTimer("ResourceResolution");
         final SlingHttpServletRequest request = getSlingRequest();
 
-        StringBuffer requestURL = servletRequest.getRequestURL();
-        String path = request.getPathInfo();
-        if (requestURL.indexOf(";") > -1 && !path.contains(";")) {
-            final String decodedURL;
-            try {
-                decodedURL = URLDecoder.decode(requestURL.toString(), "UTF-8");
-            } catch (UnsupportedEncodingException e) {
-                throw new AssertionError("UTF-8 encoding is not supported");
-            }
-            path = path.concat(decodedURL.substring(decodedURL.indexOf(';')));
+        // Set by o.a.s.auth.core bundle
+        SlingUri slingUri = (SlingUri) request.getAttribute(AuthenticationSupport.REQUEST_ATTRIBUTE_URI);

Review comment:
       ok fair enough, I'll add a fallback handling :)




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [sling-org-apache-sling-engine] cziegeler commented on a change in pull request #10: SLING-9662 Use SlingUri as provided in request from auth.core

Posted by GitBox <gi...@apache.org>.
cziegeler commented on a change in pull request #10:
URL: https://github.com/apache/sling-org-apache-sling-engine/pull/10#discussion_r498024611



##########
File path: src/main/java/org/apache/sling/engine/impl/request/RequestData.java
##########
@@ -237,19 +237,17 @@ public Resource initResource(ResourceResolver resourceResolver) {
         requestProgressTracker.startTimer("ResourceResolution");
         final SlingHttpServletRequest request = getSlingRequest();
 
-        StringBuffer requestURL = servletRequest.getRequestURL();
-        String path = request.getPathInfo();
-        if (requestURL.indexOf(";") > -1 && !path.contains(";")) {
-            final String decodedURL;
-            try {
-                decodedURL = URLDecoder.decode(requestURL.toString(), "UTF-8");

Review comment:
       I didn't check, but I assume this logic when into SlingUri?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org