-
Notifications
You must be signed in to change notification settings - Fork 13
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
Update content addressed github uri scheme #129
Conversation
86d8a40
to
4126a33
Compare
ethpm/backends/http.py
Outdated
"Expected contents returned from Github to be base64 encoded, " | ||
f"instead received {contents['encoding']}." | ||
) | ||
return base64.b64decode(contents["content"]) |
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.
Maybe I missed it but I expected to see somewhere that we verify that the contents match the hash. If that isn't present I think it needs to be added.
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.
Yup, I chose to remove that for some reason that escapes me now, I'll set about re-implementing that.
ethpm/utils/uri.py
Outdated
if not all((path, scheme, authority)): | ||
return False | ||
|
||
if [term for term in expected_path_terms if term not in path]: |
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.
Generally the pattern for this is to use any
if any(term for term in expected_path_terms if not term in path):
...
The nice part about this is that it is lazy meaning it will exit on the first one that is truthy.
b287c43
to
7c911ed
Compare
4ea7c34
to
7655cb4
Compare
7655cb4
to
4a90b18
Compare
@pipermerriam ping - added a validation util to verify that the hash of the contents retrieved from a github blob uri match the hash in the uri |
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.
Badass and good investigative work on how the sha1
hash is calculated.
What was wrong?
Decision was made to sunset the fragment based hash verification for github uris and use their native blob system instead - Also mirrors what is suggested in the ERC 1319, so this will go towards having a cleaner standard amongst libraries.
How was it fixed?
Updated the utils and
GithubOverHTTPS
backend to use the blob scheme.Cute Animal Picture
fixes #125