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

Server's Request#getRequestURI returns null when original URI has violations #11073

Open
Illapikov opened this issue Dec 15, 2023 · 4 comments
Open
Labels

Comments

@Illapikov
Copy link

Jetty Version
Jetty version: 11.0.18

Java Version
Java version: openjdk version "17.0.9" 2023-10-17

Question

Hello,

We have recently switched from Jetty 9 to Jetty 11 and noticed that behavior of Server's Request has changed, it doesn't return request URI when using Request.getRequestURI method anymore.

The request URI was not valid and caused BadMessageException, however, the request URI is still present in the request's metadata, but not the request itself.

The request itself is the same as in #8148:

curl -i http://localhost:8180/cgi-bin/.%2e/.%2e/.%2e/.%2e/bin/sh -d 'echo;id'

I have seen that has been changed in Jetty 10.x and consequently in 11.x.

I wonder what was the reason and curious what is the harm of doing _uri != null ? _uri : _metadata.getURI() internally in the Request#getRequestURI method?

@gregw
Copy link
Contributor

gregw commented Dec 15, 2023

@Illapikov The changes have been around keeping illegal and ambiguous URI paths out of the servlet API, as they can have security implications if used.
In this case, the URI is not illegal, but is ambiguous once decoded (the % encoding prevents a strict implementation of the URI RFC from detecting the .. elements, but once decoded then those .. elements might be passed to a File, Path or other API from following them). Thus the decision of the servlet spec group was to just not allow such requests into the servlet container.

Do you have a need to see the URI when handling a bad message? what is your use-case?

@Illapikov
Copy link
Author

@gregw, thank you for your explanation.

We are using Logback Access as a logging tool, which internally tries to fetch request URI using HttpServletRequest#getRequestURI`.

For the request above using Jetty 11 we see an access log entry without request URI, while using 9 the request URI was present.

@gregw
Copy link
Contributor

gregw commented Dec 19, 2023

@lachlan-roberts @joakime Do we have a way of configuring the request log to feed into Logback Access in such a way that it is using core APIs and thus can see the bad URI?

@dimas
Copy link

dimas commented Dec 24, 2023

We had a very lengthy conversation with @joakime before. In this issue: #6973
The bottom line was that Logback's implementation of ServerAdapter is not good for modern day Jetty and he actually created a new one for them - qos-ch/logback#532
(We are using this new adapter obviously)

The issue on our side looks like this:

  • we do not have any URI in the logs when request is logged via logback-access
  • there is an URI when request is logged using your standard logging (but we cannot use your logger because we need JSON-formatted logs)

Now the first big question is - should these URIs be logged in the first place? I understand your concerns that they are not safe and should not be made available to servlets. We would not be complaining at all if your own logger did not log the URI but it does which makes everything a bit inconsistent and we want logback-access to log them too.

And what exactly needs to be changed in Logback - should it be changed to take URI from metadata just the way your logger does? This seem like a significant amount of work because Logback's ServerAdapter at the moment does not provide any methods for accessing URI, the interface (and all implementation) need to be extended. With that I see two problems:

  • I am not sure if Logback team is going to accept such a change because in theory it breaks the binary backward compatibility
  • Given the ServletAdapter needs to be universal and work with multiple versions of Jetty - how do we choose when to use metadata vs when to use plain old request.

The first item may also be looked at from the perspective of how Logback is going to integrate with Jetty 12 where, as I was told, you are getting rid of HttpRequest/HttpResponse and these are currently required by logback-access.

I am not entirely sure about what relationship Jetty team has with Logback - was it a one-off contribution or are you willing to maintain Jetty adapter for logback-access. But at least it would be really helpful to get you opinion on the whole thing.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants