-
Notifications
You must be signed in to change notification settings - Fork 2.7k
fix: embedded UI redirect error when behind proxy #12566
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
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. |
There was a problem hiding this comment.
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.
I reran the tests. Also, I agree with @bzp2010 's opinion. |
dcf7bb5
to
8fb297e
Compare
7930b6b
to
8fb297e
Compare
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. |
There was a problem hiding this comment.
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.
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. |
@feranwq The test for this mcp-bridge often fails. I've rerun it for you. |
Description
add absolute_redirect off; in /ui location to fix redirect port err
Which issue(s) this PR fixes:
Fixes #12511
Checklist