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 SHA256 digest to cached file name #61

Merged
merged 1 commit into from
Apr 19, 2024

Conversation

zFERDQFREZrzfq
Copy link
Contributor

@zFERDQFREZrzfq zFERDQFREZrzfq commented Apr 19, 2024

Replaces base64 encoding by SHA256 digest to generate cached files names.

Fixes caching failure when URL is too long (e.g. discord hosted ones).
Fixes illegal characters on Windows (namely '\') that can show up in base64.

I also did a patch for the last release (2.0.31) : https://github.com/zFERDQFREZrzfq/watermedia/tree/patch-cache-2.0.31

build.gradle Outdated Show resolved Hide resolved
@SrRapero720 SrRapero720 added Type: Enhancement New feature or request Status: Concerning This issue pulls me to different levels of concern Status: Waiting answer and removed Status: Concerning This issue pulls me to different levels of concern labels Apr 19, 2024
@SrRapero720 SrRapero720 added this to the WATERMeDIA v3 milestone Apr 19, 2024
@SrRapero720
Copy link
Member

you can PR the same patch for 2.0.x branch. with the requested changes ofc

@zFERDQFREZrzfq zFERDQFREZrzfq force-pushed the master branch 2 times, most recently from 6b32dc7 to a999f37 Compare April 19, 2024 21:22
@zFERDQFREZrzfq
Copy link
Contributor Author

Should I better import the org.apache.commons.codec.binary.Hex class in the top of leave it like this?

@SrRapero720
Copy link
Member

SrRapero720 commented Apr 19, 2024

for this branch master yes. import the Hex
minecraft 1.16.5 have commons-codec. But 1.12.2 is not testable to get if had the proper dependency (and if 1.12.2 have not commons-io i doubt have commons-codecs). so this patch can't be ported to 2.0.x

if can be removed Hex usage can be aproved on 2.0.x

@SrRapero720
Copy link
Member

honestly, if can be copied the method into a IOTool class can be way better, considering we only are using one method, not a worth amount of methods to justificate have another shaded library

@zFERDQFREZrzfq
Copy link
Contributor Author

for this branch master yes. import the Hex minecraft 1.16.5 have commons-codec. But 1.12.2 is not testable to get if had the proper dependency (and if 1.12.2 have not commons-io i doubt have commons-codecs). so this patch can't be ported to 2.0.x

if can be removed Hex usage can be aproved on 2.0.x

Oh I didn't knew that Minecraft must have the dependency too. I'm brand new to Minecraft modding so I don't know the shenanigans...
If we can't embed the package, then I need to write a function that convert bytes to hex string. I used commons-codecs to avoid reinventing the wheel.

@zFERDQFREZrzfq
Copy link
Contributor Author

honestly, if can be copied the method into a IOTool class can be way better, considering we only are using one method, not a worth amount of methods to justificate have another shaded library

Oh yes we can copy the function alone if this is allowed

Fixes caching failure when URL is too long (e.g. discord hosted ones).
Fixes illegal characters on Windows (namely '\') that can show up in base64.
@zFERDQFREZrzfq
Copy link
Contributor Author

I replaced the dependency to commons-codec as suggested :)

@SrRapero720 SrRapero720 added PR Status: Queue PR is aproved and done. Time to wait for the right time and removed Status: Waiting answer labels Apr 19, 2024
@SrRapero720 SrRapero720 merged commit c1482cc into WaterMediaTeam:master Apr 19, 2024
3 checks passed
zFERDQFREZrzfq added a commit to zFERDQFREZrzfq/watermedia that referenced this pull request Apr 19, 2024
Fixes caching failure when URL is too long (e.g. discord hosted ones).
Fixes illegal characters on Windows (namely '\') that can show up in base64.
SrRapero720 pushed a commit that referenced this pull request Apr 20, 2024
Fixes caching failure when URL is too long (e.g. discord hosted ones).
Fixes illegal characters on Windows (namely '\') that can show up in base64.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR Status: Queue PR is aproved and done. Time to wait for the right time Type: Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants