Everything
php
Learn Community Releases The PHP Project
Home » Project » Newsgroups » php.internals » [RFC] Escaping RFC for PHP Core - Updates? »

[RFC] Escaping RFC for PHP Core - Updates?

php.internals

Yasuo Ohgaki

12 years ago
Hi, Any updates for "Escaping RFC for PHP Core"? https://wiki.php.net/rfc/escaper I think this is must have item for next PHP.
-- Yasuo Ohgaki yohgaki@ohgaki.net

Leigh

12 years ago
On Sep 7, 2013 4:37 AM, "Yasuo Ohgaki" <yohgaki@ohgaki.net> wrote:
> > Hi, > > Any updates for "Escaping RFC for PHP Core"? > https://wiki.php.net/rfc/escaper > > I think this is must have item for next PHP. > > -- > Yasuo Ohgaki > yohgaki@ohgaki.net
Looks like the rfc author was unable to implement it himself at the time of the proposal. The last thing in the discussion thread looks like "implement it in PECL first, and it might get bundled later" http://marc.info/?l=php-internals&m=134822086426610&w=2

Yasuo Ohgaki

12 years ago
Hi Leigh, On Sat, Sep 7, 2013 at 2:56 PM, Leigh <leight@gmail.com> wrote:
> Looks like the rfc author was unable to implement it himself at the time > of the proposal. > > The last thing in the discussion thread looks like "implement it in PECL > first, and it might get bundled later" > > http://marc.info/?l=php-internals&m=134822086426610&w=2 >
Thank you for the info. I searched my mailbox, but I couldn't find this. It would be better to implement this as SPL_Escape class. SPL_Escape::jsString() SPL_Escape::phpString() SPL_Escape::html() SPL_Escape::htmlAttribute() etc It looks nicer than Escaper::escapeJs(), Escaper::escapeHtml(), etc. Any comments? Anyone mind if I edit the RFC? Regards,
-- Yasuo Ohgaki yohgaki@ohgaki.net

Pierre Joye

12 years ago
Hi, On Sep 7, 2013 10:35 AM, "Yasuo Ohgaki" <yohgaki@ohgaki.net> wrote:
> > Hi Leigh, > > On Sat, Sep 7, 2013 at 2:56 PM, Leigh <leight@gmail.com> wrote: > > > Looks like the rfc author was unable to implement it himself at the time > > of the proposal. > > > > The last thing in the discussion thread looks like "implement it in PECL > > first, and it might get bundled later" > > > > http://marc.info/?l=php-internals&m=134822086426610&w=2 > > > Thank you for the info. I searched my mailbox, but I couldn't find this. > > It would be better to implement this as SPL_Escape class. > > SPL_Escape::jsString() > SPL_Escape::phpString() > SPL_Escape::html() > SPL_Escape::htmlAttribute() > etc > > It looks nicer than Escaper::escapeJs(), Escaper::escapeHtml(), etc. > > Any comments? > Anyone mind if I edit the RFC?
I like the goal of this proposal. It would however fits much better in ext/filter. Yes, escaping has different purposes than filtering. I have some worries about the implementation. It is not an easy task and some external libraries may already have these features (esp. CSS or JS). About the API: To have a kind of wrapper class is not very useful. It will most likely end with static methods with a large set of arguments or options array, not very sexy. Cheers, Pierre

Yasuo Ohgaki

12 years ago
Hi Pierre, On Sat, Sep 7, 2013 at 5:51 PM, Pierre Joye <pierre.php@gmail.com> wrote:
> I like the goal of this proposal. > > It would however fits much better in ext/filter. Yes, escaping has > different purposes than filtering. > > I have some worries about the implementation. It is not an easy task and > some external libraries may already have these features (esp. CSS or JS). > > About the API: > > To have a kind of wrapper class is not very useful. It will most likely > end with static methods with a large set of arguments or options array, not > very sexy. >
I agree. It would end up with static methods and not cool. If it's going to be static method only calls, usual functions are better. ext/filter is a good option. I'll add this in the RFC as a choice. Regards,
-- Yasuo Ohgaki yohgaki@ohgaki.net

Levi Morrison

12 years ago
> It looks nicer than Escaper::escapeJs(), Escaper::escapeHtml(), etc. > > Any comments?
Please, don't go down this route. You do not want one class to escape all kinds of data; delegate each type of escaping to its own class: JavaScriptEscaper->escape(); PhpEscaper->escape(); HtmlEscaper->escape(); HtmlAttributeEscaper->escape(); I should not have to defend this but I am willing to explain in more detail if someone would like me to.

Michael

12 years ago
On 07/09/2013 15:41, Levi Morrison wrote:
>> It looks nicer than Escaper::escapeJs(), Escaper::escapeHtml(), etc. >> >> Any comments? > > > Please, don't go down this route. You do not want one class to escape all > kinds of data; delegate each type of escaping to its own class: > > JavaScriptEscaper->escape(); > PhpEscaper->escape(); > HtmlEscaper->escape(); > HtmlAttributeEscaper->escape(); > > I should not have to defend this but I am willing to explain in more detail > if someone would like me to. >
There doesnt need to be any object-oriented version for this problem. It's a series of pure functions. Wraping them in one or more classes adds nothing. Michael

Nikita

12 years ago
On Sat, 07 Sep 2013 20:08:45 +0400, Michael John Burgess <michael@mjburgess.co.uk> wrote:
> On 07/09/2013 15:41, Levi Morrison wrote: >>> It looks nicer than Escaper::escapeJs(), Escaper::escapeHtml(), etc. >>> >>> Any comments? >> >> >> Please, don't go down this route. You do not want one class to escape >> all >> kinds of data; delegate each type of escaping to its own class: >> >> JavaScriptEscaper->escape(); >> PhpEscaper->escape(); >> HtmlEscaper->escape(); >> HtmlAttributeEscaper->escape(); >> >> I should not have to defend this but I am willing to explain in more >> detail >> if someone would like me to. >> > > > There doesnt need to be any object-oriented version for this problem. > It's a series of pure functions. Wraping them in one or more classes > adds nothing. > > Michael >
Hi, Wrapping those functions in methods means they can be extended in child classes. So suppose you have some library that takes object of type Spl_Escaper and uses its methods for escaping some data. Now if you will need some additional escaping you just need to make child class for Spl_Escaper and override methods which behavior you need to change. This can't be done with pure functions (in PHP).

Levi Morrison

12 years ago
On Sat, Sep 7, 2013 at 10:36 AM, Nikita Nefedov <inefedor@gmail.com> wrote:
> On Sat, 07 Sep 2013 20:08:45 +0400, Michael John Burgess < > michael@mjburgess.co.uk> wrote: > > On 07/09/2013 15:41, Levi Morrison wrote: >> >>> It looks nicer than Escaper::escapeJs(), Escaper::escapeHtml(), etc. >>>> >>>> Any comments? >>>> >>> >>> >>> Please, don't go down this route. You do not want one class to escape all >>> kinds of data; delegate each type of escaping to its own class: >>> >>> JavaScriptEscaper->escape(); >>> PhpEscaper->escape(); >>> HtmlEscaper->escape(); >>> HtmlAttributeEscaper->escape()**; >>> >>> I should not have to defend this but I am willing to explain in more >>> detail >>> if someone would like me to. >>> >>> >> >> There doesnt need to be any object-oriented version for this problem. >> It's a series of pure functions. Wraping them in one or more classes adds >> nothing. >> >> Michael >> >> > Hi, > > Wrapping those functions in methods means they can be extended in child > classes. So suppose you have some library that takes object of type > Spl_Escaper and uses its methods for escaping some data. Now if you will > need some additional escaping you just need to make child class for > Spl_Escaper and override methods which behavior you need to change. This > can't be done with pure functions (in PHP).
You have a flawed understanding of good functional design. Instead of directly calling the escaping function you would simply ask for a callable and pass in the escaping function. Thus, you could use an alternative escaping function at runtime. The methods route is a poor choice. If we use classes at all, separate the responsibility of each type of escaping to a separate class. Escaping JSON and HTML code have little (possibly nothing) in common and do not belong in the same class.

Florin Razvan Patan

12 years ago
On Sat, Sep 7, 2013 at 6:43 PM, Levi Morrison <morrison.levi@gmail.com> wrote:
> On Sat, Sep 7, 2013 at 10:36 AM, Nikita Nefedov <inefedor@gmail.com> wrote: > >> On Sat, 07 Sep 2013 20:08:45 +0400, Michael John Burgess < >> michael@mjburgess.co.uk> wrote: >> >> On 07/09/2013 15:41, Levi Morrison wrote: >>> >>>> It looks nicer than Escaper::escapeJs(), Escaper::escapeHtml(), etc. >>>>> >>>>> Any comments? >>>>> >>>> >>>> >>>> Please, don't go down this route. You do not want one class to escape all >>>> kinds of data; delegate each type of escaping to its own class: >>>> >>>> JavaScriptEscaper->escape(); >>>> PhpEscaper->escape(); >>>> HtmlEscaper->escape(); >>>> HtmlAttributeEscaper->escape()**; >>>> >>>> I should not have to defend this but I am willing to explain in more >>>> detail >>>> if someone would like me to. >>>> >>>> >>> >>> There doesnt need to be any object-oriented version for this problem. >>> It's a series of pure functions. Wraping them in one or more classes adds >>> nothing. >>> >>> Michael >>> >>> >> Hi, >> >> Wrapping those functions in methods means they can be extended in child >> classes. So suppose you have some library that takes object of type >> Spl_Escaper and uses its methods for escaping some data. Now if you will >> need some additional escaping you just need to make child class for >> Spl_Escaper and override methods which behavior you need to change. This >> can't be done with pure functions (in PHP). > > > You have a flawed understanding of good functional design. Instead of > directly calling the escaping function you would simply ask for a callable > and pass in the escaping function. Thus, you could use an alternative > escaping function at runtime. > > The methods route is a poor choice. If we use classes at all, separate the > responsibility of each type of escaping to a separate class. Escaping JSON > and HTML code have little (possibly nothing) in common and do not belong in > the same class.
How about allowing to register a custom escape function for each type which can override the native one or the previous registered one? There should be no way to restore a previous user registered function but there should be a way to reset it to the internal php one. That would solve the described problem as well as keeping it simple. And having this as a class doesn't add anything imho, mainly due to the point described above by Levi. Kind regards ---- Florin Patan https://github.com/dlsniper http://www.linkedin.com/in/florinpatan

Yasuo Ohgaki

12 years ago
Hi, On Sun, Sep 8, 2013 at 1:43 AM, Levi Morrison <morrison.levi@gmail.com>wrote:
> You have a flawed understanding of good functional design. Instead of > directly calling the escaping function you would simply ask for a callable > and pass in the escaping function. Thus, you could use an alternative > escaping function at runtime. > > The methods route is a poor choice. If we use classes at all, separate the > responsibility of each type of escaping to a separate class. Escaping JSON > and HTML code have little (possibly nothing) in common and do not belong in > the same class. >
You are proposing something like $escape_function = function($input) { $output=avaScriptEscaper->escape($input); $output=do_some_additional($output); return $output; } my_escape($some_data, $escape_function) { return $escape_function($some_data); } Is this correct? Regards,
-- Yasuo Ohgaki yohgaki@ohgaki.net

Yasuo Ohgaki

12 years ago
Hi, On Sun, Sep 8, 2013 at 1:36 AM, Nikita Nefedov <inefedor@gmail.com> wrote:
> On Sat, 07 Sep 2013 20:08:45 +0400, Michael John Burgess < > michael@mjburgess.co.uk> wrote: > > On 07/09/2013 15:41, Levi Morrison wrote: >> >>> It looks nicer than Escaper::escapeJs(), Escaper::escapeHtml(), etc. >>>> >>>> Any comments? >>>> >>> >>> >>> Please, don't go down this route. You do not want one class to escape all >>> kinds of data; delegate each type of escaping to its own class: >>> >>> JavaScriptEscaper->escape(); >>> PhpEscaper->escape(); >>> HtmlEscaper->escape(); >>> HtmlAttributeEscaper->escape()**; >>> >>> I should not have to defend this but I am willing to explain in more >>> detail >>> if someone would like me to. >>> >>> >> >> There doesnt need to be any object-oriented version for this problem. >> It's a series of pure functions. Wraping them in one or more classes adds >> nothing. >> >> Michael >> >> > Hi, > > Wrapping those functions in methods means they can be extended in child > classes. So suppose you have some library that takes object of type > Spl_Escaper and uses its methods for escaping some data. Now if you will > need some additional escaping you just need to make child class for > Spl_Escaper and override methods which behavior you need to change. This > can't be done with pure functions (in PHP). > >
I'm in favor of implementing this in ext/filter now, but extend-ability is good benefit.
-- Yasuo Ohgaki yohgaki@ohgaki.net

Levi Morrison

12 years ago
On Sat, Sep 7, 2013 at 10:08 AM, Michael John Burgess < michael@mjburgess.co.uk> wrote:
> On 07/09/2013 15:41, Levi Morrison wrote: > >> It looks nicer than Escaper::escapeJs(), Escaper::escapeHtml(), etc. >>> >>> Any comments? >>> >> >> >> Please, don't go down this route. You do not want one class to escape all >> kinds of data; delegate each type of escaping to its own class: >> >> JavaScriptEscaper->escape(); >> PhpEscaper->escape(); >> HtmlEscaper->escape(); >> HtmlAttributeEscaper->escape()**; >> >> I should not have to defend this but I am willing to explain in more >> detail >> if someone would like me to. >> >> > > There doesnt need to be any object-oriented version for this problem. It's > a series of pure functions. Wraping them in one or more classes adds > nothing.
If you are making a class then do it properly; if you aren't using a class then use sensible functions. Using a poorly designed class as originally proposed is not helping anything and may hurt as well.