[@_ck_ Participant 15 years, 9 months ago]

Maybe I am getting rusty, but looking at the diff, I don’t see it.

Basically it’s the same thing, shuffled around a bit and the same esc_url and esc_attr is being used at the end, which is the same pattern as the existing 1.0/1.1 code.

Are you sure the fix was not being caused by just having a more updated version of esc_url and esc_attr ?

Or was it just being solved by moving those two sanitizers up higher before bb_safe_redirect could ever be called. Because that was definitely an oversight on someone’s part (not me). Based on the comments I’d guess they thought their functionality was for display filtering and not actually sanitization.

Looking at esc_url though, its default is indeed meant for displaying urls by default, note the comment in the code “Replace ampersands and single quotes only when displaying.” and how the context is set by default to “display”.

Maybe test your solution with a url that contains an ampersand, it probably will not work?

Also, are you relying on the list of protocols to do the sanitization? Because that could be defeated too (and would prevent using relative urls that are legitimate).

ps. very minor but you have confusing indentation indicating nesting with the code at line 18 – which actually is not nested

Feel free to email me directly if that’s better.