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

[5.3] Routing: Allow to mark parsed URLs as tainted #44455

Merged
merged 3 commits into from
Dec 3, 2024

Conversation

Hackwar
Copy link
Member

@Hackwar Hackwar commented Nov 14, 2024

Summary of Changes

This PR implements a flag in the router to mark parsed URLs as tainted. A tainted URL was able to be parsed, but has errors, for example it is missing the suffix or in a site with enabled IDs the alias of an article is wrong. In those cases the URL could be parsed, but it isn't the correct URL. In that cases we don't have to throw a 404, but can redirect to the correct URL. This is implemented in the SEF system plugin here as well, doing a 301 redirect when the URL is marked as tainted. The benefit is, that we prevent multiple redirects when more than one thing is wrong with the URL (for example missing suffix AND wrong alias for an ID)

This PR was made possible by the support of djumla. Thank you for that.

Testing Instructions

Codereview

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org: Documenting application routing Manual#330

  • No documentation changes for manual.joomla.org needed

@brianteeman
Copy link
Contributor

Just to make sure I understand this correctly.

This would **for example ** detect this url https://brian.teeman.net/joomla/906-full-width-potatoes and do a 301 redirect to https://brian.teeman.net/joomla/906-full-width-joomla-modules

@Hackwar
Copy link
Member Author

Hackwar commented Nov 18, 2024

Correct. And if you disable IDs, it would automatically redirect to brian.teeman.net/joomla/full-width-joomla-modules

@Hackwar
Copy link
Member Author

Hackwar commented Nov 18, 2024

Correction: This PR together with #44477 would do that.

@brianteeman
Copy link
Contributor

Thanks for confirming. Will be able to test these two pr correctly now

@brianteeman
Copy link
Contributor

I have tested this item ✅ successfully on 7d9e478

tested together with #44477
manually typed this url in my browser https://brian.teeman.net/joomla/906-full-width-potatoes.html and it did a 301 redirect to https://brian.teeman.net/joomla/906-full-width-joomla-modules


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/44455.

@SniperSister
Copy link
Contributor

I have tested this item ✅ successfully on 51f23a9

Tested via both, code review and testing the patch together with #44477


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/44455.

@Hackwar Hackwar added RTC This Pull Request is Ready To Commit and removed RTC This Pull Request is Ready To Commit labels Nov 21, 2024
@Hackwar
Copy link
Member Author

Hackwar commented Nov 21, 2024

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/44455.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Nov 21, 2024
@rdeutz rdeutz merged commit 9800738 into joomla:5.3-dev Dec 3, 2024
4 checks passed
@rdeutz
Copy link
Contributor

rdeutz commented Dec 3, 2024

Thanks

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Dec 3, 2024
@rdeutz rdeutz added this to the Joomla! 5.3.0 milestone Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants