Skip to content

fix: emit servlet-relative href for AppShell @StyleSheet#24233

Merged
mcollovati merged 3 commits intomainfrom
appshell-stylesheet-reverse-proxy
May 4, 2026
Merged

fix: emit servlet-relative href for AppShell @StyleSheet#24233
mcollovati merged 3 commits intomainfrom
appshell-stylesheet-reverse-proxy

Conversation

@Artur-
Copy link
Copy Markdown
Member

@Artur- Artur- commented May 1, 2026

AppShellRegistry.resolveStyleSheetHref expanded context://-prefixed @Stylesheet values server-side using request.getContextPath() + "/", producing absolute server paths like that get baked into index.html. This breaks behind reverse proxies that don't preserve the servlet container's context path in the public URL: the server emits /foo/styles.css but the browser fetches it from the public host where /foo/ doesn't exist.

Use service.getContextRootRelativePath(request) instead — the same servlet-relative path (./, ../, etc.) that the bootstrap callback populates into CONTEXT_ROOT_URL for the UIDL path. The resulting href is resolved by the browser against , which Vaadin sets from the actual request URL (honoring X-Forwarded-* headers).

This brings AppShell-level @Stylesheet resolution in line with the component-level UIDL path, which already used the relative form via the client-side URIResolver.

Test fixtures updated to reflect the new servlet-relative hrefs. AppShellRegistryAuraAutoLoadTest had a Mockito mock that returned null for getContextRootRelativePath; it now stubs "./".

Related to #24218.

AppShellRegistry.resolveStyleSheetHref expanded context://-prefixed
@Stylesheet values server-side using request.getContextPath() + "/",
producing absolute server paths like <link href="/foo/styles.css">
that get baked into index.html. This breaks behind reverse proxies
that don't preserve the servlet container's context path in the
public URL: the server emits /foo/styles.css but the browser fetches
it from the public host where /foo/ doesn't exist.

Use service.getContextRootRelativePath(request) instead — the same
servlet-relative path (./, ../, etc.) that the bootstrap callback
populates into CONTEXT_ROOT_URL for the UIDL path. The resulting
href is resolved by the browser against <base>, which Vaadin sets
from the actual request URL (honoring X-Forwarded-* headers).

This brings AppShell-level @Stylesheet resolution in line with the
component-level UIDL path, which already used the relative form via
the client-side URIResolver.

Test fixtures updated to reflect the new servlet-relative hrefs.
AppShellRegistryAuraAutoLoadTest had a Mockito mock that returned
null for getContextRootRelativePath; it now stubs "./".

Related to #24218.
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 1, 2026

Test Results

 1 394 files  ±0   1 394 suites  ±0   1h 15m 5s ⏱️ - 1m 16s
10 069 tests +1   9 999 ✅ +1  70 💤 ±0  0 ❌ ±0 
10 544 runs  +1  10 465 ✅ +1  79 💤 ±0  0 ❌ ±0 

Results for commit 2ec2886. ± Comparison against base commit cce6379.

♻️ This comment has been updated with latest results.

Copy link
Copy Markdown
Collaborator

@mcollovati mcollovati left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to have at least one test that verifies the relative path when custom servlet mapping is used.

@Artur-
Copy link
Copy Markdown
Member Author

Artur- commented May 4, 2026

@Artur-
Copy link
Copy Markdown
Member Author

Artur- commented May 4, 2026

Ah no, it's missing the servlet path

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented May 4, 2026

@Artur- Artur- requested a review from mcollovati May 4, 2026 07:37
Copy link
Copy Markdown
Collaborator

@mcollovati mcollovati left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested with different reverse-proxy configurations, and it works fine

@mcollovati mcollovati added this pull request to the merge queue May 4, 2026
Merged via the queue into main with commit dd7e3e6 May 4, 2026
31 checks passed
@mcollovati mcollovati deleted the appshell-stylesheet-reverse-proxy branch May 4, 2026 08:46
vaadin-bot added a commit that referenced this pull request May 4, 2026
…: 25.1) (#24245)

This PR cherry-picks changes from the original PR #24233 to branch 25.1.
---
#### Original PR description
> AppShellRegistry.resolveStyleSheetHref expanded context://-prefixed
@Stylesheet values server-side using request.getContextPath() + "/",
producing absolute server paths like <link href="/foo/styles.css"> that
get baked into index.html. This breaks behind reverse proxies that don't
preserve the servlet container's context path in the public URL: the
server emits /foo/styles.css but the browser fetches it from the public
host where /foo/ doesn't exist.
> 
> Use service.getContextRootRelativePath(request) instead — the same
servlet-relative path (./, ../, etc.) that the bootstrap callback
populates into CONTEXT_ROOT_URL for the UIDL path. The resulting href is
resolved by the browser against <base>, which Vaadin sets from the
actual request URL (honoring X-Forwarded-* headers).
> 
> This brings AppShell-level @Stylesheet resolution in line with the
component-level UIDL path, which already used the relative form via the
client-side URIResolver.
> 
> Test fixtures updated to reflect the new servlet-relative hrefs.
AppShellRegistryAuraAutoLoadTest had a Mockito mock that returned null
for getContextRootRelativePath; it now stubs "./".
> 
> Related to #24218.
>

Co-authored-by: Artur Signell <artur@vaadin.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants