Escaping the value of the srcset attribute

[henry.wright]

Should we be using esc_url() instead of esc_attr() to escape the value of the srcset attribute in get_avatar()?

The value is a URL, so to me it would make sense to use esc_url(). That said, there might be a valid reason why esc_attr() is used.

[joemcgill]

Hi @henry.wright,

Most likely, esc_attr() was used because the srcset attribute is a string like:

http://1.gravatar.com/avatar/{some_img_id}?s=64&d=mm&r=g 2x

Where the x descriptor at the end of the string means that it's not really a URL. Since this is a fairly simple srcset implementation, we could probably use esc_url() on just the URL part of the image source. 41215.diff​ shows how that would work.

[henry.wright]

I had an idea the "2x" would be the reason. 41215.diff looks like a good solution.

[henry.wright]

Could even do this?

[joemcgill]

You certainly could, but seems a bit overkill to me to run the extra escaping function on a text string that isn't variable. I thought about doing srcset="%s 2x" and then use esc_url for the image source URL only, but handling the whole attribute value as a string replacement means we only have to change one line if we decide to format the srcset differently in the future.

[henry.wright]

@joemcgill I guess if "2x" will remain a literal string and isn't likely to become a variable at a later date then I agree running esc_attr() will be a waste. Happy to go with 41215.diff :)

[SergeyBiryukov]

In 41156:

Users: Use esc_url() instead of esc_attr() to escape the value of the srcset attribute in get_avatar().

Props joemcgill, henry.wright.
Fixes #41215.