Skip to content

Add CONTEXT_PATH support for sub-path deployments#3944

Closed
spike83 wants to merge 2 commits intomapfish:masterfrom
spike83:issue-3728-context-path
Closed

Add CONTEXT_PATH support for sub-path deployments#3944
spike83 wants to merge 2 commits intomapfish:masterfrom
spike83:issue-3728-context-path

Conversation

@spike83
Copy link
Copy Markdown
Contributor

@spike83 spike83 commented Jan 22, 2026

Implements issue #3728: allow running behind a reverse proxy under a sub path by configuring servlet mappings and UI base href via CONTEXT_PATH.

Implements issue mapfish#3728: allow running behind a reverse proxy under a sub path by configuring servlet mappings and UI base href via CONTEXT_PATH.

This comment was marked as outdated.

@sebr72
Copy link
Copy Markdown
Contributor

sebr72 commented Jan 22, 2026

@spike83 I have assign to review your PR. But it is not successfully building. Could you please fix you PR, then I will review it.

@sebr72
Copy link
Copy Markdown
Contributor

sebr72 commented Jan 22, 2026

@sbrunner Could we do sthg about Pull request links that never stops running ?

@spike83 spike83 force-pushed the issue-3728-context-path branch from eabe877 to 901814b Compare January 23, 2026 05:27
@sbrunner
Copy link
Copy Markdown
Member

sbrunner commented Jan 23, 2026

@sebr72 I just blacklist 'issue' to be a Jira project.

@sbrunner sbrunner closed this Feb 10, 2026
@sbrunner sbrunner reopened this Feb 10, 2026
@sbrunner
Copy link
Copy Markdown
Member

I just closed and reopened it to be sure that the integration tests are passing.

@sbrunner sbrunner enabled auto-merge February 10, 2026 15:00
@sbrunner sbrunner disabled auto-merge February 10, 2026 15:01
@sbrunner sbrunner requested a review from Copilot February 10, 2026 15:01
Copy link
Copy Markdown
Contributor

@sebr72 sebr72 left a comment

Choose a reason for hiding this comment

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

Good work. Thanks.
@spike83 : The build failing was due to the usage of the word issue in your branch name. We fixed it.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 2 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +34 to +35
Prefix the servlet mappings (for example <code>/metrics</code> becomes <code>/print/metrics</code>).
</li>
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

The documentation example is slightly misleading. It states that /metrics becomes /print/metrics, which is correct. However, it doesn't clarify what happens to the existing /print/* servlet mapping. With CONTEXT_PATH=/print, the servlet mapping /print/* would become /print/print/*, meaning the print API would be accessible at /print/print/... rather than just /print/.... This could confuse users.

Consider adding a note that the CONTEXT_PATH should not conflict with existing servlet paths (/print or /sec/print), or document the resulting double-path behavior explicitly.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@copilot open a new pull request to apply changes based on this feedback

@sebr72
Copy link
Copy Markdown
Contributor

sebr72 commented Feb 11, 2026

@spike83 Without reply from you, and without sufficient test, we tried to get copilot to add some and it could not not). So we are closing this PR and creating #3978
Your feedback will be more than welcome

@sebr72 sebr72 closed this Feb 11, 2026
auto-merge was automatically disabled February 11, 2026 15:43

Pull request was closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants