-
Notifications
You must be signed in to change notification settings - Fork 16
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
feat: create a C++ sample plugin for HMAC token authorization. #114
base: main
Are you sure you want to change the base?
feat: create a C++ sample plugin for HMAC token authorization. #114
Conversation
Here is the summary of changes. You are about to add 1 region tag.
This comment is generated by snippet-bot.
|
|
||
FilterHeadersStatus onRequestHeaders(uint32_t headers, | ||
bool end_of_stream) override { | ||
boost::system::result<boost::urls::url> url = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be good to handle the case where the boost::system::result contains an error, i.e. has_value() returns false (or equivalently, has_error() returns true).
This applies to a few other existing plugins as well, e.g. plugins/samples/jwt_auth/plugin.cc and plugins/samples/ab_testing/plugin.cc, which both dereference a result without first checking if it is non-error. I'm guessing that would trigger an exception or undefined behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it... Added a URL parsing error check.
return FilterHeadersStatus::ContinueAndEndStream; | ||
} | ||
|
||
const auto token = (*it).value; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expand auto
into actual type, since it's not obvious from the expression alone
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
const auto token = (*it).value; | ||
// Strip the token from the URL. | ||
url->params().erase(it); | ||
const auto path = url->buffer(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expand auto
into actual type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
|
||
private: | ||
// Helper function to convert binary data to a hexadecimal string. | ||
std::string toHexString(const unsigned char* data, size_t length) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could absl::BytesToHexString() be used instead?
If this method needs to be retained, suggest passing data
and length
as a std::string_view
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
…e auto variables to more explicity type
This plugin is a HMAC token authorization showcase.
Technically, this is performed by ensuring that the request query string has a valid HMAC token.
This examples contains only a C++ version.