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

JStringPunycode methods strip # (fragment) element from URL #4362

Closed
wants to merge 1 commit into from

Conversation

alexvanniel
Copy link
Contributor

This change fixes the issue where an url processed by the JStringPunycode class gets it's fragment element stripped. Since this code is widely used, any url submitted through a form in which the field type is set to url, validated as an url and filtered as an url will have it's fragment part stripped thus making it impossible to link to a specific anchor on a page. This is a pull request that fixes the issue: http://issues.joomla.org/tracker/joomla-cms/4357

Steps to reproduce the issue

Easy reproduction of this behaviour can be achieved by opening the Weblinks component in de Administrator, create a new Weblink (or edit an existing Weblink) and add #sometag to the end of the url in the URL field. For instance: http://www.google.nl/index.php#anchor

Expected result

The expected result would be a Weblink with the # part still in tact. Like with the google example the url would remain: http://www.google.nl/index.php#anchor

Actual result

Instead of the expected result, the url get's "sanitized" and looses the # element. In the example of the google url it results in http://www.google.nl/index.php where everything after the # is removed.

System information (as much as possible)

  • Joomla 3.3.4 Stable [ Ember ] 23-September-2014 14:00 GMT
    Joomla Platform 13.1.0 Stable [ Curiosity ] 24-Apr-2013 00:00 GMT
    Ubuntu 14.04
    PHP 5.5.9
    Apache 2.4.7
  • Joomla 3.3.4 Stable [ Ember ] 23-September-2014 14:00 GMT
    Joomla Platform 13.1.0 Stable [ Curiosity ] 24-Apr-2013 00:00 GMT
    Windows Server 2008
    PHP 5.3.15
    IIS7

Additional comments

To be clear, this is NOT an issue with the com_weblinks component since it is also something that happens with components we build ourselves from scratch.

It looks like the fragment part is not reassembled when JStringPunycode::urlToUTF8() or JStringPunycode::urlToPunycode() is executed. In both methods the scheme, port, path and query are put back in the $newuri variable that is returned but the fragment part is left out.

A possible fix would be to add the following three lines to the end of both methods:

if (!empty($parsed['fragment']))
{
    $newuri .= '#' . $parsed['fragment'];
}

This change fixes the issue where an url processed by the JStringPunycode class gets it's fragment element stripped. Since this code is widely used, any url submitted through a form in which the field type is set to url, validated as an url and filtered as an url will have it's fragment part stripped thus making it impossible to link to a specific anchor on a page. This is a pull request that fixes the issue: http://issues.joomla.org/tracker/joomla-cms/4357
@infograf768
Copy link
Member

@test
tested with UTF8 urls and works fine.
One more tester.

@peterlose
Copy link
Contributor

@test All good! I'm now able to save URL's with anchors

@Kubik-Rubik
Copy link
Member

@alexvanniel This is the correct solution, well done!

RTC

@jissues-bot jissues-bot changed the title Update punycode.php JStringPunycode methods strip # (fragment) element from URL Sep 27, 2014
@mbabker
Copy link
Contributor

mbabker commented Sep 27, 2014

Merged, thanks!

@mbabker mbabker closed this in bff3a3d Sep 27, 2014
mbabker added a commit that referenced this pull request Sep 27, 2014
@mbabker mbabker added this to the Joomla! 3.3.5 milestone Sep 27, 2014
piotr-cz pushed a commit to piotr-cz/joomla-cms that referenced this pull request Oct 6, 2014
piotr-cz pushed a commit to piotr-cz/joomla-cms that referenced this pull request Oct 6, 2014
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

7 participants