Fix _parsely_canonical_url storing an http scheme on https sites

[TrivediKavit]

Description

While digging into an http:// URL that had turned up in _parsely_canonical_url on an https-only site, we traced it back to the order of two blocks in Post_Data_Trait::extract_post_data().

This moves the https normalization above the set_canonical_url() call, so the URL that gets saved is the same normalized one already used for the response fields. It is a straight statement reorder, with no change to logic.

Motivation and context

In src/rest-api/stats/trait-post-data.php, $post_url comes from $item['url'], which is whatever the Parse.ly API returned. That is often http:// for sites Parse.ly first crawled before an https migration. The meta was being written first, with the normalization running just after:

$post_url = Parsely::get_url_with_itm_source( $item['url'], null );

// Saves the raw value.
if ( 0 !== $post_id ) {
    Parsely::set_canonical_url( $post_id, $post_url );
}

// Only updates the local variable, which the response fields below use.
if ( Utils::parsely_is_https_supported() ) {
    $post_url = str_replace( 'http://', 'https://', $post_url );
}

So rawUrl, dashUrl, id and url all came out correct, and only the saved value kept the original scheme.

Two things stop it from correcting itself:

The stored value sticks. get_canonical_url_from_post() prefers the meta and only falls back to get_permalink() when it is empty, so once an http value is written it keeps winning over the correct permalink.

Nothing further down catches it either. set_canonical_url() runs the value through sanitize_url( $canonical_url, array( 'http', 'https' ) ), which allows http, and get_canonical_url() only swaps the host for the Site ID without looking at the scheme. From there it reaches canonical_url in the REST metadata endpoint and the Smart Links suggestion payloads, so those can end up carrying an http canonical for an https page.

It is also written when the stats sidebar loads rather than on save, which makes it hard to attribute. We found it on a post that had not been edited for five days.

A note on scope

Utils::get_post_id_by_url( $item['url'] ) still uses the original, unnormalized URL. It works as it is, and changing it could affect post matching, so it seemed better left alone.

A broader fix would be to normalize inside set_canonical_url(), which would also cover the two callers in src/Models/class-smart-link.php (lines 670 and 699) that take a caller-supplied URL and look to have the same exposure. That changes the behavior of a public static method, so we left it as your call. Happy to switch if you would prefer it.

Either way, meta already written by affected versions will not fix itself, because of the precedence above. Normalizing on read in get_canonical_url_from_post(), or a small migration, would clear those out. Happy to add either if useful.

How has this been tested?

There are two new tests in EndpointStatsPostTest.php, one for each side of the Utils::parsely_is_https_supported() condition. Both mock pre_http_request to return an http:// URL for a test post, dispatch the endpoint, and check the saved _parsely_canonical_url. They assert on the scheme rather than the whole URL, to stay clear of the Site ID host swap in get_canonical_url().

• test_get_details_stores_canonical_url_with_https_scheme forces home_url() and site_url() to https, and expects the saved value to be https://.
• test_get_details_keeps_http_canonical_url_without_https_support leaves the site http-only, and expects the saved value to stay http://.

The second test guards the http-only path, which this change should leave exactly as it was.

Run against the unfixed code, the https test fails and the http-only one still passes:

✔ Get details keeps http canonical url without https support
✘ Get details stores canonical url with https scheme
  Failed asserting that 'http://example.com/?p=4' starts with "https://".

With the fix, both pass. So the only behavior that changes is the https case.

The tests are in the first commit and the fix in the second, which does mean the first commit is red on its own. Happy to squash or reorder if you would rather it were not.

Everything run locally against wp-env, on PHP 8.3 with WordPress 7.0.2:

• composer testwp: 497 tests, 1509 assertions, no failures. The 3 skips are the usual multisite ones on a single-site run.
• composer cs: clean.
• php lint.php: clean.
• vendor/bin/phpstan analyse: 21 errors, but all of them are already there on develop with these changes stashed, so none are introduced here. They look to be an artifact of running the analysis on PHP 8.3 while the project targets 7.4, and phpstan does not appear to run in CI. Glad to look at them separately if that would be useful.

CanonicalURLsTest.php also still passes untouched, which is what we would expect given set_canonical_url() itself is unchanged.

Thanks for taking a look.

Screenshots (if appropriate)

N/A

Summary by CodeRabbit

• Bug Fixes

  • Canonical post URLs in statistics are now correctly saved with HTTPS when the site supports secure connections.
  • HTTP-only sites continue to preserve HTTP canonical URLs.

• Tests

  • Added coverage verifying canonical URL persistence for both HTTPS-enabled and HTTP-only sites.

[coderabbitai]

📝 Walkthrough

Walkthrough

The change moves HTTPS normalization before canonical URL persistence. Integration tests verify HTTPS conversion on HTTPS sites and HTTP preservation on HTTP-only sites.

Changes

Canonical URL persistence

Layer / File(s)	Summary
Normalize canonical URLs before persistence src/rest-api/stats/trait-post-data.php, tests/Integration/RestAPI/Stats/EndpointStatsPostTest.php	The endpoint now stores the normalized HTTPS URL when HTTPS is supported. Tests verify HTTPS conversion and HTTP preservation on HTTP-only sites.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely identifies the main fix: preventing an HTTP scheme in _parsely_canonical_url on HTTPS sites.
Description check	✅ Passed	The description covers the change, motivation, scope, testing, results, known baseline PHPStan errors, and screenshots status.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 PHPStan (2.2.7)

⚠️ You're running an old version of PHPStan.️

The last release in the 1.12.x series with new features
and bugfixes was released on July 17th 2025,
that's 385 days ago.

Since then more than 65 new PHPStan versions were released
with hundreds of new features, bugfixes, and other
quality of life improvements.

To learn about what you're missing out on, check out
this blog with articles about the latest major releases:
https://phpstan.org/blog

Upgrade today to PHPStan 2.2 or newer by using
"phpstan/phpstan": "^2.2" in your composer.json.

Invalid configuration:
Unexpected item 'parameters › type_coverage'.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

tests/Integration/RestAPI/Stats/EndpointStatsPostTest.php (1)

716-717: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

End each line comment with a period.

• tests/Integration/RestAPI/Stats/EndpointStatsPostTest.php#L716-L717: make Line 716 a complete sentence ending with a period.
• tests/Integration/RestAPI/Stats/EndpointStatsPostTest.php#L724-L725: make Line 724 a complete sentence ending with a period.
• tests/Integration/RestAPI/Stats/EndpointStatsPostTest.php#L814-L815: make Line 814 a complete sentence ending with a period.

As per path instructions, “Ensure each line comment concludes with a period.”

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/Integration/RestAPI/Stats/EndpointStatsPostTest.php` around lines 716 -
717, Update the comments at
tests/Integration/RestAPI/Stats/EndpointStatsPostTest.php lines 716-717,
724-725, and 814-815 so each comment line forms a complete sentence ending with
a period.

Source: Path instructions

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@tests/Integration/RestAPI/Stats/EndpointStatsPostTest.php`:
- Around line 716-717: Update the comments at
tests/Integration/RestAPI/Stats/EndpointStatsPostTest.php lines 716-717,
724-725, and 814-815 so each comment line forms a complete sentence ending with
a period.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 9ea03003-e909-4788-bf2b-c3a59d32ded9

📥 Commits

Reviewing files that changed from the base of the PR and between cfd667d and 4b24fc1.

📒 Files selected for processing (2)

• src/rest-api/stats/trait-post-data.php
• tests/Integration/RestAPI/Stats/EndpointStatsPostTest.php