Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:94622
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 74.125.82.44 as permitted sender)
To: internals@lists.php.net
References: <CAEDdnaWVYbh=t1-bvDh9fixCoC7Be4asoEus=dRhGggx_9VLNg@mail.gmail.com>
Message-ID: <8a39df34-4a23-c496-15f6-20a62d27fc59@gmail.com>
Date: Thu, 21 Jul 2016 23:44:40 +0100
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101
 Thunderbird/45.2.0
MIME-Version: 1.0
In-Reply-To: <CAEDdnaWVYbh=t1-bvDh9fixCoC7Be4asoEus=dRhGggx_9VLNg@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: rowan.collins@gmail.com (Rowan Collins)

On 16/07/2016 16:33, Michael Vostrikov wrote:
> Operator has the following form:
>
> <?* $str ?>
> <?* $str, 'html' ?>
> <?* $str, 'js | html' ?>
[...]
> Operator is compiled into the following AST:
>
> echo PHPEscaper::escape(first_argument, second_argument);
>

Hi Michael,

I'm coming around to the need for (or at least value of) this operator, 
but I'm not keen on the details of the current draft.

For this operator to meet its stated aims it needs to be simple, 
obvious, and easier to get right than wrong; and the current proposal 
feels like it fails on all counts.

- The trick with the magic class name and namespace aliasing is neat, 
but feels likely to confuse a lot of users.
- The basic <?* $foo ?> syntax seems OK, but sticking the escaping types 
after the output makes it hard to spot what's going on with anything 
other than a simple variable. e.g. <?* 
$this->renderView($thing->getViewName(), 'html'), 'js' ?>
- Because it's not obviously part of the <?* operator, someone might 
think the escape parameter can be used elsewhere: <?php echo 
'<blink>oops</blink>', 'html' ?> Operators don't normally have a list of 
arguments.
- The fact that the escape filter is a string, or any kind of 
expression, compounds this. What happens if you mistype the argument? 
<?* $foo, 'hmtl' ?> Or with the current proposal's use of '|', what if 
you get that syntax wrong? <?* $foo, 'html || js' *?> <?* $foo, 'html, 
js' ?>


Now, I'm not saying this just to demolish the current proposal, but if 
the idea is to make escaping second nature, I think the syntax needs to 
be much more obvious, and much more "special". (If we make it too 
flexible, we're basically inventing a new templating language, and we're 
not actually short of those.)

I quite like Mathieu's <?[$escaper...]= $data ?> suggestion, but not the 
free-form callable part (if you're mentioning the whole function name, 
you can just call it already).

How about using a compound like <?*= and putting the list of filters up 
front?

<?*= $foo ?> (defaulting to HTML escape)
<?*html= $foo ?>
<?*js= $foo ?> (JS escape only; not sure if this should encode as JSON, 
or just a JS safe string; maybe <?*json= $foo ?> as well / instead...)
<?*js*html= $foo ?> (JS escape and then HTML escape)

The common escape types should be built in, with maybe a function to 
override them and register additional types, similar to 
stream_wrapper_register. The biggest use case for this is people who 
*aren't* using a framework, which would have either a templating engine 
or a bunch of escaping helpers already, so customising the definitions 
is going to be the exception, not the rule.

The biggest barrier, though, remains that <?= $something ?> will still 
work, and will still be used in examples people see, so people will 
still be in the habit of using the unsafe operator. It's pretty marginal 
whether remembering to reach for <?* $something ?> is actually any 
easier than remembering to reach for <?= h($foo) ?>

Regards,

-- 
Rowan Collins
[IMSoP]