GoogleCloudPlatform / appengine-java-standard

Google App Engine Standard Java runtime: Prod runtime, local devappserver, Cloud SDK Java components, GAE APIs, and GAE API emulators.
Apache License 2.0
194 stars 44 forks source link

Java 21 regression bug: devserver (but not the production server) responds with HTTP 403 Forbidden when a securtiy constraint is defined upon a static file (e.g. index.html) #242

Open cyril-briquet opened 3 days ago

cyril-briquet commented 3 days ago

When setting <java21> in appengine-web.xml, and when a security constraint is defined over a static file (e.g. index.html), the devserver (and probably the production server) responds with HTTP 403 instead of serving the file.

Note that setting <java17> in appengine-web.xml is a workaround that prevents the issue.

test-files.txt

cyril-briquet commented 3 days ago

and I forgot to add: ...despite being logged in.

ludoch commented 3 days ago

Devappserver code path is really different than prod. Could you also try to deploy in prod and report? Thanks!

cyril-briquet commented 2 days ago

I deployed the test files (test-files.txt) on the production server. Apparently, the production server is not impacted by this bug.

ludoch commented 2 days ago

Thanks for attaching the repro app, @lachlan-roberts has already some ideas for a fix.

lachlan-roberts commented 2 days ago

Your admin security constraint is redundant as admin is already covered under *.

I have been able to reproduce this, and removing the admin constraint stopped me getting 403's.

I am still investigating why these are combining to not allow the request. In my tests I am not seeing the request for /index.html reach the Jetty security handler at all.

lachlan-roberts commented 1 day ago

I can even reproduce this on prod in the java8 runtime.

The request is not served by Jetty because it is detected as a static file, and I can see in app.yaml. It has defined the required role as admin instead of required which would allow any user.

- url: (/index\.html)
  static_files: __static__\1
  upload: __NOT_USED__
  require_matching_file: True
  login: admin
  secure: always

This is different to how these constrains would combine in the servlet spec which should go to required. So if the request were to reach Jetty it should serve the index.html.