Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PAYARA-2111 Fixed warning in logs from mismatched error page #2037

Conversation

michaelranaldo
Copy link
Contributor

Tested with example application and custom application, behaviour remains the same, just without the error page. If a file is provided and configured within Wicket, but cannot be found, then Wicket will throw the error - if a file is correct but incorrectly configured then Payara's behaviour remains the same.

Thanks for @arjantijms and @MattGill98 for the help!

Fixes #1886

@michaelranaldo
Copy link
Contributor Author

jenkins test please

@payara-ci
Copy link
Contributor

Quick build and test passed!

Copy link
Contributor

@smillidge smillidge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add the Portions copyright comment...

@michaelranaldo
Copy link
Contributor Author

Jenkins test please

@payara-ci
Copy link
Contributor

Quick build and test passed!

@arjantijms
Copy link
Contributor

arjantijms commented Sep 27, 2017

2 extra things to look at:

The pattern is now compared via "glob", but the Servlet patterns are a bit different, namely:

In the Web application deployment descriptor, the following syntax is used to define mappings:

  • A string beginning with a ‘/’ character and ending with a ‘/*’ suffix is used for path mapping.
  • A string beginning with a ‘*.’ prefix is used as an extension mapping.
  • A string containing only the ’/’ character indicates the "default" servlet of the application. In this case the servlet path is the request URI minus the context path and the path info is null.
  • All other strings are used for exact matches only.

Internally there should be a specific matcher for this available, as Payara used this too internally to dispatch. From a spec point of view, WebRoleRefPermission on a PermissionCollection does this same kind of match as well. Semi-internally, see also javax.security.jacc.URLPattern.

Another thing that I'm missing is the check on the filter to see if the filter filters the error dispatch. Only those filters should be taken into account for the URL match.

Otherwise, looks good!

@arjantijms
Copy link
Contributor

Discussed this internally and javax.security.jacc.URLPattern seems like the easiest way; idea is to copy this (shade it) to a Payara package, and then create a small utility method, e.g. URLPatternMatcher.match(String pattern1, String pattern2); that instantiates URLPattern with pattern1 and then called implies(pattern2) on it.

@michaelranaldo michaelranaldo force-pushed the PAYARA-2111-Warning-in-logs-with-an-error-page branch from b7e677a to 5174e19 Compare October 10, 2017 08:49
@michaelranaldo
Copy link
Contributor Author

jenkins test please

@payara-ci
Copy link
Contributor

Quick build and test passed!

@michaelranaldo
Copy link
Contributor Author

jenkins test please

@payara-ci
Copy link
Contributor

Quick build and test passed!

@arjantijms arjantijms dismissed smillidge’s stale review October 11, 2017 09:48

Change request addressed here: 1310659

Copy link
Contributor

@arjantijms arjantijms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thx!

@arjantijms arjantijms merged commit 420f203 into payara:master Oct 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants