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
