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

1.27: Add base64url fallback to base64_decode (#330) #337

Closed
wants to merge 5 commits into from

Conversation

nfuden
Copy link
Collaborator

@nfuden nfuden commented May 9, 2024

Support for base64url in transformations

* add base64url

* clean up cout

* example of test failing without proposed change

* add discrete callbacks for base64url

* fallback on standard base64 for base64url decode

* keep existing behavior without fallback
@solo-changelog-bot
Copy link

Issues linked to changelog:
#329

@nfuden nfuden requested a review from ashishb-solo May 9, 2024 14:20
Copy link
Contributor

@ashishb-solo ashishb-solo left a comment

Choose a reason for hiding this comment

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

a few comments, happy to discuss if it would be helpful

transformer.transform(headers, &headers, body, callbacks);
EXPECT_NE(std::string::npos, body.toString().find(expect_string));
}

Copy link
Contributor

Choose a reason for hiding this comment

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

would it be possible to add a test where we test for equality of the base64-decode callback?

@@ -66,7 +66,9 @@ class TransformerInstance {
nlohmann::json env(const inja::Arguments &args) const;
nlohmann::json cluster_metadata_callback(const inja::Arguments &args) const;
nlohmann::json base64_encode_callback(const inja::Arguments &args) const;
nlohmann::json base64url_encode_callback(const inja::Arguments &args) const;
Copy link
Contributor

Choose a reason for hiding this comment

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

can we also add a test for base64url_encode?

changelog/v1.27.5-patch2/base64url.yaml Outdated Show resolved Hide resolved
changelog/v1.27.5-patch2/base64url.yaml Outdated Show resolved Hide resolved
@nfuden nfuden closed this May 29, 2024
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