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!: Use SHA-256 for the config file name #57

Merged
merged 3 commits into from
Nov 8, 2021
Merged

Conversation

silverwind
Copy link
Contributor

@silverwind silverwind commented Jun 13, 2020

Fixed a maximum line length eslint warning on this line as well. This change doubles the length of the file name from a previous 32 hex characters to 64.

SHA-256 was added in OpenSSL 0.9.8, released 2005-06-06, so every Node.js version should have it.

Fixes #56

Verified

This commit was signed with the committer’s verified signature.
Fixed a maximum line length eslint warning on this line as well. This
change doubles the length of the file name from a previous 32 hex
characters to 64.

Fixes: gulpjs#56
@phated
Copy link
Member

phated commented Jun 14, 2020

One thing that we need to be conscious of for this is that the increase in filename length might cause Windows systems to fail with a max filepath limit.

We might want to release this as a breaking change due to that ☝️

@silverwind
Copy link
Contributor Author

silverwind commented Jun 14, 2020

Indeed, thought I do consider it unlikely we'll reach MAX_PATH of 260 chars. In the general case the path will be something like

C:\Users\<username>\AppData\Local\js-v8flags\.v8flags-1-8.1.307.31-node.33.<hash>.json

which is around 135 chars + length of the username, so should be well within that limit.

@phated phated self-requested a review October 22, 2020 01:05
@tgflet
Copy link

tgflet commented Feb 18, 2021

Bump, I could really use this fix in a project. It is breaking the project when enforcing FIPS when enforcing FIPS because MD5 is not FIPS compliant @silverwind @phated

@Shawn1874

This comment has been minimized.

@phated
Copy link
Member

phated commented Jul 20, 2021

@Shawn1874 As with all open source, it'll get done when it gets done. It wasn't until recently that gulp started to have any significant sponsors since all funding disappeared since covid.

@Shawn1874

This comment has been minimized.

@phated phated changed the title Use SHA-256 for the config file name fix!: Use SHA-256 for the config file name Nov 8, 2021

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
index.js Outdated Show resolved Hide resolved

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@phated phated merged commit f30a18e into gulpjs:master Nov 8, 2021
@github-actions github-actions bot mentioned this pull request Nov 8, 2021
@phated
Copy link
Member

phated commented Nov 8, 2021

Thanks for this @silverwind! Sorry for the delay, but wanted to include this in the next major in case it breaks someone.

@silverwind silverwind deleted the sha branch November 8, 2021 17:30
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.

Consider using SHA-256 over MD5 for the config file
4 participants