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

hashing support #243

Merged
merged 10 commits into from
Jul 15, 2024
Merged

hashing support #243

merged 10 commits into from
Jul 15, 2024

Conversation

pschrammel
Copy link
Contributor

I need some help here to have hashing configurable

@pschrammel pschrammel requested a review from a team as a code owner June 24, 2024 10:20
@pschrammel pschrammel changed the title hasing support hashing support Jun 24, 2024
@guilhermef
Copy link
Member

Hi @pschrammel, what's the use case for hashing in the storage?

@pschrammel
Copy link
Contributor Author

pschrammel commented Jun 27, 2024

It is a feature of TC_AWS that we were using for several reasons:

  • we use full urls (with https:// ....) to load the original image and that was leading to very strange s3 keys
  • we don't want to have our url structure visible in s3

I switched from sha1 (original in TC_AWS) to sha256 as sha1 is regarded unsafe by our security scanners though in this case it wouldn't matter

@pschrammel
Copy link
Contributor Author

@guilhermef , I changed the concept, getting my very specific hashing is hard to get upstream. The idea now is that you can define a lambda expression that does your personal normalization magic. I hope this approach is more generic and easier to accept.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 9775890186

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 20 of 20 (100.0%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.07%) to 91.618%

Totals Coverage Status
Change from base Build 9687721972: 0.07%
Covered Lines: 317
Relevant Lines: 346

💛 - Coveralls

@guilhermef guilhermef merged commit 7159efc into thumbor:main Jul 15, 2024
9 checks passed
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.

3 participants