WordPress.org

Make WordPress Core

Opened 11 days ago

Last modified 10 days ago

#48261 new defect (bug)

"noopener noreferrer" mis-parses links with "rel=" parameters

Reported by: mvandemar Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 5.2.3
Component: Formatting Keywords: has-patch has-unit-tests
Focuses: Cc:
PR Number:

Description (last modified by birgire)

If a link contains a url parameter named "rel" and the rel="noopener noreferrer" attribute is triggered (ie. the link has the target="_blank" attribute as well) then it botches the parsing, and you wind up with a link that looks like:

<a href="https://www.somesite.com/index.php?v=yes&amp;rel=0&quot; noopener noreferrer" target="_blank">Anchor Text</a>

Attachments (6)

noreferrer noopener 48261.png (92.5 KB) - added by adhitya03 10 days ago.
Screenshot from 2019-10-09 06-07-11.png (19.4 KB) - added by mvandemar 10 days ago.
Screenshot from 2019-10-09 06-07-20.png (18.1 KB) - added by mvandemar 10 days ago.
48261.diff (2.5 KB) - added by birgire 10 days ago.
48261-2.diff (2.5 KB) - added by birgire 10 days ago.
48261-3.diff (2.5 KB) - added by birgire 10 days ago.

Download all attachments as: .zip

Change History (13)

#1 @SergeyBiryukov
11 days ago

  • Component changed from General to Formatting
  • Focuses ui removed
  • Keywords needs-patch needs-unit-tests added

#2 @adhitya03
10 days ago

Hi @mvandemar
Thanks for the report, I can't reproduce this issue, but all works fine from my side.

#3 @mvandemar
10 days ago

I just confirmed it is still happening on a vanilla installation of WordPress. Enter this link in html mode, exactly as shown, then hit Publish (or update), then view source on the post/page:

<a href="https://www.somesite.com/index.php?v=yes&rel=0" target="_blank">Anchor Text</a>

@birgire
10 days ago

#4 @birgire
10 days ago

  • Description modified (diff)
  • Keywords has-patch has-unit-tests added; needs-patch needs-unit-tests removed

Thanks @mvandemar for the report and screenshots.

48261.diff is a first iteration, just to help the discussion further.

This patch adds the extra condition preg_match( '#(?:^\s*|\s+)rel\s*=#', $link_html ) if the attributes-regex are matched. Here (?:) is a non-capturing group and we check if rel is the first attribute (that can be without white space from left in $link_html) or the second (or later) attribute (that need a white space from left in $link_html). I first tried to modify the existing attributes-regex with this approach but it ended up with more changes, so I currently prefer this approach.

ps: we could simplify to preg_match( '#(^\s*|\s+)rel\s*=#', $link_html ) as the first version was when I tested it within the attributes-regex, with existing capturing groups.

Last edited 10 days ago by birgire (previous) (diff)

#5 @mvandemar
10 days ago

Confirming that this patch appears to fix the issue on 5.2.3. I also tested the case where there is an existing rel that is not "noopener noreferrer" and it inserts the additional parameters correctly, eg:

<a href="https://www.somesite.com/index.php?v=yes&rel=0" rel="randomrel" target="_blank">Anchor Text</a>

correctly becomes:

<a href="https://www.somesite.com/index.php?v=yes&#038;rel=0" rel="randomrel noopener noreferrer" target="_blank">Anchor Text</a>

@birgire
10 days ago

#6 @birgire
10 days ago

Thanks for testing @mvandemar

48261-2.diff is a slight simplification as mentioned earlier.

@birgire
10 days ago

#7 @birgire
10 days ago

48261-3.diff adds the case-insensitive, as used with regex for the attributes.

Note: See TracTickets for help on using tickets.