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 double port in error pages base URL #4518

Merged

Conversation

massa-man
Copy link
Contributor

Description (*)

This fixes an issue where the base URL injected into the error page HTML HEAD includes the port twice, and breaks the CSS and image imports. This happens because the code uses HTTP_HOST server variable that already includes the port if the browser URL has a port.

Manual testing scenarios (*)

  1. Deploy OpenMage in a server on a port other than 80 or 443, for example http://localhost:8080
  2. Go to an error page, such as http://localhost:8080/errors/404.php
  3. You will notice in the HTML, the <head> the <base> element has a URL like http://localhost:8080:8080/errors/default/

Questions or comments

The code uses substr_compare instead of str_ends_with for php 7 compatibility.

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All automated tests passed successfully (all builds are green)

@github-actions github-actions bot added the errors Relates to error pages label Jan 31, 2025
@sreichel
Copy link
Contributor

The code uses substr_compare instead of str_ends_with for php 7 compatibility.

We have polyfills for php8, you can use str_ends_with.

PS: please run composer run php-cs-fixer:fix

@massa-man
Copy link
Contributor Author

Thanks for the feedback, i run CS fixer and changed the code to use str_ends_with

Copy link

sonarqubecloud bot commented Feb 1, 2025

@sreichel sreichel added the bug label Feb 2, 2025
@sreichel sreichel merged commit e8c49e6 into OpenMage:main Feb 2, 2025
17 checks passed
@sreichel
Copy link
Contributor

sreichel commented Feb 2, 2025

@all-contributors add @massa-man code

Copy link
Contributor

@sreichel

@massa-man already contributed before to code

@sreichel sreichel added this to the 20.13.0 milestone Feb 2, 2025
@sreichel
Copy link
Contributor

sreichel commented Feb 2, 2025

Thanks.

sreichel added a commit that referenced this pull request Feb 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug errors Relates to error pages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants