-
-
Notifications
You must be signed in to change notification settings - Fork 85
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
Validate binary hash #336
base: development
Are you sure you want to change the base?
Validate binary hash #336
Conversation
… return it to ForgeBox
@@ -539,6 +543,11 @@ component accessors="true" implements="IEndpointInteractive" { | |||
throw( 'No download URL provided in #getNamePrefixes()#. Manual install only.', 'endpointException' ); | |||
} | |||
|
|||
// Validate the binary hash |
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.
@bdw429s does this look good to you?
src/cfml/system/endpoints/HTTP.cfc
Outdated
@@ -81,7 +81,7 @@ component accessors=true implements="IEndpoint" singleton { | |||
|
|||
// Validate the binary hash | |||
if( len( binaryHash ) && binaryHash != hash( fileReadBinary( fullPath ), "MD5" ) ) { | |||
throw( 'The binary hash of the downloaded file does not match the expected hash.', 'endpointException' ); | |||
throw( 'The hash of the downloaded file [#hash( fileReadBinary( fullPath ))#] doesn''t match the excepted hash [#binaryHash#] #fullPath#', 'endpointException' ); |
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 put the hash in a variable instead of calculating it again. For a large file, this may be a non-trivial operation.
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.
done
@@ -339,7 +340,8 @@ or just add DEBUG to the root logger | |||
installInstructionsFormat = arguments.installInstructionsFormat, | |||
changeLog = arguments.changeLog, | |||
changeLogFormat = arguments.changeLogFormat, | |||
forceUpload = arguments.forceUpload | |||
forceUpload = arguments.forceUpload, | |||
binaryHash = arguments.binaryHash |
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.
I assume you've also updated ForgeBox to receive and store this?
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.
that's correct
Can we merge this PR? @bdw429s |
No description provided.