Fix: Site icon in the toolbar is broken (mixed content) in the admin when siteurl is stored with http:// on an SSL site

[hbhalodia]

Trac ticket: https://core.trac.wordpress.org/ticket/65696

Screenshot

Before	After

Use of AI Tools

• None

Update: Use Claude Code Opus 4.8 for drafting test case.

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props hbhalodia, mukesh27, tyxla, westonruter, youknowriad, fushar, mirmpro.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[Copilot]

Pull request overview

Fixes mixed-content breakage for the Site Icon used in the admin toolbar when the stored attachment URL is http:// but the admin is loaded over SSL, by normalizing the Site Icon URL to the current request scheme.

Changes:

• Update get_site_icon_url() to return a scheme-corrected attachment URL via set_url_scheme().

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[github-actions]

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

• All changes will be lost when closing a tab with a Playground instance.
• All changes will be lost when refreshing the page.
• A fresh instance is created each time the link below is clicked.
• Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
  it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[mukeshpanchal27]

Just out of curiosity, does this mean the issue could occur anywhere we use wp_get_attachment_image_url()?

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

tests/phpunit/tests/general/template.php:170

• This test mutates $_SERVER['HTTPS'] and unconditionally unsets it at the end, which can leak global state into later tests when $_SERVER['HTTPS'] was already set. Saving the previous value, forcing a non-SSL baseline for $url_http, and restoring the original value will make the test isolated and more reliable.

		// Simulate the same admin request served over HTTPS.
		$_SERVER['HTTPS'] = 'on';
		$url_https        = get_site_icon_url();

		unset( $_SERVER['HTTPS'] );

[hbhalodia]

Just out of curiosity, does this mean the issue could occur anywhere we use wp_get_attachment_image_url()?

Yeah it could. Because the function would return the URL and currently we are applying the fix limited to site icon URL only and we are setting the scheme as expected.

I guess the issue can be more broader and should not be limited only to site icon, but wherever this is used. Though this needs verification and would verify the same, if that is the case.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

[youknowriad]

@mukeshpanchal27 yes, it can happen with any attachment call but it seems to have been done on purpose to avoid "https" leaking into attachment added to post content resulting in mixed content. That's why I went with a safer suggestion (site icon only)

[tyxla]

Fix makes sense to me 👍

I'm not super sure about the test coverage - at first glance it feels a bit like a test is unnecessary, but at the same time I can imagine there are cases that we might want to test separately like:

• if admin is accessed via http (in the case of force_ssl_admin() being off or no redirect yet)
• with multisite, if we're requesting an icon for an HTTP-only blog during an HTTPS request for example

What do you all think?

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

tests/phpunit/tests/general/template.php:166

• This test mutates $_SERVER['HTTPS'] but doesn’t control $_SERVER['SERVER_PORT'], so the “HTTP baseline” can still be detected as SSL in environments where SERVER_PORT is 443. It also risks leaking modified $_SERVER state into later tests if an assertion fails. Save/restore the original values and explicitly set a non-SSL port for the baseline (and restore everything in a finally).

		unset( $_SERVER['HTTPS'] );

		$this->assertFalse( is_ssl(), 'Baseline request should not be detected as SSL.' );

[fushar]

@westonruter I see the PR author has addressed all the comments. Can you check and see if the PR is good now? I believe, this fixes the regression. Thanks!

[mirmpro]

Testing notes (the hard way) 🥲

To reproduce this bug locally, you need the admin running over HTTPS — which sounds simple until you realise the default Docker dev environment only speaks HTTP.

What it actually took:

• Generated a local SSL certificate
• Added HTTPS support to the Nginx container
• Trusted the CA cert in macOS Keychain so Chrome doesn't throw ERR_SSL_PROTOCOL_ERROR
• Updated the WordPress DB URLs to https://
• Restarted containers

Once that was all in place, the bug was 100% reproducible — the site icon in the admin toolbar was blocked as mixed content when the stored attachment URL was http:// and the admin loaded over https://. The set_url_scheme() fix resolves it cleanly.

The fix is correct. Tested and verified. ✅

(Might be worth adding a note to the PR description about needing an HTTPS environment to test this — future reviewers will thank you.)

[mirmpro]

Due to a change in my Git username, my WordPress.org profile link is now broken.