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

301 Moved Permanently produces query with ; instead of ? #10794

Closed
janitza-bjag opened this issue Oct 26, 2023 · 5 comments · Fixed by #10796
Closed

301 Moved Permanently produces query with ; instead of ? #10794

janitza-bjag opened this issue Oct 26, 2023 · 5 comments · Fixed by #10796
Assignees
Labels
Bug For general bugs on Jetty side
Milestone

Comments

@janitza-bjag
Copy link

Jetty Version 12.0.2

Jetty Environment ee8

Java Version 17

Question
Hi,

we used a ServletContextHandler to map the context path "foo" to static ressources with DefaultServlet.class
Before we used Jetty 10 and the URL "localhost/foo?q=bar" worked as expected.
Now with Jetty 12 the same URL is redirected to "localhost/foo/;q=bar" with http code 301 ...

If we use "localhost/foo/?q=bar" it worked correctly ...

Does anyone know whats going on with "?" and "/?" ?

with best regards

@joakime
Copy link
Contributor

joakime commented Oct 26, 2023

A context path of /foo means that your resources are served at /foo/.

Eg: You could also have a root context at / and a file resource called foo.
If you request http://localhost/foo what are you asking for? (the file resource foo in the root context? or the top level of the /foo context?)

This is why the Servlet spec auto redirects a context of /blah to /blah/ when it sees it.
This happens in Jetty 10 too btw. (even happens as far back as Jetty 5 to be honest)

@janitza-bjag
Copy link
Author

A context path of /foo means that your resources are served at /foo/.

Eg: You could also have a root context at / and a file resource called foo. If you request http://localhost/foo what are you asking for? (the file resource foo in the root context? or the top level of the /foo context?)

This is why the Servlet spec auto redirects a context of /blah to /blah/ when it sees it. This happens in Jetty 10 too btw. (even happens as far back as Jetty 5 to be honest)

In our case we have no ressources in the root context, so every "/xyz" should be its own context.

The redirect from "/blah" to "/blah/" is fine, but the redirect from "/blah?q=test" to "/blah/;q=test" is not.
If it would be "/blah/?q=test" everything were nice, but I dont know where the semicolon comes from ...

@joakime
Copy link
Contributor

joakime commented Oct 26, 2023

Steps to replicate this issue.

Download the Jetty Home tarball

[distros]$ curl -O https://repo1.maven.org/maven2/org/eclipse/jetty/jetty-home/12.0.2/jetty-home-12.0.2.tar.gz
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 31.9M  100 31.9M    0     0  33.0M      0 --:--:-- --:--:-- --:--:-- 33.0M
[distros]$ tar -zxf jetty-home-12.0.2.tar.gz 
[distros]$

Setup the Jetty Base with ee8 demos

[bases]$ mkdir demos-12
[demos-12]$ java -jar ~/code/jetty/distros/jetty-home-12.0.2/start.jar --add-modules=http,deploy,ee8-demos
INFO  : mkdir ${jetty.base}/start.d
INFO  : alpn-java       transitively enabled
INFO  : demo-realm      transitively enabled, ini template available with --add-modules=demo-realm
...(snip lots of output)...
INFO  : copy ${jetty.home}/modules/demo.d/demo-login.properties to ${jetty.base}/etc/demo-login.properties
INFO  : Base directory was modified
[demos-12]$

Run Jetty

[demos-12]$ java -jar ~/code/jetty/distros/jetty-home-12.0.2/start.jar 
2023-10-26 11:18:20.489:WARN :oejk.KeystoreGenerator:main: Generating Test Keystore: DO NOT USE IN PRODUCTION!
2023-10-26 11:18:21.049:WARN :oejx.XmlConfiguration:main: Deprecated method public void org.eclipse.jetty.security.HashLoginService.setHotReload(boolean) in file:///home/joakim/code/jetty/distros/bases/demos-12/etc/jetty-demo-realm.xml
2023-10-26 11:18:21.051:WARN :oe.jetty:main: demo-realm is deployed. DO NOT USE IN PRODUCTION!
2023-10-26 11:18:21.140:INFO :oejs.Server:main: jetty-12.0.2; built: 2023-10-10T01:59:47.552Z; git: b01e3611cfcec50c9157c50f6d32a93515e01510; jvm 17.0.6+10
...(snip lots of output)...
webapp/,a=AVAILABLE,h=oeje8n.ContextHandler$CoreContextHandler$CoreToNestedHandler@2c7a8af2{STARTED}}
2023-10-26 11:18:23.526:INFO :oejs.AbstractConnector:main: Started ServerConnector@eca6a74{SSL, (ssl, alpn, h2, http/1.1)}{0.0.0.0:8443}
2023-10-26 11:18:23.532:INFO :oejs.AbstractConnector:main: Started ServerConnector@4f66ffc8{HTTP/1.1, (http/1.1)}{0.0.0.0:8080}
2023-10-26 11:18:23.541:INFO :oejs.Server:main: Started oejs.Server@4bff64c2{STARTING}[12.0.2,sto=5000] @4040ms

Test behavior of GET to resource served by DefaultServlet with query param.

[tmp]$ curl -vvv http://localhost:8080/ee8-test?a=b
*   Trying 127.0.0.1:8080...
* Connected to localhost (127.0.0.1) port 8080 (#0)
> GET /ee8-test?a=b HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/7.81.0
> Accept: */*
> 
* Mark bundle as not supporting multiuse
< HTTP/1.1 301 Moved Permanently
< Server: Jetty(12.0.2)
< Set-Cookie: visited=yes
< Expires: Thu, 01 Jan 1970 00:00:00 GMT
< Location: /ee8-test/;a=b
< Content-Length: 0
< 
* Connection #0 to host localhost left intact
[tmp]$

Behavior Confirmed!

Test scope of issue

Lets see if this is true in ee9 / ee10 too. (with the ee9-demos and ee10-demos enabled as well)

Yup, present on EE9 as well.

[tmp]$ curl -vvv http://localhost:8080/ee9-test?a=b
*   Trying 127.0.0.1:8080...
* Connected to localhost (127.0.0.1) port 8080 (#0)
> GET /ee9-test?a=b HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/7.81.0
> Accept: */*
> 
* Mark bundle as not supporting multiuse
< HTTP/1.1 301 Moved Permanently
< Server: Jetty(12.0.2)
< Set-Cookie: visited=yes
< Expires: Thu, 01 Jan 1970 00:00:00 GMT
< Set-Cookie: visited=yes
< Set-Cookie: visited=yes
< Location: /ee9-test/;a=b
< Content-Length: 0
< 
* Connection #0 to host localhost left intact

Yup, present on EE10 as well.

[tmp]$ curl -vvv http://localhost:8080/ee10-test?a=b
*   Trying 127.0.0.1:8080...
* Connected to localhost (127.0.0.1) port 8080 (#0)
> GET /ee10-test?a=b HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/7.81.0
> Accept: */*
> 
* Mark bundle as not supporting multiuse
< HTTP/1.1 301 Moved Permanently
< Server: Jetty(12.0.2)
< Set-Cookie: visited=yes
< Expires: Thu, 01 Jan 1970 00:00:00 GMT
< Set-Cookie: visited=yes
< Set-Cookie: visited=yes
< Location: /ee10-test/;a=b
< Content-Length: 0
< 
* Connection #0 to host localhost left intact

@joakime
Copy link
Contributor

joakime commented Oct 26, 2023

Opened PR #10796 to fix this

joakime added a commit that referenced this issue Oct 26, 2023
…ith-query

Issue #10794 - fixing Moved Permanently handling of query strings
@joakime
Copy link
Contributor

joakime commented Oct 26, 2023

Merged PR #10796 into jetty-12.0.x.

@joakime joakime closed this as completed Oct 26, 2023
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in Jetty 12.0.3 - FROZEN Oct 26, 2023
@joakime joakime changed the title Weird rewrite with "foo?q=bar" 301 Moved Permanently produces query with ; instead of ? Oct 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side
Projects
No open projects
Status: ✅ Done
2 participants