[wp-hackers] Fixing some SSL cases for 3.4

Fri Nov 11 19:45:33 UTC 2011

I took a quick look at the patch.  In general, I like the approach.  It
will be a little while before I have time to  confirm my understanding of
exactly what it does.  For instance, I haven't evaluated whether it would
change the scheme for these constants:

WP_CONTENT_DIR  // no trailing slash, full paths only
WP_CONTENT_URL  // full url
WP_PLUGIN_DIR  // full path, no trailing slash
WP_PLUGIN_URL  // full url, no trailing slash

As noted by Jeremy Clarke and others, (see:
http://core.trac.wordpress.org/ticket/13941) before WP3.0, plugin and theme
authors where 'doing_it_right()' using these constants.  There's been no
serious effort to eliminate their continued use.  Without fixing the
constants, or enforcing that they're not used by plugins and themes, we
won't be done.  And then we'll have to deal with content urls while editing
posts and comments when FORCE_SSL_ADMIN is defined.

So - to recap:

1) decide on implementation of set_url_scheme() (
http://core.trac.wordpress.org/ticket/18017) and whether to go with this
refactoring of scheme (
http://core.trac.wordpress.org/attachment/ticket/19037/ssl.patch )

2) determine our way forward on whether to change the constants so that
they respect scheme()

3) decide how to support editing if content urls should be http when
published, if FORCE_SSL_ADMIN is in effect.

4) related tickets: #13941 <http://core.trac.wordpress.org/ticket/13941>,
#15928 <http://core.trac.wordpress.org/ticket/15928>,
#19023<http://core.trac.wordpress.org/ticket/19023>,
#18005 <http://core.trac.wordpress.org/ticket/18005>,
#19037<http://core.trac.wordpress.org/ticket/19037>,
#18017 <http://core.trac.wordpress.org/ticket/18017>

Am I missing anything?

Pauli

On Tue, Nov 8, 2011 at 5:57 PM, Marcus Pope <Marcus.Pope at springbox.com>wrote:

> I'm all for this change as much as I am for the root-relative url switch
> because it improves performance and robustness of wp.
>
> I'm even in support of putting wordpress-https AND my own
> root-relative-urls out of business if possible, but schemeless urls
> generated by the system would not put either to bed because of content urls
> and the long-standing assumptions made in hundreds of plugins, not just
> wordpress-https.
>
> Here's the strategy I took to fix the SSL problems you pointed out:
>
> http://core.trac.wordpress.org/ticket/19037
> http://core.trac.wordpress.org/attachment/ticket/19037/ssl.patch
>
> In addition to the tickets you reference for open/existing ssl problems I
> discovered a number of undocumented ones while refactoring the core of
> wordpress to support root-relative urls.  When I realized it could be
> achieved via a plugin, I backed out all of the work except for this scheme
> refactoring.  I didn't keep all of the ssl fixes because after finding half
> a dozen or so I realized that I could spend another week finding more.
>  This patch was just to get the ball rolling on a strategy for moving
> forward.
>
> Implement this patch, and then wrap scheme(...) around each of the
> referenced code points in those trac tickets and I think you'll find the
> problem solved... temporarily.  I say temporarily because it's only a
> matter of time before someone extends the core framework in some way
> related to urls and forgets to wrap scheme() around it, in the same way
> they forgot to do the is_ssl() check, or http/s string substitution in
> those existing tickets.
>
> (and yeah I totally should have seen that 3.4 reference in the subject, or
> in the text, or every other place it was plastered! :D)
>
> Thanks,
> Marcus
>
> -----Original Message-----
> From: wp-hackers-bounces at lists.automattic.com [mailto:
> wp-hackers-bounces at lists.automattic.com] On Behalf Of Pauli Price
> Sent: Friday, November 04, 2011 1:28 PM
> To: wp-hackers at lists.automattic.com
> Subject: Re: [wp-hackers] Fixing some SSL cases for 3.4
>
> "last minute stake in the 3.3 game"
>
> This work is targeting 3.4, so not to worry.  The point is to put
> http://wordpress.org/extend/plugins/wordpress-https/ out of business,
> except for shared SSL certificate support.  That seems like enough of an
> edge case that it can be delegated to a plugin.
>
> Pauli
>
>
> On Thu, Nov 3, 2011 at 8:22 PM, Marcus Pope <Marcus.Pope at springbox.com
> >wrote:
>
> > Oh man, I swear I did *not* just create a new profile with the name
> > Pauli Price!!!
> >
> > :D
> >
> > Thanks Pauli, those tickets along with the other 500 or so
> > created/fixed/pending on the subject sure are a pain to deal with.
> >
> > But I wanted to say there are some webservers that cannot handle
> > scheme-relative urls.  Not sure if wordpress is supported on anything
> > other than apache and iis (which do support them fully) but if there
> > is support for other webserver packages we should be cautious to
> implement.
> >
> > And to be devil's advocate here, there are a number of plugins that do
> > call parse_url on the home_url's returned by wordpress.  And a smaller
> > number of those logic branch on the "scheme" key in the returned hash
> > array.  Granted it will only generate php notices to access the null
> > scheme key, but it could change the logic branch in a detrimental way
> > if scheme-less urls are used.
> >
> > This little guy has over 25k downloads
> > http://wordpress.org/extend/plugins/wordpress-https/
> > As well as some admin/lockout issues raised in the 2.0 launch a couple
> > days ago.  But I'm sure there are others like the BWP-Minify that
> > could see some problems as well.
> >
> > I have always been an advocate of the argument that it could be
> > problematic for the community, so my concern here is that it's a last
> > minute stake in the 3.3 game, that might need some more heads-up
> > notice to developers if we were to do a scheme-less switch.
> >
> > But I have also always been a fall-back supporter of scheme-relative
> > urls if root-relative urls were out of the question.
> >
> > Thanks!
> >
> > -----Original Message-----
> > From: wp-hackers-bounces at lists.automattic.com [mailto:
> > wp-hackers-bounces at lists.automattic.com] On Behalf Of Pauli Price
> > Sent: Thursday, November 03, 2011 3:47 PM
> > To: wp-hackers at lists.automattic.com
> > Subject: [wp-hackers] Fixing some SSL cases for 3.4
> >
> > Here¹s a transcript from #wordpress-dev.  Seeking guidance on how to
> > move forward with this.  It¹s clear that we don¹t have a consensus yet.
> >
> > Discussion happens here, on the wp-hackers?  Or on a trac ticket?
> >
> > Is it appropriate to consolidate these tickets?  If so how?
> >
> > ======== transcript follows ==========
> >
> > marfarma:
> >  I'd like to help resolve these related tickets for 3.4 - anybody
> > working on them? #13941, #15928, #18017, #19023, #18005
> >
> > trac-bot:
> >  http://core.trac.wordpress.org/ticket/13941 Future Release,
> > micropat->ryan, new, WP_CONTENT_URL should use site_url() to support
> > micropat->HTTPS /
> > SSL
> >  http://core.trac.wordpress.org/ticket/15928 Future Release,
> > atetlaw->(no owner), assigned, wp_get_attachment_url does not check
> > for HTTPS
> >  http://core.trac.wordpress.org/ticket/18017 minor, Future Release,
> > jkudish->jkudish, new, set_url_scheme() function
> >  http://core.trac.wordpress.org/ticket/19023 high, Awaiting Review,
> > joostdevalk->nacin, reviewing, Images in Edit Comments break SSL
> >  http://core.trac.wordpress.org/ticket/18005 minor, Awaiting Review,
> > rfc1437->(no owner), reopened, mixed http/https installation and
> > add_custom_background / body_class
> >
> > nacin:
> >  That's one chunk of tickets.
> >  Yeah, I'd definitely like to tackle all of that for 3.4.
> >
> > marfarma:
> >  but all the same underlying problem
> >
> > nacin:
> >  Indeed.
> >
> > rboren:
> >  Fixing those SSL cases and using home_url() everywhere it is needed
> > would be nice for 3.4.
> >
> > rboren:
> >  And, dare I say it, introduce 'relative' as a scheme argument.
> >
> > marfarma:
> >  relative -- isn't that a 'live-wire' topic?
> >  as in 'untouchable'
> >
> > nacin:
> >  I'd like to do protocol-independent, at least.
> >  like //
> >  anyway,
> >
> > (nacin is now known as nacin|afk.)
> >
> > rboren:
> >  It'd just be an arg for use by plugin and theme authors. No core
> > movement to it.  Anyhow, just something we might want to discuss.
> >
> > AaronCampbell:
> >  I like the idea of being able to do relative links when it makes
> > sense
> >
> > iamfriendly:
> >  where's the "+1 million" button?
> >  what do you mean I don't get a million votes?
> >
> > ======== transcript ends ==========
> >
> > Pauli (aka: marfarma)
> > _______________________________________________
> > wp-hackers mailing list
> > wp-hackers at lists.automattic.com
> > http://lists.automattic.com/mailman/listinfo/wp-hackers
> > _______________________________________________
> > wp-hackers mailing list
> > wp-hackers at lists.automattic.com
> > http://lists.automattic.com/mailman/listinfo/wp-hackers
> >
> _______________________________________________
> wp-hackers mailing list
> wp-hackers at lists.automattic.com
> http://lists.automattic.com/mailman/listinfo/wp-hackers
> _______________________________________________
> wp-hackers mailing list
> wp-hackers at lists.automattic.com
> http://lists.automattic.com/mailman/listinfo/wp-hackers
>