Skip to content
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

Fix hash length #1504

Merged
merged 1 commit into from
Nov 18, 2019
Merged

Fix hash length #1504

merged 1 commit into from
Nov 18, 2019

Conversation

NicolasMahe
Copy link
Member

Check the length of hash in Marshal and Unmarshal and return always the actual length.

I had the issue where the Marshal function accepted a hash with the wrong length, and the unmarshal function was return an unrecognized EOF error because the length return by the hash was hardcoded...

@NicolasMahe NicolasMahe self-assigned this Nov 15, 2019
@NicolasMahe NicolasMahe added this to the next milestone Nov 15, 2019
@@ -19,6 +19,13 @@ var DefaultHash = sha256.New
// size is a default size for hashing algorithm.
var size = DefaultHash().Size()

func checkLen(data []byte) error {
if len(data) != size {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if this is hash.Hahs length it should be in hash package , what do you think?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is used with different parameters: 2 with directly Hash, 2 with []byte.
So it doesn't make sense to put it as a struct function (func (h *Hash) checkLen(data []byte) error {).

it should be in hash package

it's already in hash package.

@krhubert krhubert merged commit 4ea418a into dev Nov 18, 2019
@krhubert krhubert deleted the fix/hash-len branch November 18, 2019 07:41
@NicolasMahe NicolasMahe added the release:fix Pull requests that fix something label Nov 26, 2019
@NicolasMahe NicolasMahe mentioned this pull request Nov 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:fix Pull requests that fix something
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants