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

Separate function for future reusing #470

Merged
merged 6 commits into from
Dec 5, 2023
Merged

Conversation

babenek
Copy link
Contributor

@babenek babenek commented Dec 3, 2023

Description

Please include a summary of the change and which is fixed.

  • base64 decode function is extracted for reuse
  • applied padding auto augmentation for base64 JWT which may be without padding
  • ASN1 size check applied for small size case

How has this been tested?

Please describe the tests that you ran to verify your changes.

  • UnitTest
  • Benchmark

@codecov-commenter
Copy link

codecov-commenter commented Dec 3, 2023

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (841679e) 90.83% compared to head (cf06554) 90.81%.
Report is 1 commits behind head on main.

Files Patch % Lines
credsweeper/filters/value_github_check.py 88.00% 1 Missing and 2 partials ⚠️
credsweeper/utils/util.py 80.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #470      +/-   ##
==========================================
- Coverage   90.83%   90.81%   -0.03%     
==========================================
  Files         123      124       +1     
  Lines        4169     4202      +33     
  Branches      658      663       +5     
==========================================
+ Hits         3787     3816      +29     
- Misses        252      254       +2     
- Partials      130      132       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@babenek babenek marked this pull request as ready for review December 3, 2023 11:58
@babenek babenek requested a review from a team as a code owner December 3, 2023 11:58
Copy link
Contributor

@kmnls kmnls left a comment

Choose a reason for hiding this comment

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

You always use (..., padding_safe=True, urlsafe_detect=True)
There is no need for code with False values of these parameters

@babenek
Copy link
Contributor Author

babenek commented Dec 4, 2023

You always use (..., padding_safe=True, urlsafe_detect=True) There is no need for code with False values of these parameters

I intend to use the function in PEM decoder https://github.com/Samsung/CredSweeper/pull/456/files#diff-9ebaed290816b8f8b45e2c2b198282a65c6fbc6b0584c0d0b5d55d3be88979a3

  • there is no padding safe required

@babenek babenek requested a review from kmnls December 4, 2023 09:23
Yullia
Yullia previously approved these changes Dec 4, 2023
credsweeper/utils/util.py Outdated Show resolved Hide resolved
Copy link
Contributor

@kmnls kmnls left a comment

Choose a reason for hiding this comment

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

See comment

@babenek babenek requested review from kmnls and Yullia December 5, 2023 06:49
@babenek
Copy link
Contributor Author

babenek commented Dec 5, 2023

See comment

Thank you for review. The issue was fixed.

Copy link
Collaborator

@csh519 csh519 left a comment

Choose a reason for hiding this comment

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

decode_base64() method has been extracted from ValueBase64DataCheck filter.

LGTM 👍

Copy link
Contributor

@xDizzix xDizzix left a comment

Choose a reason for hiding this comment

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

Copy link
Contributor

@kmnls kmnls left a comment

Choose a reason for hiding this comment

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

Agree

@babenek babenek merged commit 35be458 into Samsung:main Dec 5, 2023
29 checks passed
@babenek babenek deleted the base64urlsafe branch December 5, 2023 09:50
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.

6 participants