Go to main content

Textpattern CMS support forum

You are not logged in. Register | Login | Help

#11 2012-07-25 23:19:29

Bloke
Developer
From: Leeds, UK
Registered: 2006-01-29
Posts: 9,719
Website

Re: Plugin filtering for admin list pages

Gocom wrote:

If the callback_event is placed at the beginning of the statement

Hadn’t thought of doing that. I guess it’s the difference between:

$criteria .= callback_event('list', 'criteria');

and

$criteria = callback_event('list', 'criteria') . $criteria;

Is one way “better” than the other? Dunno. I suppose the first one is more logical as it appends to what is already there and places less burden on the developer to remember the trailing operator, but as you say, as long as the rules are known then either would work. Toss a coin for it? :-)

I could stuff this in now on all ‘list’ pages, but does it class as a new feature when we’re technically in feature lockdown? Or is it a relatively minor addition that we can sneak in without too much worry?

Last edited by Bloke (2012-07-25 23:20:11)


The smd plugin menagerie — for when you need one more gribble of power from Textpattern. Bleeding-edge code available on GitHub.

Txp Builders – finely-crafted code, design and Txp

Offline

#12 2012-07-25 23:54:53

Gocom
Plugin Author
From: Helsinki, Finland
Registered: 2006-07-14
Posts: 4,533
Website

Re: Plugin filtering for admin list pages

Bloke wrote:

Is one way “better” than the other?

Hmm. The first, no the second.. on second thought the first, no wait… Both…? The more, the better, right?

I could stuff this in now on all ‘list’ pages, but does it class as a new feature when we’re technically in feature lockdown? Or is it a relatively minor addition that we can sneak in without too much worry?

You could classify it as a minor addition, me thinks. New, very great feature nonetheless. Opens new possibilities, that’s for sure.

Offline

#13 2012-07-26 00:22:20

mrdale
Moderator
From: Walla Walla
Registered: 2004-11-19
Posts: 2,203
Website

Re: Plugin filtering for admin list pages

Yay, I think{?}

Offline

#14 2012-07-26 07:11:24

etc
Developer
Registered: 2010-11-11
Posts: 3,858
Website

Re: Plugin filtering for admin list pages

Yay+1!

Bloke wrote:

I could stuff this in now on all ‘list’ pages…

Me feeling is that it could be implemented more widely, probably by introducing callback argument in safe_rows_like functions, why not?

Offline

#15 2012-07-26 08:48:00

Bloke
Developer
From: Leeds, UK
Registered: 2006-01-29
Posts: 9,719
Website

Re: Plugin filtering for admin list pages

etc wrote:

Me feeling is that it could be implemented more widely, probably by introducing callback argument in safe_rows_like functions, why not?

Do we always know the $event and $step in safe_* functions? I’m not convinced we do. If not we can’t very easily distinguish between one callback and another.


The smd plugin menagerie — for when you need one more gribble of power from Textpattern. Bleeding-edge code available on GitHub.

Txp Builders – finely-crafted code, design and Txp

Offline

#16 2012-07-26 11:30:54

etc
Developer
Registered: 2010-11-11
Posts: 3,858
Website

Re: Plugin filtering for admin list pages

Bloke wrote:

Do we always know the $event and $step in safe_* functions?

Not internally, but we can pass them to safe_* as appropriate. For example, we could call safe_count('textpattern', "$criteria", $debug, 'list.criteria'); in list_list(), so safe_count() would know that it has to fire $where = callback_event('list', 'criteria') . $where; before querying the db.

Last edited by etc (2012-07-26 11:31:51)

Offline

#17 2012-07-26 11:49:45

Bloke
Developer
From: Leeds, UK
Registered: 2006-01-29
Posts: 9,719
Website

Re: Plugin filtering for admin list pages

etc wrote:

Not internally, but we can pass them to safe_* as appropriate.

OK. So this possible extra param that we split at . inside safe_* functions:

  1. What impact on performance would it have to detect the presence of the 5th param and issue the callback at this level?
  2. I assume we would only fire the callback if the event/step param was present?
  3. If so, would this mitigate any potential attack vectors from the public side, given that safe_* are used extensively in, say, taghandlers and commenting? Could someone potentially inject/fake the 5th param (somehow?) and trigger the callback from the public side?

It seems nice to do it at the DB level as it opens up all sorts of possibilities but I’d need to be sure it’s controllable, safe and still fast before going this route. Plus it may mean waiting for the next release because it’s a broader change than just adding callbacks to the list pages directly.


The smd plugin menagerie — for when you need one more gribble of power from Textpattern. Bleeding-edge code available on GitHub.

Txp Builders – finely-crafted code, design and Txp

Offline

#18 2012-07-26 12:57:41

etc
Developer
Registered: 2010-11-11
Posts: 3,858
Website

Re: Plugin filtering for admin list pages

  1. Very small per se, but given that many functions (like list_list) do multiple calls to safe_*, each registered plugin will be called as many times, so this is a real issue.
  2. Yes, certainly, unless the performance impact is so low that it is worth doing systematically. event, step and so on could be retrieved from global variables.
  3. We could introduce safe_*_with_callback functions on the admin side to mitigate this hypothetical risk (I have no idea how a client could fake a param).

But you have convinced me, I am not convinced anymore in its utility. May be we should wait to see if some usecases arise, and certainly until some post 4.5 release.

Offline

#19 2012-07-26 13:40:34

Bloke
Developer
From: Leeds, UK
Registered: 2006-01-29
Posts: 9,719
Website

Re: Plugin filtering for admin list pages

etc wrote:

event, step and so on could be retrieved from global variables.

/me puts on his nervous trousers

I have no idea how a client could fake a param

Nor do I! Was just throwing it out there in case it was a distant possibility via some PHP / global / cookie / injection wrangling.

I could certainly put this in place now on the list pages — i.e. not at the safe_* level. Worst (or best?) case scenario is that the calls move to the safe_* level in some future release; at that point, from an outside plugin perspective, very little would change in real terms as the callback event/step would stay the same for any given Page. Plugin callbacks would just be called more often per page, with the same event/step, so the output from the callback would remain the same in each safe_* call.

Last edited by Bloke (2012-07-26 13:41:26)


The smd plugin menagerie — for when you need one more gribble of power from Textpattern. Bleeding-edge code available on GitHub.

Txp Builders – finely-crafted code, design and Txp

Offline

#20 2012-07-26 16:28:20

etc
Developer
Registered: 2010-11-11
Posts: 3,858
Website

Re: Plugin filtering for admin list pages

Bloke wrote:

/me puts on his nervous trousers

Man! I use these global $prefs on nearly everyday basis, and no one ever alerted me I needed a special equipment?

List pages are fine, great if you can do it right now. But I meant this could be useful in some other places, where a regular callback or pluggable_ui is an overkill. If someone needs to restrict the Section list on the Write tab according to the author privs, for example. But safe_* level is finally not a good idea for the moment, because of the callback redundancy, you are right (and I am wrong).

Offline

Board footer

Powered by FluxBB