-
-
Notifications
You must be signed in to change notification settings - Fork 30.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bpo-40355: ast.literal_eval rejects malformed Dict nodes #19868
Changes from 2 commits
36eb5b8
2c72460
3e68c2e
2427ed2
bc9274f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -62,11 +62,13 @@ def literal_eval(node_or_string): | |
node_or_string = parse(node_or_string, mode='eval') | ||
if isinstance(node_or_string, Expression): | ||
node_or_string = node_or_string.body | ||
def _malformed(node): | ||
raise ValueError(f'malformed node or string: {node!r}') | ||
def _convert_num(node): | ||
if isinstance(node, Constant): | ||
if type(node.value) in (int, float, complex): | ||
return node.value | ||
raise ValueError('malformed node or string: ' + repr(node)) | ||
_malformed(node) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: I suppose at this point we can invert the check to make the intent a bit more clear:
|
||
def _convert_signed_num(node): | ||
if isinstance(node, UnaryOp) and isinstance(node.op, (UAdd, USub)): | ||
operand = _convert_num(node.operand) | ||
|
@@ -88,6 +90,8 @@ def _convert(node): | |
node.func.id == 'set' and node.args == node.keywords == []): | ||
return set() | ||
elif isinstance(node, Dict): | ||
if len(node.keys) != len(node.values): | ||
_malformed(node) | ||
return dict(zip(map(_convert, node.keys), | ||
map(_convert, node.values))) | ||
elif isinstance(node, BinOp) and isinstance(node.op, (Add, Sub)): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -965,6 +965,12 @@ def test_literal_eval_complex(self): | |
self.assertRaises(ValueError, ast.literal_eval, '3+(0+6j)') | ||
self.assertRaises(ValueError, ast.literal_eval, '-(3+6j)') | ||
|
||
def test_literal_eval_malformed(self): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I find this name is too generic. Maybe |
||
malformed = ast.Dict(keys=[ast.Constant(1), ast.Constant(2)], values=[ast.Constant(3)]) | ||
self.assertRaises(ValueError, ast.literal_eval, malformed) | ||
malformed = ast.Dict(keys=[ast.Constant(1)], values=[ast.Constant(2), ast.Constant(3)]) | ||
self.assertRaises(ValueError, ast.literal_eval, malformed) | ||
|
||
def test_bad_integer(self): | ||
# issue13436: Bad error message with invalid numeric values | ||
body = [ast.ImportFrom(module='time', | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
Fixed case where malformed :class:`ast.Dict` nodes could have keys or values | ||
thrown away by :func:`ast.literal_eval`. Patch by Curtis Bucher. | ||
curtisbucher marked this conversation as resolved.
Show resolved
Hide resolved
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, call this function with a verb, not a noun. Finding
malformed
in the middle of the code is a bit surprising compared withreport_malformed_node
orraise_malformed_exception
or something similar.