fix(web): make root->dev-ui redirect reverse-proxy-prefix-aware - #1458
Open
luisSilva1234 wants to merge 1 commit into
Open
fix(web): make root->dev-ui redirect reverse-proxy-prefix-aware#1458luisSilva1234 wants to merge 1 commit into
luisSilva1234 wants to merge 1 commit into
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
luisSilva1234
force-pushed
the
fix/redirect-prefix-aware
branch
from
August 26, 2026 08:22
ec5a866 to
08be647
Compare
AdkWebServer.addViewControllers() redirects "/" to the root-relative
path "/dev-ui". A root-relative sendRedirect() target is left untouched
by Spring's own reverse-proxy support (ForwardedHeaderFilter only
applies X-Forwarded-Prefix to context-relative targets, via
StringUtils.applyRelativePath, which it skips whenever the target
already starts with "/"). So when this app is deployed behind a
path-stripping reverse proxy, the browser follows an unprefixed
Location header the proxy has no route for, and the dev UI 404s.
Making the redirect context-relative ("dev-ui") instead lets
server.forward-headers-strategy=framework + an X-Forwarded-Prefix
header from the proxy correctly restore the prefix, with no behavior
change for the common local/non-proxied case (RedirectView's
contextRelative handling, and the servlet container's own relative-URL
resolution, both still resolve "dev-ui" from "/" to "/dev-ui").
Adds AdkWebServerReverseProxyTest, a dedicated test class (separate
from AdkWebServerUITest since server.forward-headers-strategy is a
server-wide property) covering both the no-forwarded-headers baseline
and the X-Forwarded-Prefix case that this fix specifically enables.
luisSilva1234
force-pushed
the
fix/redirect-prefix-aware
branch
from
August 26, 2026 08:56
08be647 to
4681eff
Compare
Contributor
|
Hi @luisSilva1234, thank you for your contribution and We appreciate you taking the time to submit this pull request. Since this is your first contribution, could you please complete the Google CLA so that we can proceed with the review? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1457
Summary
AdkWebServer.addViewControllers()redirects"/"to the root-relative path"/dev-ui". Root-relativesendRedirect()targets are left untouched by Spring's own reverse-proxy support (ForwardedHeaderFilteronly appliesX-Forwarded-Prefixto context-relative targets, viaStringUtils.applyRelativePath, which it skips whenever the target already starts with/). So when this app is deployed behind a path-stripping reverse proxy, the browser follows an unprefixedLocationheader the proxy has no route for, and the dev UI 404s.This PR makes the redirect context-relative (
"dev-ui") instead, soserver.forward-headers-strategy=framework+ anX-Forwarded-Prefixheader from the proxy can correctly restore the prefix.Behavior for the non-proxied/local case
No change:
RedirectView'scontextRelativehandling only prepends the context path when the target starts with/(so a context-relative target is passed through as-is), and the servlet container's own relative-URL resolution forsendRedirectstill resolves"dev-ui"against the current request path"/"to"/dev-ui".Test plan
Added
AdkWebServerReverseProxyTest, covering both cases withserver.forward-headers-strategy=frameworkenabled:"dev-ui") — confirmedForwardedHeaderFilterdoesn't wrap the response at all when the request carries noForwarded/X-Forwarded-*headers, so this matches the property being off entirely.X-Forwarded-Prefix: /my-app-prefix: redirect becomeshttp://.../my-app-prefix/dev-ui, confirming the fix.Also updated
AdkWebServerUITest.rootShouldRedirectToDevUito expect the new context-relative value (MockHttpServletResponse.sendRedirectstores the location verbatim with no resolution, so this assertion needed to change from"/dev-ui"to"dev-ui").Verified locally end-to-end (JDK 17, as required by this repo):
→
BUILD SUCCESS, 1776 core tests + alldevmodule tests (including the new and updated ones) pass, 0 failures.mvn compilewas also run to confirmgoogle-java-formatis happy with the new file (auto-formatted cleanly, no manual formatting issues found).