get_the_post_format_url() should not escape data

[tollmanz]

Like get_permalink(), get_the_post_format_url() should not escape data. The responsibility of escaping the data should be placed on the calling function. An analogous situation in core is that get_permalink() does not escape the data and the_permalink() does. I think this pattern should continue in the get_the_post_format_url() and the_post_format_url() functions. It is very handy to have access to the rawest form of the data via get_the_post_format_url().

[tollmanz]

The patch removes the use of esc_url_raw() in get_the_post_format_url().

get_the_post_format_url() is used once in Twenty Eleven, once in Twenty Thirteen, and once in WordPress core. In each instance, the data is escaped after the function is used. Removing the escaping within the function itself should not leave any data ultimately unescaped.

Note that if my patch in #24416 is accepted, there may be an additional esc_url_raw() usage that would need to be removed.

[tollmanz]

​24417.2​ also cleans up some PHPDoc junk in the_post_format_url().

[nacin]

If we had to do functions like get_permalink() over again, we'd escape almost everywhere — and at the very least, we should esc_url_raw() where we can.

In the case of get_permalink(), the URL is only generated by sanitized post data and a filter. In the case of this function, it acts directly on user-supplied input. There's a distinct difference.

Wontfix?

[tollmanz]

In keeping with the idea of escaping data late, doesn't it seem like escaping in this function is escaping too early? In the each of the 3 usages of get_the_post_format_url() in core (and this includes in the core themes), the function is escaped again after it is called. It seems unnecessary to add in excessive escaping functions. It makes sense that this function should not escape the data and instead leave it to the calling function.

If we had to do functions like get_permalink() over again, we'd escape almost everywhere — and at the very least, we should esc_url_raw() where we can.

Can you expand on this? I definitely understand the need for good security in WordPress core and see that this would help safe guard 3rd party extension developers; however, I think that much of that responsibility needs to fall on the developers. They need to ensure the safety of their works.

[jeremyfelt]

Agreed that it is on the developer to escape data as late as possible both for security and for general code readability and confidence.

If I want a way to echo the URL to the front end without manipulating the data, I know I can use the_post_format_url() to do so.

If I want to manipulate the URL in any way before outputting it, I should be able to use get_the_post_format_url(). Once I take on that responsibility, it's up to me to escape on output.

At some level, I can see the included esc_url_raw() providing a false sense of security to a developer that should instead be thinking of this as they are working. For output to the front end, we should be using esc_url(). The current state makes for plenty of use cases where esc_url() will be run almost immediately after esc_url_raw(), duplicating a lot of effort.

As a smaller point, valid characters can be stripped from the URL and having access to the raw data would be nice in those rare scenarios.

[wonderboymusic]

RIP this function

[SergeyBiryukov]

[24399] / #24452