python / cpython

The Python programming language
https://www.python.org
Other
63.29k stars 30.3k forks source link

`ast.literal_eval` is still referred to as safe by the documentation for `eval` #100305

Open Zephyrkul opened 1 year ago

Zephyrkul commented 1 year ago

Documentation

From https://docs.python.org/3/library/functions.html#eval:

See ast.literal_eval() for a function that can safely evaluate strings with expressions containing only literals.

Linked PRs

ramvikrams commented 1 year ago

Could you please elabourate about it, I would like to work on this but I can't get what the issue is trying to say.

pochmann commented 1 year ago

@ramvikrams Have you read literal_eval's documentation?

ramvikrams commented 1 year ago

@ramvikrams Have you read literal_eval's documentation?

yes, I read it now earlier I read ast.literal_eval from a different source I saw there was a warning there I'll put the warning in the eval documentation

Zephyrkul commented 1 year ago

Could you please elabourate about it, I would like to work on this but I can't get what the issue is trying to say.

From the docs for literal_eval:

This function had been documented as “safe” in the past without defining what that meant. That was misleading.... [It] is not free from attack.

Python 3.11 changed the documentation to no longer refer to the function as "safe" due to the misleading vagueness of the term (see #95588), but this entry under eval() seems to have been missed.

ramvikrams commented 1 year ago

Could you please elabourate about it, I would like to work on this but I can't get what the issue is trying to say.

From the docs for literal_eval:

This function had been documented as “safe” in the past without defining what that meant. That was misleading.... [It] is not free from attack.

Python 3.11 changed the documentation to no longer refer to the function as "safe" due to the misleading vagueness of the term (see #95588), but this entry under eval() seems to have been missed.

thanks i'll change it

pochmann commented 1 year ago

Your proposed replacement text looks weirdly out of place and negative to me:

ast.literal_eval is no longer marked as safe for evaluating strings with expressions containing only literals. For further information, please check the documentation for ast.literal_eval

I'd wonder why it's suddenly talking about some other function as if the reader knew about it already, as if it had been pointed out before.

I'd just weaken the "safely", like (adding "more")

See ast.literal_eval() for a function that can more safely evaluate strings with expressions containing only literals.

or (adding quote characters):

See ast.literal_eval() for a function that can "safely" evaluate strings with expressions containing only literals.

These keep introducing it properly and keep the positive tone.

And in my opinion there's no need for the extra sentence telling the reader to read the documentation for details, that's what one does anyway.

ramvikrams commented 1 year ago

Yeah I'll do that