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

Add support for base64-encoded hash values to 'get_file_hash' #1339

Merged
merged 4 commits into from
Feb 20, 2021
Merged

Add support for base64-encoded hash values to 'get_file_hash' #1339

merged 4 commits into from
Feb 20, 2021

Conversation

SkypLabs
Copy link
Contributor

@SkypLabs SkypLabs commented Feb 4, 2021

IMPORTANT: Please do not create a Pull Request adding a new feature without discussing it first.

The place to discuss new features is the forum: https://zola.discourse.group/
If you want to add a new feature, please open a thread there first in the feature requests section.

Sanity check:

  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Code changes

(Delete or ignore this section for documentation changes)

  • Are you doing the PR on the next branch?

If the change is a new feature or adding to/changing an existing one:

  • Have you created/updated the relevant documentation page(s)?

This PR followings up with the discussion in #519.

The global template function 'get_file_hash' can now return a
base64-encoded hash value when its 'base64' parameter is set to true.

See discussion in #519.
SRI hash values must be base64-encoded.
@@ -95,18 +96,36 @@ fn compute_file_sha256(mut file: fs::File) -> result::Result<String, io::Error>
Ok(format!("{:x}", hasher.finalize()))
}

fn compute_file_sha256_base64(mut file: fs::File) -> result::Result<String, io::Error> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it will be cleaner to pass the base64 arg to those functions than to duplicate them all.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought about it but there were already 3 different functions for the 3 possible values of sha_type so I opted for keeping it consistent.

Wouldn't it be even better to pass sha_type and base64 to one unique function handling all the possible combinations?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't it be even better to pass sha_type and base64 to one unique function handling all the possible combinations?

Even better!

@SkypLabs
Copy link
Contributor Author

SkypLabs commented Feb 5, 2021

Documentation updated. I also fixed the SRI issue in test_site.

@Keats
Copy link
Collaborator

Keats commented Feb 19, 2021

Is it still a WIP somehow or ok to merge?

@SkypLabs
Copy link
Contributor Author

Hey @Keats.

I didn't find the time to implement your suggestion of merging the different hashing functions into a single one (see #1339 (comment)). However, my PR can be merged in its current state so it's up to you.

@Keats
Copy link
Collaborator

Keats commented Feb 20, 2021

I'll fix it post-merge, thanks!

@Keats Keats merged commit d3ab393 into getzola:next Feb 20, 2021
@Keats
Copy link
Collaborator

Keats commented Feb 20, 2021

3262f69

@SkypLabs
Copy link
Contributor Author

Awesome! Thanks

@SkypLabs SkypLabs changed the title [WIP] Add support for base64-encoded hash values to 'get_file_hash' Add support for base64-encoded hash values to 'get_file_hash' Feb 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants