Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:94532
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.216.182 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CADqTB_ikRepsbO_9vhw568=SaypdPh-z+FzOEr4yji1LibVbjQ@mail.gmail.com>
References: <CAEDdnaWVYbh=t1-bvDh9fixCoC7Be4asoEus=dRhGggx_9VLNg@mail.gmail.com>
 <CADqTB_ikRepsbO_9vhw568=SaypdPh-z+FzOEr4yji1LibVbjQ@mail.gmail.com>
Date: Sun, 17 Jul 2016 19:47:49 +0500
Message-ID: <CAEDdnaVtb8eboyNpr8CjzysHvKiG26bHPy7eiH76PVMPrYNW+A@mail.gmail.com>
To: PHP Internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=001a113d3aac8ee27c0537d5efa7
Subject: Re: [PHP-DEV] [RFC] New operator for context-dependent escaping
From: michael.vostrikov@gmail.com (Michael Vostrikov)

--001a113d3aac8ee27c0537d5efa7
Content-Type: text/plain; charset=UTF-8

>
> 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.

--001a113d3aac8ee27c0537d5efa7--