WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 5 months ago

#36397 new defect (bug)

add_query_arg doesn't work with numbered html entities

Reported by: omarreiss Owned by:
Milestone: Priority: normal
Severity: normal Version: 2.8
Component: Formatting Keywords: has-patch has-unit-tests dev-feedback
Focuses: Cc:
PR Number:

Description

In #20771 we'd like to use esc_url instead of esc_html to escape the url that is generated by wp_nonce_url. Unfortunately this is currently not possible because add_query_arg has some buggy behavior with regard to its dealing with hashes in urls. I am creating this ticket to deal with that issue separately.

add_query_arg searches for the first hash in a url and cuts everything that comes after it from the url as the hashfragment and appends it back at the end of the operation. There are two problems with this:

  1. No hash found in the url necessarily indicates a hashfragment. It could also indicate a numbered html entity.
  2. If there are multiple hashes in the url, we should probably only look at the last hash present to find a possible hashfragment.

This can for instance become a problem when we useesc_url on a url with more than one parameter. esc_url escapes ampersands by replacing them with their numbered html entity equivalents; #038;

When I now want to use add_query_arg on such a url, the parameters get moved to the end of the url because it thinks everything after the second parameter is a hashfragment.

I am adding a patch with a some passing and some failing testcases that cover this issue. I am also adding a patch that takes care of the issue of multiple hashes in the url and fixes the issue for ampersands, which should unblock #20771 if it were committed.

Attachments (2)

36397.tests.diff (2.7 KB) - added by omarreiss 4 years ago.
Testcases
36397.diff (573 bytes) - added by omarreiss 4 years ago.
Make sure numerically escaped ampersands are not mistaken for hash fragments

Download all attachments as: .zip

Change History (3)

@omarreiss
4 years ago

Testcases

@omarreiss
4 years ago

Make sure numerically escaped ampersands are not mistaken for hash fragments

#1 @omarreiss
4 years ago

  • Keywords has-patch has-unit-tests dev-feedback added
  • Version changed from trunk to 2.8

I am adding a patch with a some passing and some failing testcases that cover this issue. I am also adding a patch that takes care of the issue of multiple hashes in the url and fixes the issue for ampersands, which should unblock #20771 if it were committed.

Note: See TracTickets for help on using tickets.