Make WordPress Core

Opened 12 years ago

Closed 11 years ago

Last modified 11 years ago

#31632 closed defect (bug) (fixed)

Don't strip newline in esc_url() when protocol is mailto:

Reported by: danielbachhuber Owned by: danielbachhuber
Priority: normal Milestone: 4.3
Component: Formatting Version:
Severity: normal Keywords: has-patch
Cc: Focuses:

Description

esc_url() strips encoded newlines when it shouldn't.

The genesis of newline stripping is r4148. mailto was added as a mentioned protocol in r8743, but it doesn't appear there was any technical change, only the PHPdoc mention.

Here's the failing testcase:

$body = <<<EOT
Hi there,

I thought you might want to sign up for this newsletter
EOT;
$email_link = 'mailto:?body=' . rawurlencode( $body );

// Expected: mailto:?body=Hi%20there%2C%0A%0AI%20thought%20you%20might%20want%20to%20sign%20up%20for%20this%20newsletter
var_export( $email_link );

// Actual: mailto:?body=Hi%20there%2CI%20thought%20you%20might%20want%20to%20sign%20up%20for%20this%20newsletter
$email_link = esc_url( $email_link );
var_export( $email_link );

Attachments (1)

31632.1.diff​ (1.6 KB ) - added by danielbachhuber 12 years ago.
Don't strip newline in esc_url when protocol is mailto:

Download all attachments as: .zip

Change History (12)

@danielbachhuber
12 years ago

Don't strip newline in esc_url when protocol is mailto:

This ticket was mentioned in ​Slack in #core by danielbachhuber. ​View the logs.


12 years ago

#2 @danielbachhuber
12 years ago

  • Keywords has-patch 4.3-early added

#3 @obenland
11 years ago

  • Owner set to danielbachhuber
  • Status new → assigned

#4 @danielbachhuber
11 years ago

  • Milestone Future Release → 4.3

#5 @obenland
11 years ago

  • Keywords 4.3-early removed

#6 @dd32
11 years ago

This feels like a wontfix edgecase to me..

This ticket was mentioned in ​Slack in #core by jorbin. ​View the logs.


11 years ago

#8 @jorbin
11 years ago

I think that this is an edgecase, but one worth supporting. mailto is an acceptable protocal to be run through esc_url and it allows new lines. The fact that this contains unit tests means that the cost of the supporting the edge case is low.

#9 @dd32
11 years ago

I'm fine with letting it in if others feel strongly, but I still think it's an significant edge-case that doesn't realistically need to be supported.

The unit tests should be expanded to cover other use-cases as well. It should show it in action, and that it still works as intended for other url forms (assuming that there isn't another test that handles that already)

#10 @jorbin
11 years ago

  • Resolution → fixed
  • Status assigned → closed

In 33064:

Don't strip newline in esc_url() when protocol is mailto:

The mailto protocol is a bit different than the other protocols in that new lines are something you might realistically want to include. Includes tests to make sure that http protocol urls that contain mailto: aren't affected. Tests for stripping newlines in general already exist.

Fixes #31632
Props danielbachhuber

This ticket was mentioned in ​Slack in #core by eric. ​View the logs.


11 years ago

Note: See TracTickets for help on using tickets.