RFC: removing some boilerplate: dont_hack, $probelang

Discussion corner for Developers of Serendipity.
Post Reply
mt2
Regular
Posts: 9
Joined: Fri Jul 13, 2007 1:59 pm
Contact:

RFC: removing some boilerplate: dont_hack, $probelang

Post by mt2 »

I propose two minor changes to remove some cruft from plugin scripts.

A single dont_hack() could replace the three-liner if(IN_serendipity){}. This is some PhpNuke-era inline code execution prevention boilerplate. IMO this has no place in properly architected apps, but I realize some plugins might indeed still have some autoexecuting code.
By switching to a function like "dont_hack()" you basically get a fatal error if that wasn't defined before. So in essence a simple function call replaces the manual constant check. Also, I'd use less pity wording for the die() error message, or rather a different name for this replacement function. Name suggestions!

The $probelang code can also be centralized easily. These 7 lines in every plugin could be supplanted by a simple include_lang(__FILE__). Additionally this prepares Serendipity for a transition off oldschool translation constants. (I have something gettext-esque in mind.)

Both changes are simple and non-intrusive enough, but I'd volunteer a patch or branch, of course. <- temporary developer self-invite ;)
garvinhicking
Core Developer
Posts: 30022
Joined: Tue Sep 16, 2003 9:45 pm
Location: Cologne, Germany
Contact:

Re: RFC: removing some boilerplate: dont_hack, $probelang

Post by garvinhicking »

Hi!

Hm, personally I prefere a customized output instead of triggereing some PHP error reporting (which might even disclose some information).

I do agree that a better architecture would have been to make the plugin files not autoexecute any code, and instead rely on calling methods/functions for each action. But now we already have too many plugins that work well ;)

I agree about a nice probleang function, though I worry about forcing a new serendipity release then on everyone who wants to just update a plugin. This isn't backwards-compatible, and in the past s9y has tried to get maximum functionality without harrassing users to force upgrades on them, or to make existing code unusable. I do see benefit in this, and cleaniness, but IMHO it alone does not suffice this BC break. What do others say about this?

Regards,
Garvin
# Garvin Hicking (s9y Developer)
# Did I help you? Consider making me happy: http://wishes.garv.in/
# or use my PayPal account "paypal {at} supergarv (dot) de"
# My "other" hobby: http://flickr.garv.in/
onli
Regular
Posts: 2828
Joined: Tue Sep 09, 2008 10:04 pm
Contact:

Re: RFC: removing some boilerplate: dont_hack, $probelang

Post by onli »

Hi
Having plugins unupdateable only because of this is in my opinion not worth the spared few lines of code. But if combined with a better translation system (with which advantages?) it should be done - maybe s9y 2.0 really should break some backwards-compatibily (templates/default also needs an overhaul and there are probably some more things...)?
yellowled
Regular
Posts: 7111
Joined: Fri Jan 13, 2006 11:46 am
Location: Eutin, Germany
Contact:

Re: RFC: removing some boilerplate: dont_hack, $probelang

Post by yellowled »

I really can't contribute much on this, but this:
onli wrote:templates/default also needs an overhaul
is something I also strongly recommend. (And, yes, I would enjoy contributing to this.)

templates/default is over six years old now. The author is no longer involved w/ s9y. It has a partially table-based layout, for crying out loud!

This needs to be adressed, even if it breaks older templates. (Which, make no mistake, it will do. But at some point, even the most enthusiastic user of the mt-* templates will have to switch.)

YL
mt2
Regular
Posts: 9
Joined: Fri Jul 13, 2007 1:59 pm
Contact:

Re: RFC: removing some boilerplate: dont_hack, $probelang

Post by mt2 »

Hey,

That's a serious objection. I didn't actually think about the detailed setup disclosure along with the dont_hack() fatal error message. The pathname isn't probably that overly security relavent. But it reveals customer or account identifiers for most shared hosting servers, which can often be abused.

So, then my recommendation would shift towards a silent fail with @dont_hack(). Really I see no point in customized output for enhanced usability for hackers. So, a silent fail or gibberish output is functionaly sufficient here.

As for the $probelang transition, I understand the compatibility woes. However, there is no point in changing everything at once. It's sufficient to introduce the new scheme in the core, then in the default plugins, and wait another release to transition the remaning plugins. Old plugins wouldn't be affected by new S9Y releases, the old boilerplate code continues to function. And compatibility efforts shouldn't lead to a full no-progress policy.

Is there a method in the spartacus repository to log the inquiring S9Y installation versions? It would be somewhat relevant to the discussion how old installations are (e.g. 50% of userbase still on < 1.4).
But otherwise, a weasiling transition is the way to go for such changes. It doesn't add any user-visible functionality; it's just a very very low level code cleanup and in itself doesn't even benefit plugin developers much.

G!
onli
Regular
Posts: 2828
Joined: Tue Sep 09, 2008 10:04 pm
Contact:

Re: RFC: removing some boilerplate: dont_hack, $probelang

Post by onli »

It's sufficient to introduce the new scheme in the core, then in the default plugins, and wait another release to transition the remaning plugins.
Sound pretty reasonable. That way, It's not even necessary that way to break it at all (aside from the other points speaking for that) - as long as the old system remains useable, it'd be possible to change only the core-plugins and those which need an actual version anyway.
garvinhicking
Core Developer
Posts: 30022
Joined: Tue Sep 16, 2003 9:45 pm
Location: Cologne, Germany
Contact:

Re: RFC: removing some boilerplate: dont_hack, $probelang

Post by garvinhicking »

Hi!

I agree, that's a good starting point. I added a new "load_language" method to the plugin API that does the language file loading. The upside of this is that we can use it as "dont_hack" function already, so two flies with one stone. :-)

Spartacus file downloads are currently being done over the Repository, so we don't have means of introspecting the heritage sadly, so we can't really tell how many people have older versions. I can only base it on my personal experience here on the years supporting. I do think we'll get there over time, combining a "moratorium" and progress. First step is done, thanks to your effort. :-)

Regards,
Garvin
# Garvin Hicking (s9y Developer)
# Did I help you? Consider making me happy: http://wishes.garv.in/
# or use my PayPal account "paypal {at} supergarv (dot) de"
# My "other" hobby: http://flickr.garv.in/
Post Reply