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

Fix issue with non-standard ssl ports being removed from site URL #16087

Merged
merged 1 commit into from
Mar 29, 2022

Conversation

smg6511
Copy link
Collaborator

@smg6511 smg6511 commented Mar 5, 2022

What does it do?

Ensures that non-standard ports persist in the site_url.

Why is it needed?

Currently, when using a non-standard port for https (any one other than 443), the port gets stripped from the site's URL, making it impossible to properly navigate the site. Only the standard ports of 80 and 443 should be stripped.

How to test

Configure the testing site on standard and non-standard ports, both with https on and off. Verify that setup, subsequent usage of the manager, and front end browsing all work as expected (with non-standard ports being maintained in the URL and 80/443 being stripped out).

Related issue(s)/PR(s)

Resolves #16015

@smg6511 smg6511 requested review from opengeek and Mark-H as code owners March 5, 2022 04:42
@cla-bot cla-bot bot added the cla-signed CLA confirmed for contributors to this PR. label Mar 5, 2022
@smg6511 smg6511 changed the title Ensure non-standard ports are maintained in URL Fix issue with non-standard ssl ports being removed from site URL Mar 5, 2022
@Ibochkarev Ibochkarev added the pr/review-needed Pull request requires review and testing. label Mar 19, 2022
@Ibochkarev Ibochkarev added this to the v3.0.0 milestone Mar 19, 2022
@opengeek opengeek merged commit 0820e6d into modxcms:3.x Mar 29, 2022
@smg6511 smg6511 deleted the 3.x-issue-16015 branch June 4, 2022 04:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed CLA confirmed for contributors to this PR. pr/review-needed Pull request requires review and testing.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants