-
Notifications
You must be signed in to change notification settings - Fork 903
Make AIO interface not dependent on absolute location #6867
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
base: main
Are you sure you want to change the base?
Conversation
|
Hm, I tired to squash the commits but it looks like I did something wrong... I hope this doesn't create confusion. |
|
First of all, thanks for creating this PR!
Squashing is not necessarily needed but can you please rebase the branch from main? This would be important. Thanks in advance! |
ebea0e8 to
e4a0e1f
Compare
|
Rebased! |
5643f2b to
5190855
Compare
|
Today, with the release of v11.8.0, I had the chance to test also the update process, and everything went smoothly! |
|
Thanks a lot for your effort! Just to be clear: I try to review it in the coming 1-2 weeks and try to include it for the next major version which is planned to be released in ca 1 month. |
Signed-off-by: Lorenzo Moscati <[email protected]>
Signed-off-by: Lorenzo Moscati <[email protected]>
5190855 to
a019fbf
Compare
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.
Hey, I finally had the time to have a look at this and have some comments below.
Have you already tested your changes btw and ensured that a default installation in a root dir still works?
Signed-off-by: Lorenzo Moscati <[email protected]>
Signed-off-by: Lorenzo Moscati <[email protected]>
a019fbf to
72942a1
Compare
…-interface-relative
Signed-off-by: Lorenzo Moscati <[email protected]>
|
Hello, thanks for finding the time to check it out. I understand that the relative "Location" header can be confusing at times, especially since it's kind of a new feature: the HTTP spec has been updated in June 2014 to allow relative URIs in the "Location" header RFC 7231 Let me know what you think about the improved logic in the "Auth" middleware: I made the root scenario the first if case and added comments to explain the logic. |
As a followup from #6782, here are the proposed changes to make AIO interface work relative to the browser location, without assuming the interface resides on the root path of the domain.
The changes can be summarised in:
I tested every basic usage, including unauthenticated situations (which require a particular logic, as seen in the AuthMiddleware.php file) and modifying the settings.