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

request getPathInfo returns null #4577

Closed
liudonghua123 opened this issue Feb 16, 2020 · 6 comments · Fixed by #4580
Closed

request getPathInfo returns null #4577

liudonghua123 opened this issue Feb 16, 2020 · 6 comments · Fixed by #4580

Comments

@liudonghua123
Copy link

Jetty version
9.4.26.v20200117

Java version
1.8.0_191

OS type/version
Windows 10 1909

Description
I use IPAccessHandler for restricting ip and url access. My config file is something like belows.

<?xml version="1.0"?><!DOCTYPE Configure PUBLIC "-//Jetty//Configure//EN" "http://www.eclipse.org/jetty/configure_9_3.dtd">

<!-- =============================================================== --><!-- Configure the test-jaas webapp                                  --><!-- =============================================================== -->
<Configure id='wac' class="org.eclipse.jetty.webapp.WebAppContext">

  <Set name="contextPath">/auditlog</Set>
  <Set name="resourceBase">/d:/log/</Set>
  
  <Call name="insertHandler">
    <Arg>
      <New class="org.eclipse.jetty.server.handler.IPAccessHandler">
        <Call name="addWhite">
          <Arg>127.0.0.1|/*</Arg>
        </Call>
      </New>
    </Arg>
  </Call>

</Configure>

But when I browser http://127.0.0.1:8080/auditlog/, it always give me 403 error. After go through the code, I find it was baseRequest.getPathInfo() returned null instead of /auditlog/, so the validator logic failed.

image

liudonghua123 added a commit to liudonghua123/jetty.project that referenced this issue Feb 16, 2020
liudonghua123 added a commit to liudonghua123/jetty.project that referenced this issue Feb 16, 2020
This reverts commit edb5196.
liudonghua123 added a commit to liudonghua123/jetty.project that referenced this issue Feb 16, 2020
@sbordet
Copy link
Contributor

sbordet commented Feb 16, 2020

@gregw since IP|InetAccessHandler is a Handler and can be set in front of many different contexts, why do we use getPathInfo() to match, instead of the full URI path?

@gregw
Copy link
Contributor

gregw commented Feb 16, 2020

I think the idea was that if the access handler is installed outside of a context, then it will match paths on the entire URI. If it is installed within a context, it will match paths against the URI stripped of the contextPath.

However, in this case the servlet path has also been taken from the pathInfo, leaving null. So ideally we'd like to apply the path match before the servletHandler does. So perhaps it needs to be a ScopedHandler.... or operate on addPath(servletPath, pathInfo)

liudonghua123 added a commit to liudonghua123/jetty.project that referenced this issue Feb 16, 2020
Signed-off-by: liudonghua123 <[email protected]>
@sbordet
Copy link
Contributor

sbordet commented Feb 16, 2020

@gregw ok so this needs a better fix than what provided in #4579, which is wrong because it only considers the context path.

gregw added a commit that referenced this issue Feb 17, 2020
Fixes and tests #4577 IPAccessHandler in context by using target instead of pathInfo for path matching.

Signed-off-by: Greg Wilkins <[email protected]>
@gregw
Copy link
Contributor

gregw commented Feb 17, 2020

@sbordet the solution is to use the target instead of the pathInfo as it will be the pathInContext if deployed in a context. I have fixed in #4580, but I think we need to handle named dispatches also...

@liudonghua123
Copy link
Author

@gregw @sbordet Thanks. Will the next release contain this fix?

gregw added a commit that referenced this issue Feb 21, 2020
Updates from review.

Signed-off-by: Greg Wilkins <[email protected]>
@joakime joakime linked a pull request Feb 22, 2020 that will close this issue
@gregw
Copy link
Contributor

gregw commented Feb 24, 2020

yes

@gregw gregw closed this as completed Feb 24, 2020
gregw added a commit that referenced this issue Feb 24, 2020
Match on full URI path rather than target.

Signed-off-by: Greg Wilkins <[email protected]>
gregw added a commit that referenced this issue Feb 25, 2020
* Fixes #4577 IPAccessHandler in context

Fixes and tests #4577 IPAccessHandler in context by using target instead of pathInfo for path matching.

Signed-off-by: Greg Wilkins <[email protected]>

* Tests #4577 IPAccessHandler target

Updates from review.

Signed-off-by: Greg Wilkins <[email protected]>

* Issue #4577 IpAccessHandler NPE

Match on full URI path rather than target.

Signed-off-by: Greg Wilkins <[email protected]>
lachlan-roberts added a commit that referenced this issue Mar 2, 2020
lachlan-roberts added a commit that referenced this issue Mar 10, 2020
…dler_getPathInfo

Issue #4577 - request getPathInfo() could be null in InetAccessHandler
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 a pull request may close this issue.

3 participants