hi Oliver, this can work as well.
I have opened https://issues.apache.org/jira/browse/SLING-3492 and everybody is fine I am going to attach this patch. regards antonio On Apr 4, 2014, at 1:47 PM, Oliver Lietz <[email protected]> wrote: > On Friday 04 April 2014 12:20:33 Carsten Ziegeler wrote: >> Why do you want to keep getPath() unchanged? You have a get method, get >> something and then have to check the value and then default to /. >> (In the end I don't care where the fix ends up, more curious) > > because we have otherwise get something and change it to a default in > getPath() *and* we are closer to SLING-722: > > Index: > bundles/auth/core/src/main/java/org/apache/sling/auth/core/impl/SlingAuthenticator.java > =================================================================== > --- > bundles/auth/core/src/main/java/org/apache/sling/auth/core/impl/SlingAuthenticator.java > > (revision 1584529) > +++ > bundles/auth/core/src/main/java/org/apache/sling/auth/core/impl/SlingAuthenticator.java > > (working copy) > @@ -680,13 +680,11 @@ > private AuthenticationInfo getAuthenticationInfo(HttpServletRequest > request, HttpServletResponse response) { > > // Get the path used to select the authenticator, if the SlingServlet > - // itself has been requested without any more info, this will be null > + // itself has been requested without any more info, this will be > empty > // and we assume the root (SLING-722) > - final String path = getPath(request); > + String path = getPath(request); > if (path.length() == 0) { > - // should not happen, be safe an return anonymous credentials > - log.warn("get authentication info: request path is empty; > assuming anonymous"); > - return getAnonymousCredentials(); > + path = "/"; > } > > final List<AbstractAuthenticationHandlerHolder>[] localArray = > this.authHandlerCache.findApplicableHolder(request); > > > O. > >> Carsten >> >> 2014-04-04 12:13 GMT+02:00 Oliver Lietz <[email protected]>: >>> On Friday 04 April 2014 11:16:36 Carsten Ziegeler wrote: >>>> So I guess its as simple as this: >>>> >>>> Index: >>>> src/main/java/org/apache/sling/auth/core/impl/SlingAuthenticator.java >>>> =================================================================== --- >>>> src/main/java/org/apache/sling/auth/core/impl/SlingAuthenticator.java >>>> (Revision 1584521) >>>> +++ >>>> src/main/java/org/apache/sling/auth/core/impl/SlingAuthenticator.java >>>> (Arbeitskopie) >>>> @@ -674,6 +674,9 @@ >>>> >>>> if (request.getPathInfo() != null) { >>>> >>>> sb.append(request.getPathInfo()); >>>> >>>> } >>>> >>>> + if ( sb.length() == 0 ) { >>>> + sb.append("/"); >>>> + } >>>> >>>> return sb.toString(); >>>> >>>> } >>>> >>>> @@ -683,11 +686,6 @@ >>>> >>>> // itself has been requested without any more info, this will >>>> be >>>> >>>> null >>>> >>>> // and we assume the root (SLING-722) >>>> final String path = getPath(request); >>>> >>>> - if (path.length() == 0) { >>>> - // should not happen, be safe an return anonymous >>> >>> credentials >>> >>>> - log.warn("get authentication info: request path is empty; >>>> assuming anonymous"); >>>> - return getAnonymousCredentials(); >>>> - } >>>> >>>> final List<AbstractAuthenticationHandlerHolder>[] localArray = >>>> >>>> this.authHandlerCache.findApplicableHolder(request); >>>> >>>> for (int m = 0; m < localArray.length; m++) { >>> >>> So we have a case where #getServletPath() returns "" and #getPathInfo() >>> null? >>> >>> >>> http://docs.oracle.com/javaee/6/api/javax/servlet/http/HttpServletRequest >>> .html#getServletPath() >>> >>> Returns the part of this request's URL that calls the servlet. This path >>> starts with a "/" character and includes either the servlet name or a >>> path to >>> the servlet, but does not include any extra path information or a query >>> string. Same as the value of the CGI variable SCRIPT_NAME. >>> This method will return an empty string ("") if the servlet used to >>> process this request was matched using the "/*" pattern. >>> >>> >>> >>> http://docs.oracle.com/javaee/6/api/javax/servlet/http/HttpServletRequest >>> .html#getPathInfo() >>> >>> Returns any extra path information associated with the URL the client >>> sent when it made this request. The extra path information follows the >>> servlet path >>> but precedes the query string and will start with a "/" character. >>> This method returns null if there was no extra path information. >>> Same as the value of the CGI variable PATH_INFO. >>> >>> @Carsten: I would keep getPath() unchanged and change the path after the >>> >>> comment for SLING-722: >>> String path = getPath(request); >>> if (path.length() == 0) { >>> >>> path = "/"; >>> >>> } >>> >>> Regards, >>> O. >>> >>>> Do we have an issue for this? >>>> >>>> Regards >>>> Carsten >>>> >>>> 2014-04-04 10:17 GMT+02:00 Antonio Sanso <[email protected]>: >>>>> On Apr 4, 2014, at 10:14 AM, Carsten Ziegeler <[email protected] >>>>> >>>>> <mailto:[email protected]>> wrote: >>>>> the >>>>> >>>>> fallback at the end is still needed I guess. >>>>> >>>>> for sure.... :) >>>>> >>>>> regards >>>>> >>>>> antonio >>>>> >>>>> >>>>> Carsten >>>>> >>>>> >>>>> 2014-04-04 10:01 GMT+02:00 Antonio Sanso <[email protected]<mailto: >>>>> [email protected]>>: >>>>> >>>>> >>>>> On Apr 4, 2014, at 9:54 AM, Carsten Ziegeler >>>>> <[email protected]<mailto: [email protected]>> wrote: >>>>> >>>>> So we could just add the check to the getPath() method, and this >>>>> should fix >>>>> the problem, right? >>>>> >>>>> >>>>> I think so. At that point we can as well remove the associated >>>>> getAnonymousCredentials() since getPath can never be empty anymore. >>> >>> Will >>> >>>>> be at least "/" >>>>> >>>>> regards >>>>> >>>>> antonio >>>>> >>>>> Carsten >>>>> >>>>> >>>>> 2014-04-04 9:11 GMT+02:00 Antonio Sanso <[email protected]<mailto: >>>>> [email protected]>>: >>>>> >>>>> hi carsten >>>>> On Apr 3, 2014, at 6:34 PM, Carsten Ziegeler >>>>> <[email protected]<mailto: [email protected]>> >>>>> wrote: >>>>> >>>>> Hi Antonio, >>>>> >>>>> where is the NPE happening? >>>>> >>>>> I copied the subject from SLING-722 . With the current code there is >>>>> actual no NPE but some functional issue (see my mail to Oliver) >>>>> >>>>> regards >>>>> >>>>> antonio >>>>> >>>>> >>>>> Carsten >>>>> >>>>> >>>>> 2014-04-03 16:12 GMT+02:00 Antonio Sanso <[email protected]<mailto: >>>>> [email protected]>>: >>>>> >>>>> hi *, >>>>> >>>>> SLING-722 introduced an important fix to solve some issue related to >>>>> sling >>>>> being registered in a Servlet container. >>>>> Unluckily this fix has been removed in SLING-2998. >>>>> The rationale behind this is not too clear to me. I am not against >>>>> SLING-2998 per se. >>>>> I think though that SLING-2998 should also contain the fix contained >>>>> in >>>>> SLING-722. >>>>> >>>>> @oliver WDYT? >>>>> >>>>> regards >>>>> >>>>> antonio >>>>> >>>>> >>>>> [0] https://issues.apache.org/jira/browse/SLING-722 >>>>> [1] https://issues.apache.org/jira/browse/SLING-2998
