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

patch fingerprinting func in templates #195

Merged
merged 2 commits into from
Jan 25, 2021
Merged

patch fingerprinting func in templates #195

merged 2 commits into from
Jan 25, 2021

Conversation

hlwjia
Copy link
Contributor

@hlwjia hlwjia commented Jan 24, 2021

Other params instead of md5 are sha256, sha512, sha384 (from Hugo 0.55).

fixes the "failed to find a valid digest..." error

reference: https://gohugo.io/hugo-pipes/fingerprint/
fixes the "failed to find a valid digest..." error

reference: https://gohugo.io/hugo-pipes/fingerprint/
@sonarcloud
Copy link

sonarcloud bot commented Jan 24, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@adityatelange
Copy link
Owner

@hlwjia The current usage is valid.
Read https://gohugo.io/hugo-pipes/introduction/#method-aliases

Also the default one used is sha256, as you can read here: https://gohugo.io/hugo-pipes/fingerprint/

So this won't fix "failed to find a valid digest..." error

@hlwjia
Copy link
Contributor Author

hlwjia commented Jan 24, 2021

@adityatelange I noticed that when I deployed a site of mine with your theme, I get the same hash from my css filename as yours at https://adityatelange.github.io/hugo-PaperMod/.

Starts with 9fcc3f6...

I thought maybe your fingerprint didn't work or something, so I did a quick search and found resources.Fingerprint. So I did a quick test with md5. It worked.

Now, after reading your reply. Thanks for that. I didn't know the method alias before. I got confused why it didn't work for me.

Then I tested with, again, resources.Fingerprint "sha256", which worked for me except using md5.

Guess what! It doesn't work again.

And then I tested with resources.Fingerprint "sha512".

Surprisingly, it works!

Now I'm really curious why only sha256 won't work for me while md5 and sha512 works perfectly.


Well, it's probably not the issue with your theme. But I'm just documenting what I got here.

Feel free to close this PR.

@hlwjia
Copy link
Contributor Author

hlwjia commented Jan 24, 2021

Just did another test with fingerprint "sha512", works a like charm.

Any clue why only the sha256 didn't work?

Again, feel free to close if it's not really what you'd like to think about. Technically, it's not an issue with this repo.

@adityatelange
Copy link
Owner

Any clue why only the sha256 didn't work?

Maybe gohugoio/hugo#7882 ? I'm not really sure but it sometimes does not recalculate, is what my experience is !

Copy link
Owner

@adityatelange adityatelange left a comment

Choose a reason for hiding this comment

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

I think we should merge this since this also generates a smaller hash, which does the same job.

@adityatelange adityatelange merged commit 9e99493 into adityatelange:master Jan 25, 2021
@hlwjia hlwjia deleted the hlwjia-patch-fingerprinting-func branch January 26, 2021 02:31
@hlwjia hlwjia restored the hlwjia-patch-fingerprinting-func branch January 26, 2021 02:33
@hlwjia
Copy link
Contributor Author

hlwjia commented Jan 26, 2021

Do you want me to change it back to use method alias fingerprint since it's not really the cause of the problem?

If you want, I can update it tofingerprint "md5" on my branch. And then create a new pull request.

I actually prefer cleaner alias :D

@adityatelange
Copy link
Owner

Do you want me to change it back to use method alias fingerprint since it's not really the cause of the problem?

If you want, I can update it tofingerprint "md5" on my branch. And then create a new pull request.

I actually prefer cleaner alias :D

9e99493#commitcomment-46358507

also this is an issue.

I didn't read docs https://www.w3.org/TR/SRI/#cryptographic-hash-functions before merging.

It says User agents should refuse to support known-weak hashing functions like MD5 or SHA-1, idk why hugo supports it ..

@hlwjia hlwjia deleted the hlwjia-patch-fingerprinting-func branch January 26, 2021 05:32
@hlwjia
Copy link
Contributor Author

hlwjia commented Jan 26, 2021

@adityatelange Thanks for the additional research.

Sorry for causing any inconvenience. :(

kylethedeveloper pushed a commit to kylethedeveloper/hugo-PaperMod that referenced this pull request Feb 21, 2023
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.

None yet

2 participants