[wp-trac] [WordPress Trac] #15928: wp_get_attachment_url does not check for HTTPS
WordPress Trac
noreply at wordpress.org
Mon Nov 17 15:48:32 UTC 2014
#15928: wp_get_attachment_url does not check for HTTPS
--------------------------+-----------------------------
Reporter: atetlaw | Owner: boonebgorges
Type: defect (bug) | Status: accepted
Priority: normal | Milestone: Future Release
Component: Permalinks | Version: 3.0.3
Severity: normal | Resolution:
Keywords: needs-patch | Focuses:
--------------------------+-----------------------------
Comment (by boonebgorges):
joemcgill - Thanks for this patch. A couple requests and questions:
* I'd like to see some unit tests for the content filter.
* The content filter looks too aggressive to me. I think we probably only
want to replace asset URLs, which means stuff in `src` attributes, etc. I
want to be very conservative about modifying things like anchor tags and
URLs in links.
* I think it will probably also be faster to do a couple separate runs
through `str_replace()` than to do a `preg_replace()`, but this could use
some benchmarks with a fairly large piece of content.
* Running through `set_url_scheme()` in `wp_get_attachment_url()` still
seems like an overly aggressive move, as we can't be sure whether plugins
are using this function to populate other kinds of content that could be
generated on one admin domain and then displayed elsewhere. What do you
think about replicating some of the logic from
`wp_filter_uploads_scheme()` in `wp_get_attachment_url()`? That is: only
force the scheme if `is_ssl()` *and* the current URL matches what's coming
out of `wp_upload_dir()`. The difference from what you've written is that,
with [15928.8.patch], posting from https://secure.example.com/wp-admin
will create https links that need to be *removed* on http://example.com,
while my suggestion will create http links from the admin that will be
converted to https by the filter only if https is available on
example.com. (Whew.) What do you think of this?
--
Ticket URL: <https://core.trac.wordpress.org/ticket/15928#comment:84>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform
More information about the wp-trac
mailing list