Fix validate_object crashing when object prototype keys used in style's objects
#1028
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes an unhandled error occurring when validated objects (e.g. sources) had keys that were found in Object.prototype. This caused certain truthyness checks to incorrectly pass, as e.g. object["__proto__"] always returns a truthy value (unless object has a null prototype or otherwise non-Object prototype). This commit fixes the problem by checking that the value is indeed set on the object via Object.hasOwn(), or Object.prototype.hasOwnProperty.call() if hasOwn is not available.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1028 +/- ##
=======================================
Coverage 92.73% 92.74%
=======================================
Files 107 108 +1
Lines 4722 4728 +6
Branches 1344 1347 +3
=======================================
+ Hits 4379 4385 +6
Misses 343 343 ☔ View full report in Codecov by Sentry. |
HarelM
reviewed
Mar 1, 2025
Collaborator
|
Can you check why the coverage is complaining? |
Contributor
Author
This was because the environment in which the tests ran do support |
HarelM
reviewed
Mar 2, 2025
hasOwn can now be used to replace patterns like:
```
if (hasOwnProperty(obj, key) && obj[key]) {}
```
with
```
if (hasOwn(obj, key)) {}
```
whereas getOwn can be used to replace patterns like
```
const value = (hasOwnProperty(obj, key) && obj[key]) || fallback
```
with
```
const value = getOwn(obj, key) || fallback
```
cxcorp
force-pushed
the
fix-validate-object-crashing-for-prototype-keys
branch
from
8694268 to
2dc7b02
Compare
HarelM
reviewed
Mar 3, 2025
HarelM
approved these changes
Mar 4, 2025
Collaborator
|
THANKS! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
From #1025
Fixes an unhandled error occurring when validated objects (e.g. sources) had keys that were found in
Object.prototype. This caused certain truthyness checks to incorrectly pass, as e.g.object["__proto__"]always returns a truthy value (unless object has a null prototype or otherwise non-Objectprototype).This caused unhandled errors such as:
for sources like:
respectively. Since the issue was in
validate_object, which is widely used in the codebase, there are most likely many other inputs that would cause this error, e.g. via layer objects or any other object in the style.This PR fixes the problem by checking that the value is indeed set on the object via
Object.hasOwn(), orObject.prototype.hasOwnProperty.call()ifhasOwnis not available. I was unsure of the browser targets, so sinceObject.hasOwnwas added in ES2022, I thought it would be better to make a separate function for more ergonomic use which falls back to a more compatible implementation.The unit test for
validate_object.test.tsdoes not test for__proto__as a key, as jsonlint currently mutates the resulting objects' prototypes due to an assignment instead ofObject.defineProperty(), and the resulting objects have the wrong type when checked viainstanceof(and thus ingetType()). I will open a separate issue about that.Launch Checklist
CHANGELOG.mdunder the## mainsection.