[wp-hackers] Fixing some SSL cases for 3.4

Marcus Pope Marcus.Pope at springbox.com
Thu Nov 17 00:34:50 UTC 2011 

My patch only handled internal usage of those constants.  Basically because they're constants and can't be redefined you have to wrap a "scheme()" call around them.  From a plugin perspective they'd have to do the same (wrap the scheme function around each use), since there is no option of redefining the constant.

Personally I'd deal with this by offering a function that returns the URLs instead of constants.  We could deprecate the constants and offer the new functions at the same time.  If you want ssl functionality, update your code to use the functions instead.  Otherwise you can continue to use the constants for a while but you'll have to deal with the side effects.  Plus you could then only scheme the base site url once instead of hundreds of times per page load, much better performance.

Seems like a lot of effort especially when you start dealing with how you add content urls via the admin.  Here's how that bit of nasty breaks down:

Admin SSL & Front NonSSL requires SSL urls for content to prevent warnings, and yields slow as f*ck downloads of media over ssl on the front end.

Admin NonSSL & Front SSL yields NonSSL urls in content that throw warnings on front end.

AdminSSL & FrontSSL (or both NonSSL) = The only scenario that absolute URLS work.

Lmk if you have any questions about the scheme patch, and good luck on working this out in the core.  Frankly I fixed the problem entirely with my plugin :D

-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 11, 2011 1:46 PM
To: wp-hackers at lists.automattic.com
Subject: Re: [wp-hackers] Fixing some SSL cases for 3.4

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
>
_______________________________________________
wp-hackers mailing list
wp-hackers at lists.automattic.com
http://lists.automattic.com/mailman/listinfo/wp-hackers

More information about the wp-hackers mailing list