Share the WP.com host check and apply it to notification media - #25867
Merged
crazytonyli merged 4 commits intoAug 5, 2026
Conversation
Lift the WordPress.com host check into `URL.isWordPressComHost` in WordPressShared so `MediaRequestAuthenticator` and the notification service extension share one allowlist instead of duplicating it. Gate the notification extension's media token on that helper, replacing a `host.contains(...)` match, and drop a leftover debug `print`.
Contributor
|
| App Name | WordPress | |
| Configuration | Release-Alpha | |
| Build Number | 33614 | |
| Version | PR #25867 | |
| Bundle ID | org.wordpress.alpha | |
| Commit | 545feab | |
| Installation URL | 6qjd11ttu3am8 |
Contributor
|
| App Name | Jetpack | |
| Configuration | Release-Alpha | |
| Build Number | 33614 | |
| Version | PR #25867 | |
| Bundle ID | com.jetpack.alpha | |
| Commit | 545feab | |
| Installation URL | 6687qmg8cntko |
Parse the icon URL and compare hosts when deciding between the direct, blavatar, and Photon optimizers, replacing `contains` matches over the whole URL string. The two WP.com checks collapse into the shared `isWordPressComHost` helper; the Gravatar check gets its own host-anchored comparison.
Contributor
🤖 Build Failure AnalysisThis build has failures. Claude has analyzed them - check the build annotations for details. |
crazytonyli
reviewed
Aug 4, 2026
| @@ -35,10 +35,13 @@ extension SiteIconViewModel { | |||
| extension SiteIconViewModel { | |||
| /// Returns the Size Optimized URL for a given Path. | |||
| static func optimizedURL(for path: String, imageSize: CGSize = SiteIconViewModel.Size.regular.size, isP2: Bool = false) -> URL? { | |||
Contributor
There was a problem hiding this comment.
What you think about changing path to url: #25868?
crazytonyli
reviewed
Aug 4, 2026
| let optimizedURL = SiteIconViewModel.optimizedURL(for: path) | ||
|
|
||
| // Then | ||
| XCTAssertEqual(optimizedURL?.host, "i0.wp.com") |
crazytonyli
approved these changes
Aug 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #25863 — this targets that branch, not
trunk. Merge #25863 first.Summary
URL.isWordPressComHosthelper inWordPressShared, replacing the copies that had grown up inMediaRequestAuthenticatorand the notification service extension.SiteIconViewModelwith host comparisons.Changes
1.
WordPressShared/URL+Helpers.swiftAdd
isWordPressComHost— matcheswordpress.com/wp.comexactly or as a subdomain, lowercased first (hostnames are case-insensitive, RFC 4343). Documented as the allowlist for attaching the account-wide WP.com token, and why it is deliberately broader than the neighbouringisHostedAtWPCom.2.
MediaRequestAuthenticator.swiftReplace the local
isTokenAllowed(added in #25863) withurl.isWordPressComHostat all three call sites.3.
NotificationService.swiftGate the media token header on
url.isWordPressComHostin place of ahost.contains(...)match, remove the now-unusedisWPComSite, and drop a leftover debugprint.4.
SiteIconViewModel+Extensions.swiftRoute icon URLs by parsed host instead of
containsover the whole URL string. The two WP.com checks (isDotcomURL,isPhotonURL) fed the same branch and collapse intoisWordPressComHost; the Gravatar check becomes a host-anchored comparison. No credentials flow here — the stake is that a crafted URL could previously skip the Photon proxy and be fetched directly. Note one deliberate broadening: non-.files*.wordpress.comhosts are now sized directly (?w=&h=) rather than Photon-wrapped; both are WP.com infrastructure and both resize.5. Tests
New
URLWordPressComHostTestsinWordPressSharedTestscovers the allowlist, case-insensitivity, subdomain lookalikes, userinfo-spoofed hosts, and hostless URLs. The equivalent unit test moves out ofMediaRequestAuthenticatorTests; its behavioural checks stay.SiteIconViewModelTestsgains lookalike-routing cases and pins the*.wordpress.combroadening.Kept in
WordPressSharedrather thanWordPressCoreon purpose: the notification service extension linksWordPressSharedbut notWordPressCore, andWordPressCorepulls inwordpress-rs— too heavy for a memory-capped extension.Test plan
swift test→URLWordPressComHostTestspasses (6/6, verified locally)WordPressUnitTests(CI) →MediaRequestAuthenticatorTestsandSiteIconViewModelTests, plus the app and notification extension compiling against the shared helperRelated