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

Convert the system Redirect plugin to use lowercase urls at all times… #13853

Merged
merged 3 commits into from
Feb 4, 2017

Conversation

tonypartridge
Copy link
Contributor

@tonypartridge tonypartridge commented Feb 2, 2017

Pull Request for Issue # .

Summary of Changes

Converted the Redirect System Plugin to store in lowercase and match redirect on lowercase. I've gone for strtolower since it's url based. But we could replace it with mb_strtolower for UTF-8 standards if you think it is needed but it is slower.

Testing Instructions

Create an article on your website say: my-article and access it in the frontend. Perfect. remove e from it so change the url to say: mywebsite.com/inde.php/my-articl and now you get a 404, next change it to mywebsite.com/inde.php/My-articl and you get a 404. But you now have two redirects to do in the redirects component.

Expected result

upper and lowercase 404 urls show be equal since J! stores menu item aliases in lowercase.

Actual result

both lowercase and uppercase 404's are stored and matched independently.

Documentation Changes Required

Resolves #12666

@ghost
Copy link

ghost commented Feb 2, 2017

guess testing on legacy routing?

@tonypartridge
Copy link
Contributor Author

tonypartridge commented Feb 2, 2017

Testing is for Default router, not the new Router since that is disabled by default and doesn't appear to work with the redirects component.

@ghost
Copy link

ghost commented Feb 4, 2017

I have tested this item ✅ successfully on deae8cd

Legacy Routing, the Redirect Plugin is enabled. The option 'Collect URLs' is enabled.

With PR:

Original-URL: index.php/sample-sites

  • index.php/sample-site > 404
  • index.php/sample-Site > 404

One disabled Redirect index.php/sample-site

Same without PR got two Redirect

  • index.php/sample-site
  • index.php/sample-Site

Test on:

Joomla! 3.7.0-beta1
macOS Sierra, 10.12.3
Firefox 50.1.0
PHP 7.0.4
MySQLi 5.5.53-0


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

@zero-24
Copy link
Contributor

zero-24 commented Feb 4, 2017

I have tested this item ✅ successfully on deae8cd

👍 Thanks


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

@zero-24 zero-24 added this to the Joomla 3.7.0 milestone Feb 4, 2017
@zero-24 zero-24 added the RTC This Pull Request is Ready To Commit label Feb 4, 2017
@zero-24
Copy link
Contributor

zero-24 commented Feb 4, 2017

RTC #jc17de


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

@rdeutz rdeutz merged commit 0327087 into joomla:staging Feb 4, 2017
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Feb 4, 2017
@brianteeman
Copy link
Contributor

Please don't comment on a closed issue. It won't be seen. Please create a new issue and reference this

@tonypartridge
Copy link
Contributor Author

As brain said please open a new issue.

The problem with your scenario is it is not directly related to Joomla! This change follows Joomla! Standards directly, where uppercase characters are not used within URLs and the redirects component is for this. Plus you couldn't create a redirect uppercase and lowercase as it conflicts its self due to the way joomla handles it.

It is also good practice to not have uppercase characters in urls.

In your scenario you could create a little custom plugin which bypasses this, or use one of the many sef components.

@josarnold
Copy link

ok, thanks

@tonypartridge
Copy link
Contributor Author

You are welcome, it may also be worth pointing or most modern web servers treat upper and lower case equally and they are no longer unique see:

https://github.com/joomla/joomla-cmS

Is the same as

https://github.com/joomla/joomla-cms

You may want to look at update the server and add the required module to handle this accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants