feat: adding new scenario for the c++ check for PII responses sample plugin #136
Conversation
Signed-off-by: pweiber <periclesweiber@ciandt.com>
Signed-off-by: pweiber <periclesweiber@ciandt.com>
| // Note that for illustrative purposes, this example is kept simple and does not | ||
| // handle the case of credit card numbers that are split across multiple | ||
| // onResponseBody() calls. | ||
| // handle cases where PII is split across multiple onResponseBody() calls. | ||
| class MyHttpContext : public Context { | ||
| public: | ||
| explicit MyHttpContext(uint32_t id, RootContext* root) | ||
| : Context(id, root), root_(static_cast<MyRootContext*>(root)) {} | ||
|
|
There was a problem hiding this comment.
One current limitation is that this plugin won't strip PII as intended if the server response is compressed, since in that case the bytes returned by getBufferBytes() will be compressed rather than plaintext data.
The simplest workaround is to disallow the server from using response compression. This can be done by setting "Accept-Encoding: identity" in request headers. So I'd suggest defining an onRequestHeaders() that calls replaceRequestHeader("accept-encoding", "identity"); along with a comment explaining that this is to force the response to not be compressed.
There was a problem hiding this comment.
I added the comment explaining about it, let me know if the text is fine
plugins/samples/check_pii/plugin.cc
Outdated
| for (auto& p : pairs) { | ||
| std::string header_value = std::string(p.second); // mutable copy | ||
| if (maskPII(header_value)) { | ||
| replaceResponseHeader(p.first, header_value); |
There was a problem hiding this comment.
This is a bit of an uncommon case, but if a header appears multiple times in the request, this logic may end up collapsing the multiple occurrences into a single entry. For example, suppose the request has both of the following headers:
"credit-card: 0123-4567-8901-234"
"credit-card: 5678-9012-3456-7890"
Then after this processing, the response would only have the single header "credit-card: XXXX-XXXX-XXXX-7890".
You can avoid this problem by instead collecting the modified values in a HeaderStringPairs vector, and setting them all at once, e.g.:
const HeaderStringPairs orig_headers = getResponseHeaderPairs();
HeaderStringPairs new_headers;
bool changed = false;
for (auto [key, value] : orig_headers) {
changed = changed || maskPII(value);
new_headers.emplace_back(std::make_pair(std::move(key), std::move(value));
}
if (changed) {
setResponseHeaderPairs(new_headers);
}
There was a problem hiding this comment.
Oops, that code snippet has a bug. The line that updates changed should be:
changed = maskPII(value) || changed;
Otherwise, boolean short-circuiting will skip the call to maskPII() when changed is already true.
There was a problem hiding this comment.
I tried using
const HeaderStringPairs orig_headers = getResponseHeaderPairs();but I was getting
error: no viable conversion from 'WasmDataPtr' (aka 'unique_ptr<WasmData>') to 'const HeaderStringPairs' (aka 'const vector<pair<basic_string<char>, basic_string<char>>>')
so I tried a workaround using the original approach, I believe it should work as well, let me know what you think
Signed-off-by: pweiber <periclesweiber@ciandt.com>
As per request, this PR includes:
Tests were adjusted accordingly and also added some extra scenarios