Skip to content

Conversation

@santigimeno
Copy link
Member

lineNumber, columnNumber and url node fields can be actually undefined. Make sure we avoid integer overflows and potential crashes.

As an example, if lineNumber or columnNumber are undefined, they were taking the 4294967295 which obviously doesn't make any sense.

`lineNumber`, `columnNumber` and `url` node fields can be actually
undefined. Make sure we avoid integer overflows and potential crashes.

As an example, if `lineNumber` or `columnNumber` are undefined, they
were taking the `4294967295` which obviously doesn't make any sense.
@santigimeno santigimeno requested a review from juanarbol December 9, 2024 15:29
@santigimeno santigimeno self-assigned this Dec 9, 2024
Copy link
Contributor

@juanarbol juanarbol left a comment

Choose a reason for hiding this comment

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

sgtm, could you please open the backports for 20.x and 18.x?

writer_->AddNumber(node->GetLineNumber() - 1);
writer_->AddString(",\"columnNumber\":");
writer_->AddNumber(node->GetColumnNumber() - 1);
if (node->GetLineNumber()) {
Copy link
Member

Choose a reason for hiding this comment

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

Have you checked what GetLineNumber() returns in case of REPL? (the same applies to GetColumnNumber)

Copy link
Member Author

Choose a reason for hiding this comment

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

This is an example of what it prints:

"callFrame":{"functionName":"","lineNumber":0,"columnNumber":12,"scriptId":162,"url":"REPL6"}

santigimeno added a commit that referenced this pull request Dec 16, 2024
`lineNumber`, `columnNumber` and `url` node fields can be actually
undefined. Make sure we avoid integer overflows and potential crashes.

As an example, if `lineNumber` or `columnNumber` are undefined, they
were taking the `4294967295` which obviously doesn't make any sense.

PR-URL: #233
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
@santigimeno
Copy link
Member Author

Landed in a4dfbf6

@santigimeno santigimeno deleted the santi/fix_cpu_profiling_serializing branch December 16, 2024 14:44
santigimeno added a commit that referenced this pull request Jan 3, 2025
`lineNumber`, `columnNumber` and `url` node fields can be actually
undefined. Make sure we avoid integer overflows and potential crashes.

As an example, if `lineNumber` or `columnNumber` are undefined, they
were taking the `4294967295` which obviously doesn't make any sense.

PR-URL: #233
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
santigimeno added a commit that referenced this pull request Jan 9, 2025
`lineNumber`, `columnNumber` and `url` node fields can be actually
undefined. Make sure we avoid integer overflows and potential crashes.

As an example, if `lineNumber` or `columnNumber` are undefined, they
were taking the `4294967295` which obviously doesn't make any sense.

PR-URL: #233
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
santigimeno added a commit that referenced this pull request Jan 13, 2025
`lineNumber`, `columnNumber` and `url` node fields can be actually
undefined. Make sure we avoid integer overflows and potential crashes.

As an example, if `lineNumber` or `columnNumber` are undefined, they
were taking the `4294967295` which obviously doesn't make any sense.

PR-URL: #233
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
PR-URL: #245
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
santigimeno added a commit that referenced this pull request Jan 13, 2025
`lineNumber`, `columnNumber` and `url` node fields can be actually
undefined. Make sure we avoid integer overflows and potential crashes.

As an example, if `lineNumber` or `columnNumber` are undefined, they
were taking the `4294967295` which obviously doesn't make any sense.

PR-URL: #233
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
PR-URL: #246
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants