Interpolation inside comment results in error

[char101]

Hello,

I encountered an issue with the following code:

//
  p.
    go to #[a(href='') page]
p.
  now

I expected to get:

<!--p.
  go to <a href="">page</a>-->
<p>now</p>

But I actually get:

Phug\LexerException: Failed to lex: Unexpected Phug\Lexer\Token\TagInterpolationStartToken inside raw text.
Near: p.
        now

Line: 4
Offset: 1
Path: comment.pug in /vendor/phug/phug/src/Phug/Lexer/Lexer/State.php on line 457

Call Stack:
    0.0001     396936   1. {main}() /main.php:0
    0.0163    3302128   2. Pug\Pug->renderFile() /main.php:29
    0.0163    3302824   3. call_user_func:{/vendor/pug-php/pug/src/Pug/Engine/Renderer.php:116}() /vendor/pug-php/pug/src/Pug/Engine/Renderer.php:116
    0.0163    3302824   4. Pug\Pug->Pug\Engine\{closure:/vendor/pug-php/pug/src/Pug/Engine/Renderer.php:108-110}() /vendor/pug-php/pug/src/Pug/Engine/Renderer.php:116
    0.0163    3302824   5. Pug\Pug->renderFileWithPhp() /vendor/pug-php/pug/src/Pug/Engine/Renderer.php:109
    0.0163    3302824   6. Pug\Pug->renderFile() /vendor/pug-php/pug/src/Pug/Engine/Renderer.php:35
    0.0163    3303144   7. Pug\Pug->callAdapter() /vendor/phug/phug/src/Phug/Renderer/Renderer.php:136
    0.0255    4185496   8. Pug\Pug->handleHtmlEvent() /vendor/phug/phug/src/Phug/Renderer/Renderer/Partial/AdapterTrait.php:238
    0.0255    4185496   9. Pug\Pug->Phug\Renderer\Partial\{closure:/vendor/phug/phug/src/Phug/Renderer/Renderer/Partial/AdapterTrait.php:234-236}() /vendor/phug/phug/src/Phug/Renderer/Renderer/Partial/AdapterTrait.php:176
    0.0255    4185496  10. Pug\Pug->Phug\{closure:/vendor/phug/phug/src/Phug/Renderer/Renderer.php:139-141}() /vendor/phug/phug/src/Phug/Renderer/Renderer/Partial/AdapterTrait.php:235
    0.0255    4185496  11. Pug\Pug->compileFile() /vendor/phug/phug/src/Phug/Renderer/Renderer.php:140
    0.0255    4185496  12. Phug\Compiler->compileFile() /vendor/phug/phug/src/Phug/Renderer/Renderer.php:96
    0.0256    4185640  13. Phug\Compiler->compile() /vendor/phug/phug/src/Phug/Compiler/Compiler.php:753
    0.0256    4185800  14. Phug\Compiler->compileDocument() /vendor/phug/phug/src/Phug/Compiler/Compiler.php:727
    0.0256    4185800  15. Phug\Compiler->compileIntoElement() /vendor/phug/phug/src/Phug/Compiler/Compiler.php:694
    0.0256    4185800  16. Phug\Parser->parse() /vendor/phug/phug/src/Phug/Compiler/Compiler.php:771
    0.0260    4204616  17. Phug\Parser\State->nextToken() /vendor/phug/phug/src/Phug/Parser/Parser.php:318
    0.0260    4204616  18. Generator->next() /vendor/phug/phug/src/Phug/Parser/Parser/State.php:455
    0.0260    4204616  19. Phug\Lexer->lex() /vendor/phug/phug/src/Phug/Parser/Parser/State.php:455
    0.0260    4204616  20. Phug\Lexer->handleTokens() /vendor/phug/phug/src/Phug/Lexer/Lexer.php:254
    0.0261    4203656  21. Phug\Lexer\State->loopScan() /vendor/phug/phug/src/Phug/Lexer/Lexer.php:326
    0.0261    4203656  22. Phug\Lexer\State->scan() /vendor/phug/phug/src/Phug/Lexer/Lexer/State.php:285
    0.0261    4203656  23. Phug\Lexer\Scanner\CommentScanner->scan() /vendor/phug/phug/src/Phug/Lexer/Lexer/State.php:246
    0.0274    4213216  24. Phug\Lexer\Analyzer\LineAnalyzer->getFlatLines() /vendor/phug/phug/src/Phug/Lexer/Lexer/Scanner/CommentScanner.php:45
    0.0274    4213536  25. array_map() /vendor/phug/phug/src/Phug/Lexer/Lexer/Analyzer/LineAnalyzer.php:179
    0.0274    4213912  26. Phug\Lexer\Analyzer\LineAnalyzer->Phug\Lexer\Analyzer\{closure:/vendor/phug/phug/src/Phug/Lexer/Lexer/Analyzer/LineAnalyzer.php:171-179}() /vendor/phug/phug/src/Phug/Lexer/Lexer/Analyzer/LineAnalyzer.php:179
    0.0274    4214008  27. Phug\Lexer\State->throwException() /vendor/phug/phug/src/Phug/Lexer/Lexer/Analyzer/LineAnalyzer.php:174

Thanks!

[kylekatarnls]

I see you give as expected output the pugjs result:

<!--p.
  go to <a href="">page</a>-->
<p>now</p>

But you have to recognize this is pretty unexpected to get "p." as raw text but to get the interpolation compiled.

This is why this exception was initially added: https://github.com/phug-php/phug/blob/5aeba63f26adbab47890e2e3c136b03955e341b1/src/Phug/Lexer/Lexer/Analyzer/LineAnalyzer.php#L174

I would be embarrassed to implement strictly the same result as in pugjs. Maybe I should discuss this with pugjs because it would make more sense to me to get:

<!--p.
    go to #[a(href='') page]-->
<p>now</p>

Or eventually:

<!--<p>go to <a href="">page</a></p>-->
<p>now</p>

[char101]

I didn't even notice the dot after p in the pugjs result 😮 . I only take it as an example showing that it works in pugjs.

It is indeed debatable whether pug code should be evaluated inside comment or not. Regardless I think it should not throw an error though.

From a security standpoint I think evaluating pug inside comment is better than showing it in the comment as is.

[char101]

From https://pugjs.org/language/comments.html

Buffered comments look the same as single-line JavaScript comments. They act sort of like markup tags, producing HTML comments in the rendered page.

Pug also supports unbuffered comments. Simply add a hyphen (-) to the start of the comment.

So I think the text inside comment should not be taken as raw text.

[kylekatarnls]

"security" ? What is more secure in compiled HTML than raw Pug inside an HTML comment?

But if it's told so in the doc, it means, there is a bug in pugjs. I will open an issue to them, so we could have a common result at the end.

[char101]

"security" ? What is more secure in compiled HTML than raw Pug inside an HTML comment?

It could contain PHP code thus exposing the internal of the application.

[kylekatarnls]

First, if you comment code, you should not let it in the HTML so use //- then, security should not be based on secret. It would be extremely fragile. Many social networks, bank applications etc. are open-source, so everyone can see the whole code, but it doesn't mean it's not secure, on the contrary.

I agree, as a general idea, to help preventing users from doing security mistakes, but honestly, I won't consider the case where a clear password or a security breach is revealed by the output I would assume it's not intentionally displayed just because it's a comment. I'm pretty sure, pugjs did not take this point into account to decide if it should evaluated or not.

Recommendation: always act as if your code were publicly accessible by anyone. This way, your company won't collapse if some colleague leaks the code. Separate password, tokens and DB access into separate env file you keep only in production environments. And make your development environment a safe sandbox that contains nothing related to the real config or data of production environment and where you can do anything without constraints.

[char101]

You speak of ideals. Let me speak by experience: (1) if it can happen it will happen (2) most developers don't understand security.

Also exposing the technology used by a non open source application opens a risk that an attacker might use a bug in that technology. That's why it is a common practice to hide the web server and interpreter from http response headers. It is not about security per se, but about risk reduction.

Anyway security is not my issue here. I simply want to comment out a part of the pug code. Whether it is using // or //- the Lexer still throws an exception.

[kylekatarnls]

(1) if it can happen it will happen

That's exactly why you should consider your code as a public resource.

(2) most developers don't understand security

In this consideration, there is pretty much nothing your template engine can help you with than the standard HTML escape in data output.

hide the web server and interpreter

Your templates should not contain that, templates should be back-end technologies agnostic. Data should already be formatted, so you don't have to call controller/service/data stuff directly from inside your template.

My templates look like this:

if user.hasAccessTo('something')
  p=user.name
  p=helper.getData('some param')

Back-end could be PHP, Node.js, or any other language with any framework. Handle all the calculation you can in services/helper/object methods than writing it down in the templates. It better separate the concerns and make your templates maintainable by developers that do not need to understand deeply the controllers/services behind.

about risk reduction

What I recommended above is exactly about risk management. And it's taking it by the wrong end than hoping keeping code/technology secret.

the Lexer still throws an exception

Exception is always the safest way to deal with unexpected cases and it will remain until what the output should really be can be determined according to discussion with pugjs team.

Until then, use escaping:

//
  p.
    go to \#[a(href='') page]

[kylekatarnls]

Forbes from pugjs took the point. I will align phug on his decision.

[char101]

Thanks.

Since // outputs a HTML comment, then the content inside it should be HTML too. Otherwise if we want to comment pug code, we should use //-.

So // should contains valid pug code, while //- could contains broken pug code.

[kylekatarnls]

I'm still not sure // should compile neither the content because:

//
  My comment

should rather not be <My>comment</My>

If // compile content, then it becomes very difficult to display a multiline simple text comment. While if it does not, you still can write HTML on purpose.

The last thing (that may explain why interpolation is handled currently in pugjs): if it's not compiled, there is no way to inject variable values.

That's tricky, so I'm glad it's up to Forbes to decide. ^^

But yes, surely //- should not throw any exception or do any kind of compilation of this content. This will be done, but I will wait to know if it should be done for //- specifically, or in a common way for both // and //-, it will avoid me to potentially undo and refactor what I would have done.

[char101]

Can't the dot modifier be made to work with // too?

//.
  multi line
  comment

[char101]

Looking at pugjs behavior apparently it does not process pug code inside // so having interpolation processed seems to be the inconsistency.

//
  p text

results in

<!--p text-->

[kylekatarnls]

Yes, indeed, it would be possible to have simple text comment thanks to the dot, so it's no longer a problem. In both cases, there is an alternative to get the other output.

[kylekatarnls]

I also tried:

- foo = 9

// Foo #{foo}
// Foo #[a(href='') page]

//
  p.
    go to #[a(href='') page]

in pugjs and only the last one produces a <a href=""> the other code show the raw Pug input: <!-- Foo #{foo}--> and: <!-- Foo #[a(href='') page]-->

While it sounds like interpolating a variable inside an output comment might be an interesting feature, it's not the path took in pugjs. And we can still get equivalent result with:

- foo = 9

| <!--
| Foo #{foo}
| -->

or with tags:

- foo = 9

| <!--
p.
  go to #[a(href='') page]
| -->

So I can already recommend to use this last syntax if you want to output comment with compiled tags or expressions inside.

And I will assert compiling comment is a bug in both pugjs and pug-php. I will allow any valid or invalid Pug code inside comment and it will be considered as raw text for both //- and //