> Registry of functions - is exactly how escaping is performed in Symfony
and Twig.

For one, that does not mean it's a good idea.

For another, the registry in Symfony (at least, I don't know about Twig) is
inside an instance - it's not global state.

Do you get my point that a reference to a closure is state? And if it's
global state, that's extremely bad - the entire PHP community is fighting
like hell to avoid that, with PSR-7 and layers of abstraction on top of,
well, everything, in order to make code testable.

Catering to different skill levels is no excuse.

HTML escaping is, yes, a very pragmatic task - it's also solved already,
with htmlspecialchars() ... the main problem you appear to be solving, is
that htmlspecialchars() is too long and ugly and inconvenient, which, okay,
it is - but adding a global registry for that is overkill, and the whole
problem would go away if you could simply autoload functions:

    <h1><?= html($title) ?></h1>

That's not ugly or inconvenient. The only problem is you can't package your
html() function (and install it with e.g. Composer) because PHP can't
autoload functions.

Functions such as HTML escaping, or any other kind of escaping, do not
belong in a registry - you shouldn't swap out those functions at all, they
need to work precisely as specified, so the caller knows precisely what the
result it, because only the caller can know the context and intent.

I'm not going to go much deeper into it than that, sorry - I don't have
time...


On Sun, Jul 17, 2016 at 4:47 PM, Michael Vostrikov <
michael.vostri...@gmail.com> wrote:

> >
> > All it is, really, is a registry for functions, and syntactic sugar for
> > calling those functions - it's unnecessary, it's more global state you
> have
> > to manage and it's the kind of superficial convenience that will end up
> > breeding more complexity.
> >
>
> Registry of functions - is exactly how escaping is performed in Symfony and
> Twig.
>
> https://github.com/symfony/symfony/blob/f29d46f29b91ea5c30699cf6bdb8e65545d1dd26/src/Symfony/Component/Templating/PhpEngine.php#L421
>
> https://github.com/twigphp/Twig/blob/f0a4fa678465491947554f6687c5fca5e482f8ec/lib/Twig/Extension/Core.php#L1039
>
>
> What's also strange, is the ability to call functions in this registry
> > hinges on syntax. What if I want to call the registered functions from
> > within code?
> > ob_start();
> > ?><* $text *><?
> > $html = ob_get_clean();
> >
>
> Sorry, I don't understand what do you mean in this example. You can call
> your escapers by name or by callable value from array of handlers.
>
> <?php
>
>     function myHtmlEscaper($str) {
>         return htmlspecialchars($str, ENT_QUOTES | ENT_HTML5 |
> ENT_DISALLOWED | ENT_SUBSTITUTE);
>     }
>
>     PHPEscaper::registerHandler('html', myHtmlEscaper);
>     $text = '"Test"';
>
>     // --------
>
>     ob_start();
>     ?><* $text *><?         // do you mean a call of escaper here?
>     $html = ob_get_clean();
>     var_dump($html);
>
>     // string(11) "<* $text *>"
>
>     // --------
>
>     ob_start();
>     ?><?* $text ?><?
>     $html = ob_get_clean();
>     var_dump($html);
>
>     // string(16) "&quot;Test&quot;"
>
>     // --------
>
>     $escapers = PHPEscaper::getHandlers();
>     $htmlEscaperCallable = $escapers['html'];
>     ob_start();
>     echo $htmlEscaperCallable($text);
>     $html = ob_get_clean();
>     var_dump($html);
>
>     // string(16) "&quot;Test&quot;"
>
> ?>
>
>
> > Both variants <?= h($something) ?> and <?= $something ?> work good.
> > This is so true - and the whole syntactic convenience line of thinking
> > really should end with that.
> >
>
> Wrong and unsafe variant should not work good.
>
>
> > Also there is a problem with function autoloading.
> > I maintain that this is the real problem, and perhaps the only problem -
> > all this RFC does, is provide a stop-gap solution.
> >
>
> This RFC is not related to function autoloading. It just does not have this
> problem. The code becomes the same as if we will write PHPEscaper::escape()
> manually. In the static calls there is no problem with autoloading.
>
>
> It's somehow easier to choose between two different characters * and =
> > versus electing to call a function or not?
> >
>
> Unsafe variant 'not to call a function' is a short subset of safe variant
> 'call a function'. Safe variant requires additional actions. With new
> operator safe variant is as easy as unsafe.
> And we must not choose, operator <?* ?> must be used everywhere, except
> 1-2% of cases where we have ready HTML.
> And if we accidentally use it for HTML, this will not be an XSS, it just
> will show double-encoded content, and this will be noticeable.
>
>
> All this RFC changes is the syntax - not the problem.
> >
>
> Ok, what do you think is the problem? As I think, the problem is correct
> HTML escaping and XSS. And it can be solved the same way as in template
> engines. I don't suggest to bring the whole template engine into PHP, only
> escaping mechanism. Function autoloading - is another problem.
>
>
> Addition of a feature like this will affect even those who don't use it
> >
>
> It does not affect any existing code. It is not a replacement for <?= ?>
> operator.
>
>
> a feature like this will bring global state, side-effects and many other
> > interesting problems
> >
>
> Default implementation of PHPEscaper is not required. This is possible to
> fully remove it and use custom PHPEscaper written on PHP. Or just don't use
> this operator and don't worry about escaping.
> Ok, could you please give an example of the problem?
>
>
> when they inherit or consume code that does
> >
>
> I think this is the same as if they inherit or consume code that uses Twig
> or Smarty.
>
>
> you're asking specifically about the proposed feature, rather than asking
> > in general about the problem
> >
>
> The proposed feature solves the problem with HTML escaping. I asked about
> presence of problem and about a solution.
> Also, in the previous discussion I got the first answer that "Main issue
> with this kind of proposals is that escaping is context-dependent". This
> issue is mentioned in many other discussions. And I suggested a solution
> for this problem too.
>
> And that's why I asked about an official poll in the first message of this
> discussion. This is a serious change and we need to know community opinion
> about it.
>
>
> reasoning about the impact on the language, or deeper problems requires
> > more of an involvement
> >
>
> And RFC is intended for this, isn't it? Could you give an example of impact
> which can bring the problems?
>
>
> > More than 90% of output data - is data from DB and must be HTML-encoded
> > Yet, you argue we need a function registry for all kinds of other escape
> > operations to address the other 10%
> >
>
> Hm, wait please. Initially I suggested an operator specially for HTML
> escaping. I tried to demonstrate that this is special context and it
> deserve its own operator. I got the answer that there are many other
> contexts and making an operator for one context is a bad idea. Ok, I
> suggested the solution which allows and HTML escaping, and usage of other
> contexts. Let's define, what is more suitable - many contexts or one
> context.
>
>
> you argue we need a function registry
> >
>
> Function registry is just a default implementation of PHPEscaper class. It
> can be used 'out of the box' and provides HTML escaping by default. Default
> implementation can fully be removed from the project, and replaced with
> custom implementation - e.g. with limited set of methods. There is just one
> static call, which does not differ from function call, but allows
> autoloading.
>
>
> which could be more generally addressed by solving autoloading
> >
>
> I can only repeat that the problem is not that we don't have a function. We
> have a function and can write own functions. Maybe there are little issues
> with direct usage of static class method, but it can be solved by defining
> a function in global namespace, which will call this method, as it is done
> in some frameworks.
>
>
> Please, let's focus on improving the language in general - rather than
> > improving one isolated use-case.
> >
>
> Why do you call 90% of output data a 'one isolated use-case'?) I can only
> mention here the words which were shown to me when I created the RFC:
>
> Quoting Rasmus:
>     PHP is and should remain:
>     1) a pragmatic web-focused language
>     2) a loosely typed language
>     3) a language which caters to the skill-levels and platforms of a wide
> range of users
>
> HTML escaping is a very pragmatic task.
>

Reply via email to