Update esc_url to allow for specifying an https:// as default protocol

[mkaz]

If no protocol is specified for esc_url the function will automatically prepend the ​http:// protocol. This is likely now the wrong assumption, but potentially can break backwards compatibility if changed, since developers may rely on this.

So this change proposes an additional parameter to the function to specify a default protocol, keeping the old default but now allowing for one to ask for ​https://

This came up in this ticket: ​https://github.com/WordPress/gutenberg/pull/30100

The usage could then be:

esc_url( $url, null, 'display', 'https://' );

[SergeyBiryukov]

Thanks for the patch!

Just noting that the @since tag should be in the function DocBlock, after @since 2.8.0, and should follow this format for consistency:

@since 5.8.0 Added the `$default_protocol` parameter.

[audrasjb]

In 52886.1.diff:

• Add @since mention
• Few coding standards fixes (whitespaces inside docblocks)

Question: shouldn't we also add the new parameter in the clean_url filter at the bottom of the function? Not sure it's really needed, but I prefer to ask :)

[SergeyBiryukov]

The usage could then be:

esc_url( $url, null, 'display', 'https://' );

Looking at this again, I wonder if it's straightforward enough in practice, as it would require passing four parameters to every esc_url() call that might be missing the protocol and should default to https://. If the site is known to use https://, that seems redundant.

Perhaps this should be a filter instead, like esc_url_default_protocol? That way it could only be specified once.

[mkaz]

Thanks for the follow up on this one, it had fallen off my radar.

My potential concern with a filter is that it would apply to all esc_url() calls.

The use case we had was to implement on the Block Editor for the social links, but if we did it as a filter and once it got to core it the filter would be implemented for everyone which is the same as default to https:// everywhere.

Also, as a filter someone else could override and then it would not pick the right URL for say ​https://twitter.com/

@audrasjb I'm not sure it makes sense to pass in to the clean_url filter, I can't think of a scenario where it would help.

My other thought was should we change to esc_url_raw to also support defaulting to https://

[desrosj]

As we're at feature freeze for 5.8 and this one still needs unit tests, I'm going to punt. Please do continue to discuss and feel free to move back into a numbered milestone once tests are able to be added.

[mkaz]

Unit test added.

[costdev]

@SergeyBiryukov

Looking at this again, I wonder if it's straightforward enough in practice, as it would require passing > four parameters to every esc_url() call that might be missing the protocol and should default to
​https://. If the site is known to use ​https://, that seems redundant.

In this case, would wp_is_using_https()​Docs be appropriate regarding "known to use https://" as it tests the home and site URLs?

If so, an adjustment could be made to the latest patch:

Line
4291	<?php
4292
4293	/**
4294	* Checks and cleans a URL.
4295	*
4296	* A number of characters are removed from the URL. If the URL is for displaying
4297	* (the default behaviour) ampersands are also replaced. The {@see 'clean_url'} filter
4298	* is applied to the returned cleaned URL.
4299	*
4300	* @since 2.8.0
4301	* @since 5.8.0 Added the `$default_protocol` parameter.
4302	*
4303	* @param string   $url              The URL to be cleaned.
4304	* @param string[] $protocols        Optional. An array of acceptable protocols.
4305	*                                   Defaults to return value of wp_allowed_protocols().
4306	* @param string   $_context         Private. Use esc_url_raw() for database usage.
4307	* @param string   $default_protocol Optional. Use to specify a different default protocol.
4308	*                                   Defaults to http:// or https:// based on the result
4309	*                                   of wp_is_using_https().
4310	* @return string The cleaned URL after the {@see 'clean_url'} filter is applied.
4311	*                An empty string is returned if `$url` specifies a protocol other than
4312	*                those in `$protocols`, or if `$url` contains an empty string.
4313	*/
4314	function esc_url( $url, $protocols = null, $_context = 'display', $default_protocol = '' ) {
4315	$original_url = $url;
4316
4317	if ( '' === $default_protocol ) {
4318	$default_protocol = ( wp_is_using_https() ) ? 'https://' : 'http://';
4319	}
4320
4321	if ( '' === $url ) {
4322	return $url;
4323	}
4324
4325	$url = str_replace( ' ', '%20', ltrim( $url ) );
4326	$url = preg_replace( '|[^a-z0-9-~+_.?#=!&;,/:%@$\|*\'()\[\]\\x80-\\xff]|i', '', $url );
4327
4328	if ( '' === $url ) {
4329	return $url;
4330	}
4331
4332	if ( 0 !== stripos( $url, 'mailto:' ) ) {
4333	$strip = array( '%0d', '%0a', '%0D', '%0A' );
4334	$url   = _deep_replace( $strip, $url );
4335	}
4336
4337	$url = str_replace( ';//', '://', $url );
4338	/*
4339	* If the URL doesn't appear to contain a scheme, we presume
4340	* it needs http:// prepended (unless it's a relative link
4341	* starting with /, # or ?, or a PHP file).
4342	* Since 5.8, it uses $default_protocol which, if empty, will use
4343	* http:// or https:// based on the result of wp_is_using_https().
4344	*/
4345	if ( strpos( $url, ':' ) === false && ! in_array( $url[0], array( '/', '#', '?' ), true ) &&
4346	! preg_match( '/^[a-z0-9-]+?\.php/i', $url ) ) {
4347	$url = $default_protocol . $url;
4348	}
4349
4350	// Replace ampersands and single quotes only when displaying.
4351

This adjustment:

1. still appears to meet the aims of the ticket without requiring a change to existing esc_url() calls.
2. still allows developers to force http:// or https:// if their call requires it.
3. seems to remove the redundancy @SergeyBiryukov raised.
4. doesn't assume that http:// should be the default protocol.
5. will mean that esc_url() automatically adapts when a website is updated to support https://.

Questions

1. If wp_is_using_https() isn't appropriate because we only need to test one of the URLs, would it be wp_is_home_url_using_https() ​Docs or wp_is_site_url_using_https() ​Docs?

2. If wp_is_using_https() isn't appropriate because it fails to sufficiently check the https:// status for this issue, would wp_is_https_supported() ​Docs be more appropriate? Or is there another that stands out as appropriate?

[mkaz]

In this case, would wp_is_using_https()Docs be appropriate regarding "known to use ​https://" as it tests the home and site URLs?

No, unfortunately that wouldn't solve the case I'm looking for, that function only works for the current site while the esc_url() function can be passed any arbitrary URL which it won't know if it is SSL or not.

The example is a Twitter profile link passed in like: twitter.com/mkaz ideally it would by default return https but esc_url() will return http.

[costdev]

Replying to mkaz:

In this case, would wp_is_using_https()Docs be appropriate regarding "known to use ​https://" as it tests the home and site URLs?

No, unfortunately that wouldn't solve the case I'm looking for, that function only works for the current site while the esc_url() function can be passed any arbitrary URL which it won't know if it is SSL or not.

The example is a Twitter profile link passed in like: twitter.com/mkaz ideally it would by default return https but esc_url() will return http.

---

True! I believe I misinterpreted if the site is known to use https to refer to the current site rather than the remote site. Your patch resolves the ticket's issue without any BC breaks and I agree that esc_url_raw() should be updated to support a $default_protocol as well.