-
Notifications
You must be signed in to change notification settings - Fork 9
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
fix(jetty12): more align RequestURI attr creation with that of Jetty 11 #7
fix(jetty12): more align RequestURI attr creation with that of Jetty 11 #7
Conversation
Provide the path part of the URI in raw format. Signed-off-by: You Aoki <165284116+marinedayo@users.noreply.github.com>
b4a0fb0
to
8f01a11
Compare
@@ -68,7 +68,7 @@ public void getRequest() throws Exception { | |||
|
|||
@Test | |||
public void eventGoesToAppenders() throws Exception { | |||
URL url = new URL(JETTY_FIXTURE.getUrl()); | |||
URL url = new URL("http://localhost:" + RANDOM_SERVER_PORT + "/path/foo%20bar;param?query#fragment"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
optional: How about leaving it as it was to maintain the same entry point?
URL url = new URL("http://localhost:" + RANDOM_SERVER_PORT + "/path/foo%20bar;param?query#fragment"); | |
URL url = new URL(JETTY_FIXTURE.getUrl() + "path/foo%20bar;param?query#fragment"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your reply.
53b341a done.
@@ -68,7 +68,7 @@ public void getRequest() throws Exception { | |||
|
|||
@Test | |||
public void eventGoesToAppenders() throws Exception { | |||
URL url = new URL(JETTY_FIXTURE.getUrl()); | |||
URL url = new URL("http://localhost:" + RANDOM_SERVER_PORT + "/path/foo%20bar;param?query#fragment"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the same as above
Signed-off-by: You Aoki <165284116+marinedayo@users.noreply.github.com>
Hi @ceki , could you please review and merge? Thanks. |
Thank you. |
Ref: #4 (comment)
Why
By the following pull request, now provides the path part of the URI in Jetty 12 as well.
However,
HttpURI::getDecodedPath
method returns a decoded path, so the behavior is different from other environments, such as Jetty 11.What
HttpURI::getPath
method instead, which returns the raw format