WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 19 months ago

#19420 closed defect (bug) (invalid)

get_blog_permalink() can cache either an HTTP or HTTPS link for the same post

Reported by: mdawaffe Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.0
Component: Multisite Keywords: has-patch
Focuses: Cc:

Description

get_home_url() can return an HTTP or HTTPS URL based on an is_ssl() && !is_admin() check.

get_permalink() uses home_url() to construct the permalink.

get_blog_permalink() caches the result of get_permalink().

When using a persistent cache, this can lead to an inconsistent cache and cause unexpected results.

Suggestion: always cache the link as HTTP. Add a filter to the return value so that sites can adjust things as needed for their specific scenarios.

Patch attached.

Attachments (2)

19420.diff (541 bytes) - added by mdawaffe 2 years ago.
blog-perma-set-url-scheme.diff (455 bytes) - added by wonderboymusic 19 months ago.

Download all attachments as: .zip

Change History (10)

mdawaffe2 years ago

comment:1 johnbillion2 years ago

  • Cc johnbillion@… added

comment:2 follow-up: nacin2 years ago

It would be nice, perhaps, if we did a set_url_scheme() which sets the scheme to be whatever it should be if home_url() was called there. (Note, this isn't a function in core yet — #18017.)

comment:3 in reply to: ↑ 2 ; follow-up: mdawaffe2 years ago

Replying to nacin:

It would be nice, perhaps, if we did a set_url_scheme() which sets the scheme to be whatever it should be if home_url() was called there. (Note, this isn't a function in core yet — #18017.)

The current is_ssl() && !is_admin() check is a decent guess for home_url(), since you're always talking about the same domain.

It seems harder to make a good guess about what scheme to use for get_blog_permalink(). If I'm on https://one.example.com/, should get_blog_permalink() return "https://two.example.com/?p=1" or "http://two.example.com/?p=1"?

Hence the filter :)

comment:4 in reply to: ↑ 3 nacin2 years ago

Replying to mdawaffe:

Hence the filter :)

Seems sane, but chances are, if you are browsing a site via SSL from the frontend, the best guess would be SSL for any other site on that network. That's also the current functionality of the code (assume a non-persistent cache).

Stepping back, I don't really like that we switch_to_blog() to call get_permalink(). One, we do nothing to load the rewrite rules in a switch_to_blog(). Two, it would be much better if get_blog_permalink() could somehow leverage get_home_url(). That is then SSL by default but includes $blog_id in the filter.

Sidenote, that get_home_url() for a different blog also already relies on is_ssl() && ! is_admin() should probably be considered further support using that for get_blog_permalink().

comment:5 follow-up: mdawaffe2 years ago

The kinds of ways different sites in a network link to each other are a lot more complicated than the way one site links to itself, but you're probably right about frontend SSL for most cases. I agree that's better than always returning HTTP.

Consider, though https://example.com/some-secure-front-end-feature/ linking to posts from one.example.com and two.example.com. I can see scenarios where you'd want to link to HTTP and ones where you'd want to link to HTTPS.

Also, what we currently do is largely based on assumptions for a single site, so I don't think what we currently do should necessarily inform what we decide to do :)

In any case, as long as the cache is consistent (always HTTP, for example) and there's a filter, we should be able to satisfy any requirement.

comment:6 in reply to: ↑ 5 nacin2 years ago

  • Version changed from 3.3 to 3.0

Replying to mdawaffe:

Also, what we currently do is largely based on assumptions for a single site, so I don't think what we currently do should necessarily inform what we decide to do :)

Preaching to the choir, but consistency is a good thing to keep.

The function came from MU, setting version to 3.0.

comment:7 wonderboymusic19 months ago

Refreshed with set_url_scheme, if that matters

comment:8 nacin19 months ago

  • Milestone Awaiting Review deleted
  • Resolution set to invalid
  • Status changed from new to closed

We no longer do redundant caching in get_blog_permalink(), so this ticket should no longer be necessary.

Note: See TracTickets for help on using tickets.