lark-parser / lark

Lark is a parsing toolkit for Python, built with a focus on ergonomics, performance and modularity.
MIT License
4.88k stars 414 forks source link

refactor: add 'usedforsecurity=False' arg to hashlib.md5 usage #1190

Closed cquick01 closed 2 years ago

cquick01 commented 2 years ago

This argument was introduced in Python 3.9, but using hashlib.new() prevents <3.9 versions from throwing an exception.

Tested running locally against 3.7, 3.9, and 3.10.

Closes #1187

erezsh commented 2 years ago

I didn't realize we call md5 from 3 different places. Would you mind refactoring it into a single function, like load_grammar.md5_digest(), and call that instead?

erezsh commented 2 years ago

Thanks! Also, mypy fails with

lark/load_grammar.py:1419: error: Unexpected keyword argument "usedforsecurity" for "new"  

Can you check if there's a way to fix this? If not, adding an "ignore" comment is good enough for me.

cquick01 commented 2 years ago

Latest commit adds an explicit check for Python 3.9+ to pass the usedforsecurity argument. It's slightly messier now so if you'd prefer just changing to # type: ignore we can change back to that.

Latest commit:

if sys.version_info >= (3, 9):
    return hashlib.md5(s.encode('utf8'), usedforsecurity=False).hexdigest()
else:
    return hashlib.md5(s.encode('utf8')).hexdigest()

Or slightly cleaner version with less repeated code

hashlib.new("md5", s.encode("utf8"), usedforsecurity=False).hexdigest()  # type: ignore
erezsh commented 2 years ago

Looks good to me. I'll wait a bit to see if @MegaIng has anything to add.

erezsh commented 2 years ago

@cquick01 Thank you for your contribution!

jjtroberts commented 2 years ago

Thank you all for working on this fix. When might the next release be cut?

erezsh commented 2 years ago

It's probably about time for a new release! I'll try to do it early next week.

erezsh commented 2 years ago

@jjtroberts Released 1.1.3