Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:94610
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain rochette.cc designates 62.210.206.189 as permitted sender)
To: internals@lists.php.net
References: <CAEDdnaWVYbh=t1-bvDh9fixCoC7Be4asoEus=dRhGggx_9VLNg@mail.gmail.com>
 <CAEDdnaVtb8eboyNpr8CjzysHvKiG26bHPy7eiH76PVMPrYNW+A@mail.gmail.com>
 <CADqTB_jV9-SQmLknWoXx_uC5ThHynSfqcTdUC+E2TwirOpXR8g@mail.gmail.com>
 <CAOsHV+ujFjKP8AL+R171pFTPnwfN2nC7_TViD=qZtOVGEDyebA@mail.gmail.com>
 <CAEDdnaVFzxq5PEXMAkuAYWg9zeL4yWz2pLpv6RyCcB-rvZJ_RA@mail.gmail.com>
 <CAEDdnaUJ4y_1+hVY6cguRivZnvQTCt0=rhVz=nJSPkqJ7aASuA@mail.gmail.com>
 <CAEDdnaV_awP5g4Ee-acVxd7+LYWot2K0-ZEUbWCzc6me5vAFmw@mail.gmail.com>
 <CABdc3WoyJUGSE688S2A_wh064bgmdkPMKgRM31T56pex5Xry+w@mail.gmail.com>
 <CAEDdnaVqzP6oOjcQWWNGenYHgJtNDvCgi_J-y-fsr_bFv3FW9g@mail.gmail.com>
 <CABdc3WrwCcWzbmJC+JRZqZ0HcHj2pDeEO9MfdLUz5V23TGrGRQ@mail.gmail.com>
 <CABdc3WpGYg5t=WRJP0+Z28qfXJ2A+w-7qQC86umCWwFX+hoRcw@mail.gmail.com>
 <CAEDdnaX0-iOcA5PEEqx_nnt1ASMjt_hU1959O69eMoFHZdUH-w@mail.gmail.com>
 <CAObuZdv+UJsSLeNAPf5s5AXh0sMHG+VG9Ai=5m5y2tu9h6vZAA@mail.gmail.com>
 <CAEDdnaUfdUvqKR6jqpoC8Z65VXHSSY6K1buAeT94cdsJHy3-HA@mail.gmail.com>
Message-ID: <6cffa6e9-b3b7-cedf-ccb7-cb342c768a4f@rochette.cc>
Date: Thu, 21 Jul 2016 13:35:39 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:48.0) Gecko/20100101
 Thunderbird/48.0a2
MIME-Version: 1.0
In-Reply-To: <CAEDdnaUfdUvqKR6jqpoC8Z65VXHSSY6K1buAeT94cdsJHy3-HA@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] [RFC] New operator for context-dependent escaping
From: mathieu@rochette.cc (Mathieu Rochette)



On 20/07/2016 21:55, Michael Vostrikov wrote:
>> <?= $this->escapeHtml($value); ?>
>> I don't see what is hard in using that syntax, plus it's not a global
> registry.
>> if people aren't using templating and haven't written any of their own
> wrappers to sanitize the output
>
> They HAVE own wrappers. The problem is that unsafe variant works good, but
> unsafe variant should not work good.
I suppose you mean that <?= $row['data'] ?> is the unsafe variant.
How does this rfc makes it not "works good" ? people will still have to 
think escaping their data. And they should: I don't know of any proposal 
that makes the safe choice on behalf of the user correctly

anyway, helping escaping data is a good goal and having something easier 
than htmlspecialchars would definitely help IMHO

that's what is actually safe without helper or additional template 
engines : <?= htmlspecialchars($row['data'], ENT_QUOTES | ENT_SUBSTITUT) ?>
and with a helper: <?= $this->html($row['data']) ?>
here is what it could look like with the rfc : <?* $row['data'], 'html' ?>
and what it might look like with function autoloading: <?= 
html($row['data']) ?>

that's why people here are talking about function autoloading, it's not 
what this rfc is about but a lot think it would be a better solution 
(and would also help in many other places)
speaking only for me now, between the function autoloading and this rfc, 
I do prefer the function autoloading. but, I still think the helper 
solution is best: easier to mock and thus to customize, extend or 
configure (eg: differents htmlspecialchars flags)

something else about the rfc:
 > It may seem that different escaping is required here. But it's not. 
The call of htmlspecialchars() is required in all 3 cases

I disagree with that, htmlspecialchars is needed in all 3 cases, but 
it's not enough
To be a valid page, the html should be valid, and the javascript in it 
should be too. htmlspecialchars + json_encode is the correct way to 
encode a javascript string inside an html page. if you only use 
htmlspecialchars you have valid html, but the page in itself might not 
be valid and you could inject javascript code that way. in other words, 
it's also a security issue to forget to call json_encode there.
That's not much of an issue anyhow as your rfc handle context stack anyway

One good thing I like about having another syntax <?*$data, $context ?> 
or whatever is that you could configure your linter to throws errors 
when using <?= $data ?>
And that's not something that could be done easily with function 
autoloading or template helpers

and lastly about the syntax, I'm not a huge fan of separating the data 
from the context using a comma, I was thing about something like that 
instead:

<?[$escaper...]= $data ?>

where $escaper is a callable eg:

<?[$this->html]= $data ?>
<?['e::html']= $data ?>
<?[new Escaper\HTML(ENT_QUOTES | ENT_SUBSTITUT)]= $data ?>// you should 
not do that
<?[function ($data) {return htmlspecialchars($data, ENT_QUOTES | 
ENT_SUBSTITUT);}]= $data ?> // you should not do that
<?['e\html']= $data ?> // with function autoloading

or multiple escapers:

<?['e::html', 'e::attr']= $data ?>
<?['e::html', 'e::json']= $data ?>

and if you want to say "I know this data is already escaped":

<?[]= $data ?>

it does not need a global state and will be able to take advantage of 
new features and uses standard procedure to "register" callable escapers

<?['e::html', 'e::attr']= $data ?> would mostly equivalent to <?= 
array_reduce(array_reverse(['e::html', 'e::attr']), function ($data, 
$escape) {return $escape($data);}, $data) ?>

it does not need a global state as it uses standard procedure to 
"register" callable escapers and will still be able to take advantage of 
new features

> The problem is that they have to keep in mind that they need always write a
> wrapper, they always need to repeat the same action again. And somewhen
> they just miss this and get possible problems with security.
> The problem is that this is very frequent case, so we need a tool for this
> case, which will prevent wrong work (XSS in particular).
>
> We always perform an output to some context. Why just not to add an easy
> tool for work with contexts?
> This is just one call - PHPEscaper::escape(). Consider it like a Facade
> pattern.
>
>
>> what's going to drive them to change?
> Why then many people are asking about this feature?
> In my RFC there are 10 links to similar discussions. They are from those
> people who was not lazy to write to mailing list or bug-tracker.
> There are also the results of the poll - 286 people are for this operator
> (now 320, after I wrote about implementation with contexts).
>
> And in first message I asked about official poll from PHP developers. Why
> not conduct it?
>