Skip to content

Conversation

@juliusknorr
Copy link
Member

@juliusknorr juliusknorr commented Aug 5, 2021

This will make sure that nested objects or arrays do not cause exceeding the maximum nesting level of functions when parsing arguments of an exception trace. In addition the second commit ensures that the class of an encoded object argument is always listed first in the resulting json object.

Found when debugging issues in #25774 with @marcoambrosini where a faulty scss code caused an exception but the arguments passed in the SCSS library contain a deeply nested tree of the parsed scss.

Example patch to reproduce the exception happening:

diff --git a/core/css/css-variables.scss b/core/css/css-variables.scss
index 86f80611a6..4b17bff4c9 100644
--- a/core/css/css-variables.scss
+++ b/core/css/css-variables.scss
@@ -13,6 +13,7 @@
        --color-background-hover: #{$color-background-hover};
        --color-background-dark: #{$color-background-dark};
        --color-background-darker: #{$color-background-darker};
+       --color-background-darker: #{$color-background-darker-aaa};
 
        --color-placeholder-light: #{$color-placeholder-light};
        --color-placeholder-dark: #{$color-placeholder-dark};

Without this patch an empty page is shown and just a generic error with xdebug or even a core dump without:

  Error   PHP   Error: Xdebug has detected a possible infinite loop, and aborted   2021-08-05T09:13:49+00:00  
                your script with a stack depth of '256' frames at                                             
                /var/www/html/lib/private/Log/ExceptionSerializer.php#232

With this PR we get a proper error page and trace in the logs:

  • ScssPhp\ScssPhp\Exception\CompilerException: Undefined variable $color-background-darker-aaa: core/css/css-variables.scss on line 16, at column 2 Call Stack: #0 import core/css/css-variables.scss (unknown file) on line 1

@juliusknorr juliusknorr added bug 3. to review Waiting for reviews labels Aug 5, 2021
@juliusknorr juliusknorr requested review from a team, PVince81 and icewind1991 August 5, 2021 13:09
}

private function encodeArg($arg) {
private function encodeArg($arg, $nestingLevel = 5) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

5 is rather random, but that should be more than enough to get something meaningful out of the arguments. Also too large nesting here will make the log more unreadable.

Copy link
Member

@skjnldsv skjnldsv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch!

@skjnldsv
Copy link
Member

skjnldsv commented Aug 5, 2021

Lint fails

This will make sure that nested objects or arrays do not cause exceeding
the maximum nesting level of functions when parsing arguments of an
exception trace

Signed-off-by: Julius Härtl <[email protected]>
@juliusknorr juliusknorr force-pushed the bugfix/noid/avoid-stack-depth-exceed branch from dd8b239 to b235a85 Compare August 5, 2021 15:37
@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Aug 5, 2021
@skjnldsv skjnldsv merged commit d65a35e into master Aug 5, 2021
@skjnldsv skjnldsv deleted the bugfix/noid/avoid-stack-depth-exceed branch August 5, 2021 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4. to release Ready to be released and/or waiting for tests to finish bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants