[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:
     Type:  defect (bug)                         |      Status:  assigned
 Priority:  normal                               |   Milestone:  Future
Component:  Permalinks                           |  Release
 Severity:  normal                               |     Version:  3.0.3
 Keywords:  needs-testing 4.1-early needs-patch  |  Resolution:
                                                 |     Focuses:
-------------------------------------------------+-------------------------

Comment (by joemcgill):

 Thanks Boone. I wasn't sure about writing a single unit test vs. multiple.
 Is the general rule to keep tests to one assertion per test?

 In terms of the patch itself, I was thinking of the case where you call
 the `wp_get_attachment_url()` from a theme on the front end, which is
 where I first encountered this bug. I thought about the second case—when
 an image is included in a post, as you describe—but that seemed like a
 separate issue that might be better handled through a filter on
 `the_content()` where we would also handle cases where a site with a big
 archive decides to switch its scheme after the fact.

 Also, I was under the impression that stripping the scheme was a
 nonstarter on account of situations where the post content gets pulled
 into emails, etc.

 Thoughts?

 Replying to [comment:74 boonebgorges]:
 > Thanks for the patch, joemcgill. The logic of the unit tests looks good
 to me, though I'd prefer to have them broken into separate tests, as in
 [attachment:15928.5.patch].
 >
 > The patch is still missing the mark, though. If I understand correctly,
 the root issue in this ticket is that you may be administering your site
 over HTTPS, with your front-end viewable over HTTP. When writing a post,
 you add an attachment, and some `<img>` markup gets inserted into your
 post. But, since you're looking at the dashboard, the `src` element will
 have the 'https' scheme saved in the post content.
 >
 > I think the `set_url_scheme()` approach currently in the patch is
 probably fine, but it's not enough. My gut feeling is that we should
 probably also strip the scheme as suggested earlier (`//`) when building
 the `src` attribute for the `img` tag put into the editor.

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