Skip to content

fix: embedded UI redirect error when behind proxy#12566

Merged
moonming merged 2 commits into
apache:masterfrom
feranwq:ui_redirect_fix
Sep 17, 2025
Merged

fix: embedded UI redirect error when behind proxy#12566
moonming merged 2 commits into
apache:masterfrom
feranwq:ui_redirect_fix

Conversation

@feranwq

@feranwq feranwq commented Aug 29, 2025

Copy link
Copy Markdown
Contributor

Description

add absolute_redirect off; in /ui location to fix redirect port err

Which issue(s) this PR fixes:

Fixes #12511

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

@dosubot dosubot Bot added size:XS This PR changes 0-9 lines, ignoring generated files. bug Something isn't working labels Aug 29, 2025
Baoyuantop
Baoyuantop previously approved these changes Aug 29, 2025
View reviewed changes
@juzhiyuan

juzhiyuan commented Aug 29, 2025

Copy link
Copy Markdown
Member

Hi I notice there are some failing tests

@feranwq

feranwq commented Aug 29, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Hi I notice there are some failing tests

I've reviewed the error and it doesn't appear to be related to my PR. However, I can't be certain whether the err with the pre-existing code or the test itself.

bzp2010
bzp2010 requested changes Aug 30, 2025
View reviewed changes

@bzp2010 bzp2010 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Test case needs to be added to verify that the Location response header meets expectations.

Comment thread apisix/cli/ngx_tpl.lua
@SkyeYoung

SkyeYoung commented Sep 1, 2025

Copy link
Copy Markdown
Member

I reran the tests. Also, I agree with @bzp2010 's opinion.

@dosubot dosubot Bot added size:S This PR changes 10-29 lines, ignoring generated files. and removed size:XS This PR changes 0-9 lines, ignoring generated files. labels Sep 1, 2025
@bzp2010

bzp2010 commented Sep 2, 2025

Copy link
Copy Markdown
Contributor

Please do not force push after the review has begun; this wastes our time. 🤨

I believe the PR description already clearly explains this matter. If you do not believe this PR is ready for review, please place it in draft status until you have completed it and marked it as ready to review.

@feranwq

feranwq commented Sep 2, 2025

Copy link
Copy Markdown
Contributor Author

Please do not force push after the review has begun; this wastes our time. 🤨

I believe the PR description already clearly explains this matter. If you do not believe this PR is ready for review, please place it in draft status until you have completed it and marked it as ready to review.

Sorry, I'm used to using rebase to keep the commit history clean. I'll be mindful of this for any future modifications.

bzp2010
bzp2010 approved these changes Sep 2, 2025
View reviewed changes

@bzp2010 bzp2010 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This project always merges PRs using squash merges, so the commit history isn't very important.

@bzp2010 bzp2010 changed the title fix: fix ui redirect error when behind proxy (#12511) fix: embedded UI redirect error when behind proxy (#12511) Sep 2, 2025
@SkyeYoung SkyeYoung changed the title fix: embedded UI redirect error when behind proxy (#12511) fix: embedded UI redirect error when behind proxy Sep 3, 2025
@Baoyuantop Baoyuantop moved this to 👀 In review in ⚡️ Apache APISIX Roadmap Sep 9, 2025
@Baoyuantop

Baoyuantop commented Sep 15, 2025

Copy link
Copy Markdown
Contributor

Hi @feranwq, there's a failed test that needs to be resolved. I haven't identified the specific cause of the issue. Maybe you could merge the master branch and try again?

@feranwq

feranwq commented Sep 16, 2025

Copy link
Copy Markdown
Contributor Author

Hi @feranwq, there's a failed test that needs to be resolved. I haven't identified the specific cause of the issue. Maybe you could merge the master branch and try again?

It failed again, this time it was an mcp test failure. I've seen similar failures frequently in other pr, please help take a look.

@SkyeYoung

SkyeYoung commented Sep 16, 2025

Copy link
Copy Markdown
Member

@feranwq The test for this mcp-bridge often fails. I've rerun it for you.

@moonming
moonming merged commit 93901a9 into apache:master Sep 17, 2025
36 of 39 checks passed
@github-project-automation github-project-automation Bot moved this from 👀 In review to ✅ Done in ⚡️ Apache APISIX Roadmap Sep 17, 2025
jizhuozhi pushed a commit to jizhuozhi/apisix that referenced this pull request Oct 18, 2025
@feranwq @jizhuozhi
fix: fix ui redirect error when behind proxy (apache#12566)
5221cf4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bzp2010 bzp2010 bzp2010 approved these changes
@Baoyuantop Baoyuantop Baoyuantop approved these changes
@SkyeYoung SkyeYoung SkyeYoung approved these changes
@juzhiyuan juzhiyuan Awaiting requested review from juzhiyuan

Assignees

@feranwq feranwq

Labels

bug Something isn't working size:S This PR changes 10-29 lines, ignoring generated files.

Projects

⚡️ Apache APISIX Roadmap
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

help request: Admin ui Redirected

6 participants

@feranwq @juzhiyuan @SkyeYoung @bzp2010 @Baoyuantop @moonming