Skip to content
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

Fixed redirect failure after login #2545

Closed
wants to merge 1 commit into from
Closed

Conversation

@hangxingliu
Copy link

hangxingliu commented Aug 7, 2019

This bug appears on the website isn't hosted on port 80/443.
Use HTTP_HOST to replace the SERVER_NAME.

For example, The $_SERVER['SERVER_NAME'] maybe 127.0.0.1 when the website is hosted on 127.0.0.1:8008. But $_SERVER['HTTP_HOST'] will be 127.0.0.1:8008 in this situation.

@dgw

This comment has been minimized.

Copy link
Member

dgw commented Aug 8, 2019

Don't revert #2542, please.

This looks like it will always add :port to the URL if $_SERVER['HTTP_HOST'] is not set and the code has to fall back on $_SERVER['SERVER_NAME']. We really don't want or need it for vanilla http:// on port 80 and https:// on 443. It's more likely to confuse users than not.

@hangxingliu hangxingliu force-pushed the hangxingliu:master branch from 8cbccca to 6ab607d Aug 8, 2019
@hangxingliu

This comment has been minimized.

Copy link
Author

hangxingliu commented Aug 8, 2019

Don't revert #2542, please.

Sorry, I forgot to pull the upstream code before modify.

And I modify the code around generating redirect URL. It would not add :80 and :443 anymore.

@ozh

This comment has been minimized.

Copy link
Member

ozh commented Feb 21, 2020

Back (after so long, sorry) at this : I'm not sure why we're not simply using YOURLS_SITE instead of this complex $_SERVER['SERVER_NAME'] juggling.

If anyone here still alive : #2603 (comment)

ozh added a commit that referenced this pull request Mar 22, 2020
This all boils down to redirecting to $_SERVER['REQUEST_URI'] after successful login
@ozh

This comment has been minimized.

Copy link
Member

ozh commented Mar 22, 2020

Trying to make things even simpler. I'm closing this PR and looking forward to fixing things with PR #2625, please report there

@ozh ozh closed this Mar 22, 2020
ozh added a commit that referenced this pull request Mar 25, 2020
* Attempt at fixing #2603 and superseding #2545

This all boils down to redirecting to $_SERVER['REQUEST_URI'] after successful login
Thanks @hangxingliu and @pcolmer for insightful feedbacks which has led to this PR

* Refactor slightly the redirect function

- no dying
- no output if we're on cli

* Update tests/includes/utils.php

Co-Authored-By: Léo Colombaro <git@colombaro.fr>

* Update includes/functions.php

Co-Authored-By: Léo Colombaro <git@colombaro.fr>

* Rewrite yourls_get_request()

Do not rely on HOSTNAME, HTTP_HOST or whatever, just use YOURLS_SITE

* Remove leftover comment

[skip ci] [skip scrutinizer]

Co-authored-by: Léo Colombaro <git@colombaro.fr>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.