szaszm commented on code in PR #1321:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1321#discussion_r868111368
##########
extensions/http-curl/processors/InvokeHTTP.cpp:
##########
@@ -115,6 +107,16 @@ core::Property InvokeHTTP::AlwaysOutputResponse("Always
Output Response", "Will
core::Property InvokeHTTP::PenalizeOnNoRetry("Penalize on \"No Retry\"",
"Enabling this property will penalize FlowFiles that are routed to the \"No
Retry\" relationship.", "false");
core::Property InvokeHTTP::DisablePeerVerification("Disable Peer
Verification", "Disables peer verification for the SSL session", "false");
+
+core::Property InvokeHTTP::InvalidHTTPHeaderFieldHandlingStrategy(
+ core::PropertyBuilder::createProperty("Invalid HTTP Header Field Handling
Strategy")
+ ->withDescription("Indicates what should happen when an attribute's name
is not a valid HTTP header field name. "
+ "Options: fail - flow file is transferred to failure, transform -
invalid characters are replaced, drop - drops invalid attributes from HTTP
message")
+ ->isRequired(true)
+
->withDefaultValue<std::string>(toString(InvalidHTTPHeaderFieldHandlingOption::FAIL))
+
->withAllowableValues<std::string>(InvalidHTTPHeaderFieldHandlingOption::values())
Review Comment:
Either of the other options is more useful than the FAIL. I would default to
something else. Leaning towards transform, but drop is fine, too.
##########
extensions/http-curl/client/HTTPClient.cpp:
##########
@@ -455,6 +455,39 @@ void HTTPClient::setFollowRedirects(bool follow) {
curl_easy_setopt(http_session_, CURLOPT_FOLLOWLOCATION, follow);
}
+bool HTTPClient::isValidHTTPHeaderField(std::string_view field_name) {
+ if (field_name.size() == 0) {
+ return false;
+ }
+
+ // RFC822 3.1.2: The field-name must be composed of printable ASCII
characters
+ // (i.e., characters that have values between 33. and 126., decimal,
except colon).
+ for (auto ch : field_name) {
+ if (ch < 33 || ch > 126 || ch == ':') {
+ return false;
+ }
+ }
+ return true;
+}
+
+std::string
HTTPClient::replaceInvalidCharactersInHTTPHeaderFieldName(std::string_view
field_name) {
+ if (field_name.size() == 0) {
+ return "";
+ }
Review Comment:
It might be better to limit the domain instead of extending the range. I
would just say that the range (of return values) is a valid HTTP header field
name. We should accept anything that we can transform into one, but an empty
string is not one. Is it possible to have attributes with empty names? If so,
maybe return a special name, like "X-MiNiFi-Empty-Attribute-Name".
This doesn't apply to `isValidHTTPHeaderField`, because the range is the
same there, so extending the domain strictly adds to the usefulness of the
function.
Please use one lowerCamelCase word even for acronyms, i.e. HTTP becomes
Http. For now, I wouldn't change the class name, but I would name the new
functions (including `isValidHTTPHeaderField`) accordingly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
