[RFC] New operator for context-dependent escaping

1. Is there any specific reason why we're using a class instead of
   functions to register a callable the same way it's done for exception
   handling or error handling? Hacking non FQN resolutions to inject another
   escaper ...

I would not call it 'hacking') This is exaclty the same as if we write
'PHPEscaper::escape()' manually in PHP context.
I didn't want to add many related items to a global namespace, and with
class it is possible to use autoloading.

2. Is there any rationale why we're using strings separated by '|' to pass

context instead of an array? Ex.:
<?* $str, ['js', 'html'] ?>

Yes, I thought about array. It can be added on a par with string. Strings
just looks more similar to escaping in template engines.

3. I think the default implementation should throw a more specific

exception in case of unknown context (\PHP\EscapeException?)

4. This line on your patch is unnecessary

You are right, thanks. This commit is a concept, if RFC will be accepted, I
will prepare a patch with more correct code.
​

If some library really need to replace existing handler of application, it can call unregisterHandler() directly.

But then there is no way to restore the previous handler.

usually we don't need multiple handlers for certain context.

Stuff that is added to the core needs to cover all use-cases, not just
the typical ones.

I tried not to invent new mechanisms of usage.

Then please copy the set_error_handler behaviour.

cheers
Dan

Multiple arguments would make the syntax even cleaner:
<?* $str, 'js', 'html' ?>

I thought about it. Multiple arguments do not allow runtime modification
(and make the parser more complex).
Something like this:
<?php
$context = [];
if ($field->name == 'url') $context[] = 'url';
$context[] = 'html';
?>

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>&lt;?= 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.vostrikov@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 *>&lt;?         // do you mean a call of escaper here?
$html = `ob_get_clean()`;
var_dump($html);

// string(11) "<* $text *>"

// --------

`ob_start()`;
?>&lt;?* $text ?>&lt;?
$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.

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' ?>

In Twig escapers and filters are also written after a variable, and this is
not confusing for many users.
{{ render(thing.viewName, 'html') | escape('js') | somefilter }}

Sure, but in Twig, Smarty, etc filters aren't just a special syntax for
doing escaping, they're a fundamental part of the language. In essence,
they're just a different way of writing a function call, a bit like
Sara's pipe operator, which incidentally would allow this:

<?= render($thing->viewName, 'html') |> escape($$, 'js') |>
somefilter($$) ?>

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.

Just a default variable, nothing complicated.
If they write echo like this, they will notice 'oopshtml' and then correct
this contstruction.
Binary operators have 2 arguments (add($a, $b), escape($string, $context)),
'for' has 3 arguments.

Binary operators have a left-hand operand and a right-hand operand;
unary operators have an operand immediately before or immediately after
their symbol. You don't write +$a, $b you write $a + $b. The only
exception is the ternary decision operator, $a ? $b : $c, which still
doesn't use a comma.

Functions, on the other hand, have a name, and a pair of brackets. So
+($a, $b) looks a bit like a function call with a funky name; that would
give us <?*( $foo, 'html') ?>.

I can see your logic in defining it this way, it just doesn't look like
anything else in the language, and that's a bad thing for people using
it properly.

And this is the reason why I want it to be a call of function with constant
name. This is very clear - <?* $str, $context ?> turns into
some_escape_function($str, $context).

The more you compare it to a function call, the less I understand how it
gains over just defining a function e() and writing <?= e( $foo, 'html') ?>

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' ?>

Exception: Unknown context 'hmtl'.
Exception: Unknown context ''.
Exception: Unknown context 'html, js'.

If the handlers for these contexts are not set, of course.

But if these are run-time errors, how is the clever syntax helping
people get it right here? If you can pass a variable as the escaping
method, define arbitrary escaping functions, etc, you can't even write a
strict static analyser.

if you're mentioning the whole function name, you can just call it already
<?*html= $foo ?>
Do you mean the function autoloading? What is the difference with not-fq
name 'PHPEscaper' then?) And how to use an escaper like [$this, 'html'] ?

You've merged two unrelated lines from my e-mail here, so I'm not sure
what your question is. By the first line, I meant that if you write
"<?[$this, 'html'] $foo ?>" you might as well just write "<?=
$this->html($foo) ?>". The syntax has gained you nothing but a minimum
version of PHP and some head-scratching from new users.

JS escape only; not sure if this should encode as JSON, or just a JS safe
string; maybe <?*json= $foo ?> as well / instead...
This looks unclear for me - why I cannot use json for strings and what if
my variable sometimes is an array, sometimes a string?

'foo' is valid JSON - there is much hot air about the difference between
a "JSON value", a "JSON document", etc, but long story short, it is
perfectly fine to say <?= json_encode('hello world') ?>

On the other hand, if I have an array and ask for it to be HTML-escaped,
nothing iterates the array for me, it will just print "Array". So if I
ask for it to be "JS-escaped", why should it magically produce a JSON
array? Not to mention the fact that PHP "arrays" cannot be losslessly
represented as either an object or an array in JS (they have both
arbitrary keys and well-defined order, JS makes you choose one or the
other).

The biggest use case for this is people who aren't using a framework
... so customising the definitions is going to be the exception, not the
rule
They can setup their escapers once, this is not a problem, but the problem
is e.g. default flags for html escaping.
Customization is required.

Customizability is required, yes, but it absolutely should not be
mandatory. If people are using a framework already, they will already
have a method of doing this. This feature is really only useful for
people who are relying on absolutely minimum framework code, who want
something to work "out of the box".

If I have to write "register_escape_handler(function($string,
$mode='html') { .... })" I might as well just write "function e($string,
$mode='html') { .... }". There's not even a question of autoloading,
because nothing is going to autoload the procedural code that runs
"register_escape_handler" anyway.

If we make it too flexible, we're basically inventing a new templating
language
We cannot forbid a customization, so any custom escaper is a kind of new
templating language.
The operator must be simple for use. If someone wants to create new
templating language in his application, let he create. It will be in
application, not in PHP.

I think we're in agreement here - simplicity is key. :)

The trick with the magic class name and namespace aliasing is neat, but
feels likely to confuse a lot of users

Yes, I have to agree. Maybe more better way is to make it similar to
set_error_handler() - not for context as it is in RFC, but for 'escape'
callable.

Personally, I think having a separate handler registered for each
context argument, like streams, makes more sense. I don't see the use
case for customising how contexts are combined, passing variable
contexts around, or anything else that is gained by one callback with
two parameters.

Again, focusing on simplicity:

<?php
set_escape_handler('html', 'htmlspecialchars');
set_escape_handler('json', 'json_encode');
?>
<?htmljson= $foo ?>

becomes:

<?php echo htmlspecialchars(json_encode($foo)); ?>

And, as I say, the common escape handlers should be defined by default,
just like a whole bunch of stream types are.

Regards,

Rowan Collins
[IMSoP]

On Fri, Jul 22, 2016 at 1:31 AM, Michael Vostrikov <
michael.vostrikov@gmail.com> wrote:

The trick with the magic class name and namespace aliasing is neat, but
feels likely to confuse a lot of users

Yes, I have to agree. Maybe more better way is to make it similar to
set_error_handler() - not for context as it is in RFC, but for 'escape'
callable.

<?php

// somewhere in application

set_escape_handler(function($string, $context = 'html'){
    ...
});

// or

set_escape_handler([$this, 'escape']);

?>

<?* $myValue, $myContext ?>

Which begs the question, if you can verify that the call to
set_escape_handler comes before the template, then can you also be sure
that a function definition will come before it, and just call a function
directly?

This is a really good point, Marco.

Of course, this would be much cleaner with a set of functions, since $this
(whatever it is) is not truly a dependency for any of these functions -
they're likely sharing no context or state; they've likely been placed in
the class solely to make them available to the template, which is a
work-around creating a class dependency for no real reason.

Now, if we could autoload functions...... :-)

The syntax is weird as heck.

That said, frameworks without templating engine already have escaping
helpers, for example:
<?= $this->escapeHtml($value); ?>
<?= $this->escapeHtmlAttr($value); ?>
<?= $this->escapeJs($value); ?>
<?= $this->escapeCss($value); ?>

I don't see what is hard in using that syntax, plus it's not a global
registry.

Marco Pivetta

http://twitter.com/Ocramius

http://ocramius.github.com/

On Wed, Jul 20, 2016 at 8:17 PM, Michael Vostrikov <
michael.vostrikov@gmail.com> wrote:

Personally I don't know any developer who is using raw php in project
without template engine

Zend, Yii, various CMS like Wordperss, internal business-applications -
in
many cases such projects don't have a template engine.
I usually work with Yii and internal applications on custom engines. This
is the reason why I raised this question.
By the way, the syntax is not weird. It is just <?* $var1, $var2 ?>. How
to
use $var2 is fully up to application.

<?= $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.
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?

the problem IS NOT that we don't have a solution....
The problem IS that developer
must call these functions everywhere manually.

What you don't seem to get, is your proposal doesn't change that fact?

It changes the syntax and means by which you select and call the function,
but it still requires the same choice and the same due diligence - and
thereby doesn't truly change anything.

This new tag will not simply replace <?= $var ?> because you still need to
output HTML sometimes.

What you've coined "context" is really just a pseudo function-call - it
does not automatically establish context, and if I have to specify the
context, I'm really just specifying the function I'd like to call;
specifying the right "context" requires the exact same choice and diligence
as selecting the right function, all this does is wrap things in another
layer of complexity and blur things out even more. At least, with a
function call, I can tell which function is going to be called - rather
than digging through an extra facility that takes, essentially, a function
name, and calls it for me.

In my view, this only complicates things - it somewhat changes the problem,
but doesn't actually solve the problem...

On Sat, Jul 30, 2016 at 8:06 AM, Michael Vostrikov <
michael.vostrikov@gmail.com> wrote:

The aim in my mind would be to make escaping easier to do right, for
people who aren't already using a framework or templating engine with its
own solution.
The current implementation doesn't seem to share these priorities; it
feels like a building block for framework developers, who probably have
their own solutions already.

No! You don't understand what I'm trying to explain. This feature will be
useful for ALL applications without template engine - frameworks, CMS,
custom core. You say that frameworks have their own solutions already. But
the problem IS NOT that we don't have a solution. PHP already has built-in
function htmlspecialchars(), Yii has Html::escape(), Zend has
$this->escape(). This is not the problem. The problem IS that developer
must call these functions everywhere manually.

<div> <div data-user-id="&lt;?= Html::escape($profile->user->id) ?>"> &lt;?= Html::escape($profile->name) ?> </div> <div> &lt;?= Html::escape($profile->description) ?> </div> <div> &lt;?= Html::escape($profile->age) ?> </div> <div> &lt;?= Html::escape($profile->position->name) ?> </div> <div> &lt;?= Html::escape($profile->group->name) ?> </div> <div> &lt;?= Html::escape($profile->organisation->address) ?> </div> </div>

This is the same as calling constructor manually after every 'new'
statement: (new User)->__construct(...), (new Profile)->__construct(...).
We have special function '__construct', which is called automatically. I
suggest similar way for escaping. And because we cannot set the same magic
function name for all applications I suggest to do this by registering a
callable.
This is not intended for people who don't have escaping functions, this is
intended for people who already have escaping functions. RFC suggests the
way how to call these functions automatically. And people who don't have
escaping function may define it once, and it also will be called
automatically.

• you can define automatic escaping for a whole file or a block within a
  file
• there is an extra filter to skip the automatic escaping (not the same
  as unescaping)
• the above can be done with any "context", but the default is HTML
• a "context" is not just the argument to a single all-powerful "escape"
  function; you can register a new context by name, > without reimplementing
  any of the existing functionality
• other template functions can say that their output shouldn't be
  escaped, or that their input should be pre-escaped
• other functionality of the system is aware of these notions, and
  designed to behave sensibly

I don't think there's any way PHP can ever reach that level of
sophistication, because most of the language knows nothing about "context";
the feature we build in is only ever going to be a simple short-hand for
some basic function calls.

Almost all of these points can be done with the system described in the
RFC. Application can allow or restrict new contexts (without reimplementing
any of the existing functionality). Any behavior can be defined in
userland, we just need a point of extension.

<?php
class CsvTemplate
{
protected $escapers = [];

function render()
{
    set_escape_handler([$this, 'escape']);
    include $this->templateFile;
    restore_escape_handler();
}

function escape($str, $context = 'csv')
{
    switch ($context) {
        case 'csv':
            return $this->escapeCsv($str);
        break;

        case 'raw':
            return $str;
        break;

        default:
            if (isset($this->escapers[$context])) {
                return call_user_func($this->escapers[$context], $str);
            }

            throw new Exception('Unknown context');
        break;
    }
}

function setEscaper($context, $callable)
{
    ...
}

}
?>

header1;header2;
<?* $data['field1'] ?>;<?* (int)$data['field2'], 'raw' ?>;

But secondly, because people are lazy, or misunderstand, or make mistakes
when they're in a hurry. Your RFC isn't going to magically fix all those
things.

It exactly is going to fix all those things. If people will use this
operator with HTML escaping by default, standard <?= ?> operator will not
be needed.

Just a thought, but I can't help thinking that "improved escape
facilities and syntax" are a mere patch for a more than superficial
problem.
The problem of differentiating HTML strings, which to not require
escaping, from other string, which do, could actually be viewed as a deeper
problem, which is the inability to tell string types apart.
/** @var string product description as HTML, do not escape this */
public $description;

This is another problem, and it depends on your infrastructure or business
logic. Only your application or model knows what is stored in
'$description'. What if I have a site for learning HTML and my
'$description' contains an example with HTML, which should be HTML-encoded
on the page with a lesson, so that it will look like source code? This is
not a task of escaping system.

And a more intelligent escape function, like:

typedef HTML extends string;

class ProductView
{
/** @var HTML product description */
public $description;
}

function html($str) {
return $str instanceof HTML ? $str : htmlspecialchars($str);
}

You can do this now by defining class 'HTML' with '__toString' method,
without any typedefs. What is the problem? And anyway you must call your
'html()' function everywhere, and this is the problem.

On Sat, Jul 30, 2016 at 8:06 AM, Michael Vostrikov <
michael.vostrikov@gmail.com> wrote:

No! You don't understand what I'm trying to explain.

We understand, that's why we're discussing so much against it.

This feature will be
useful for ALL applications without template engine - frameworks, CMS,
custom core.

Not really. What you propose is Foo::escape() (static), as a language
construct.
This is a problem, as it makes the escaping statically bound to a
configured endpoint, and that endpoint may even change (what the heck?!)
Templating engines can instead switch the escaping per-template-file, which
is much more powerful, as the assumption of context may change.

This is the same as calling constructor manually after every 'new'

statement: (new User)->__construct(...), (new Profile)->__construct(...).

If any templating engine does that, I'd suggest opening an issue on their
issue tracker to make their implementation non-static instead.

Anyway, I saw that voting is open, and already voted "No" on it for the
reasons mentioned above, and because I don't believe in adding more custom
AST for something that is already working very well in userland via
functions.

Users that don't escape their output usually do so because they lived under
a rock, and they'll continue to do so regardless.
Users that know how and when to escape are already using appropriate
functions for that.

The proposed solution is a solution, but not for this problem, in my
opinion.

Greets,

Marco Pivetta

http://twitter.com/Ocramius

http://ocramius.github.com/