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

Use strict base64 encoding (no newline) #3565

Merged
merged 1 commit into from
Apr 4, 2024
Merged

Use strict base64 encoding (no newline) #3565

merged 1 commit into from
Apr 4, 2024

Conversation

lloeki
Copy link
Contributor

@lloeki lloeki commented Apr 2, 2024

2.0 Upgrade Guide notes

What does this PR do?

Use strict base64 encoding.

Motivation:

MIME-style newlines in the middle of base64 makes for some unhappy results.

Additional Notes:

Reported case was with compressed data on appsec derivative results (e.g schema), introduced in 8c52213

This also applies to a few areas like RC capabilities, although the contents was smaller thus not affected (a trailing newline was handled with a chomp.

How to test the change?

CI covers it (added a non-regression test)

For Datadog employees:

  • If this PR touches code that signs or publishes builds or packages, or handles
    credentials of any kind, I've requested a review from @DataDog/security-design-and-guidance.
  • This PR doesn't touch any of that.

Unsure? Have a question? Request a review!

@lloeki lloeki requested review from a team as code owners April 2, 2024 10:08
@github-actions github-actions bot added appsec Application Security monitoring product core Involves Datadog core libraries labels Apr 2, 2024
Copy link
Member

@ivoanjo ivoanjo left a comment

Choose a reason for hiding this comment

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

👍 LGTM

I wonder if we should introduce a helper method for this -- not to avoid repeating code (it's one line), but because we wouldn't have to keep "remembering" to use the correct variant of base64 (although we'd need to remember to use the helper ;D )

@lloeki
Copy link
Contributor Author

lloeki commented Apr 3, 2024

we wouldn't have to keep "remembering" to use the correct variant of base64

The correct version depends on what you're doing. It just turns out that the changes here all use the non-MIME one but it's really a case by case.

(although we'd need to remember to use the helper ;D )

So true. It's also less explicit, hiding the fact that should be in plain sight and in context.

@lloeki lloeki merged commit b66cf9a into master Apr 4, 2024
207 checks passed
@lloeki lloeki deleted the fix-base64-encode branch April 4, 2024 11:38
@github-actions github-actions bot added this to the 1.22.0 milestone Apr 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
appsec Application Security monitoring product core Involves Datadog core libraries
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants