[wp-trac] [WordPress Trac] #15928: wp_get_attachment_url does not check for HTTPS

[WordPress Trac]

#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 joemcgill):

 Replying to [comment:89 boonebgorges]:

 I found myself nodding my head up and down so hard to this second
 paragraph:

 > Now, in a broad sense, I think there might be value in storing in some
 global state whether a given domain is accessible over SSL, and then using
 that information in functions like `wp_get_attachment_url()` to assemble
 URLs. But doing it even *once per page* seems like an unacceptable drag on
 server resources, so I'd argue that this data should be at least semi-
 persistent. And in any case, if we were going to go down this path, it
 should be something that happens outside of the scope of this particular
 function, and is implemented wherever we build asset URLs (like, say,
 `wp_enqueue_script()`). I think this is beyond the scope of what's
 necessary for this ticket.

 I wasn't super keen on that piece of those last patches for exactly the
 reasons you pointed out, but I was trying to come up with a way to solve
 for the SSL-optional case AND the split admin/front hosts case you
 described. In reality, I agree that we can narrow the scope of this fix to
 the SSL-optional case and leave the latter alone. Small, incremental
 improvements.

 Over the long run, I still worry about returning http:// content in SSL
 contexts, but I agree that's out of the scope of this particular ticket.

 [attachment: 15928.11.patch] uses the logic you laid out above with a few
 (I hope) minor tweaks:

 {{{
 if ( is_ssl() && 'https' !== substr( $url, 0, 5 ) ) {
         if ( parse_url( $url, PHP_URL_HOST ) === $_SERVER['HTTP_HOST'] ) {
                 $url = set_url_scheme( $url, 'https' );
         }
 }
 }}}

 After the initial check to see if is_ssl() is true and the attachment url
 is not, it sees if the host for the url matches the host for the current
 request, and if so (since we've already established that the url is not
 using an https scheme) changes the scheme of the attachment url to https.

 I've updated the second test case to check for cases where the hosts
 explicitly match. I'm not entirely sure how I would test for cases where
 they don't match, so I've left that alone.

 I hope we're getting warmer.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/15928#comment:90>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform