Skip to content

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

Merged
rdeutz merged 3 commits intojoomla:5.3-devfrom
Hackwar:5.3-router-tainted
Dec 3, 2024
Merged

[5.3] Routing: Allow to mark parsed URLs as tainted#44455
rdeutz merged 3 commits intojoomla:5.3-devfrom
Hackwar:5.3-router-tainted

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.

Co-authored-by: Quy <quy@nomonkeybiz.com>
@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
@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
@Hackwar Hackwar deleted the 5.3-router-tainted branch June 10, 2025 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

Comments