Skip to content

Conversation

@suwc
Copy link

@suwc suwc commented Feb 12, 2016

Bugfix:OS5553123:Freeze while comparing valueOf()-injected null

Chakra implementation of Abstract Relational Comparison operation fails to
handle 'null'. Specifically, it cause a browser tab to freeze when a comparison ("<")
is done with the left value being 'null' injected by valueOf() call through a prototype.
Add 'null' checking to fix this problem.
Add unit test.

@suwc
Copy link
Author

suwc commented Feb 12, 2016

@akroshg @tcare @abchatra could you take a look pls?

dblRight = JavascriptConversion::ToNumber(aRight, scriptContext);
break;
case TypeIds_Boolean:
case TypeIds_Null:
Copy link
Contributor

Choose a reason for hiding this comment

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

should handle the undefined and symbol cases here.

Copy link
Author

Choose a reason for hiding this comment

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

Good point. Updated in latest rev.

@suwc suwc self-assigned this Feb 12, 2016
@akroshg
Copy link
Contributor

akroshg commented Feb 12, 2016

Looks good.

@tcare
Copy link
Contributor

tcare commented Feb 12, 2016

LGTM, signing off.

@tcare
Copy link
Contributor

tcare commented Feb 12, 2016

Actually, can we get tests for the undefined/symbol case? otherwise looks good.

}

testRelationalComparison(null);
testRelationalComparison(undefined);
Copy link
Author

Choose a reason for hiding this comment

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

@tcare, unit test covers 'undefined' and Symbol here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah saw the wrong revision. thanks :)

Chakra implementation of Abstract Relational Comparison operation fails to
handle 'null'. Specifically, it cause a browser tab to freeze when a comparison ("<")
is done with the left value being 'null' injected by valueOf() call through a prototype.
Add 'null' checking to fix this problem.
Add unit test.
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.

5 participants