fix: only allow valid identifiers to be nonreentrant keys #3605
Conversation
disallow invalid identifiers like `" "`, `"123abc"` from being keys for non-reentrant locks. this commit also refactors the `validate_identifiers` helper function to be in the `ast/` subdirectory, and slightly improves the VyperException constructor by allowing None (optional) annotations.
|
|
||
|
|
||
| def validate_identifier(attr, ast_node=None): | ||
| if not re.match("^[_a-zA-Z][a-zA-Z0-9_]*$", attr): |
There was a problem hiding this comment.
So the rule is "first letter should be alphabet or _ and remaining letter may be alphanumerical". Can we please add this to the docs here what qualifies as a valid <key>.
There was a problem hiding this comment.
it's the same as for all identifiers though. we could say "keys must be valid identifiers" but i think that might actually be more confusing than not.
There was a problem hiding this comment.
But how do people know what valid actually means?
There was a problem hiding this comment.
well i just checked, and the identifier rules are not documented. they are basically a simplified version of the python2 identifier rules tho.
There was a problem hiding this comment.
undocumented behaviour is just not great. We need to increase devex quality for Vyper :)
Codecov Report
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. @@ Coverage Diff @@
## master #3605 +/- ##
==========================================
+ Coverage 89.09% 89.13% +0.03%
==========================================
Files 85 86 +1
Lines 11392 11397 +5
Branches 2591 2592 +1
==========================================
+ Hits 10150 10159 +9
+ Misses 820 816 -4
Partials 422 422
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
disallow invalid identifiers like
" ","123abc"from being keys for non-reentrant locks.this commit also refactors the
validate_identifiershelper function to be in theast/subdirectory, and slightly improves the VyperException constructor by allowing None (optional) annotations.What I did
fixes #3382, disallows the conditions for GHSA-3hg2-r75x-g69m
How I did it
How to verify it
Commit message
Commit message for the final, squashed PR. (Optional, but reviewers will appreciate it! Please see our commit message style guide for what we would ideally like to see in a commit message.)
Description for the changelog
Cute Animal Picture