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

NIFI-6920 Add empty string key check for HTTP response headers before… #4517

Closed
wants to merge 3 commits into from

Conversation

agreenburg
Copy link

… adding as attributes

Description of PR

Fixes bug NIFI-6920

@@ -1276,7 +1276,7 @@ private String csv(Collection<String> values) {
// create a new hashmap to store the values from the connection
Map<String, String> map = new HashMap<>();
responseHttp.headers().names().forEach((key) -> {
Copy link
Contributor

Choose a reason for hiding this comment

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

You would want to use StringUtils.isBlank() see other parts of the code.

Copy link
Author

Choose a reason for hiding this comment

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

I had used the logic in org.apache.nifi.flowfile.FlowFile.KeyValidator.isValid(String) as a guideline, but I agree that this is a more appropriate solution.

@ottobackwards
Copy link
Contributor

This is pretty straight forward, +1 from me. You need a committer +1 to go with it

@ottobackwards
Copy link
Contributor

Also, thank you for the contribution!

@exceptionfactory
Copy link
Contributor

@agreenburg Thanks for your contribution! Understanding that this is a straightforward change, can you create a unit test method for InvokeHTTP to ensure that your solution filters out blank response header keys? The TestInvokeHttpCommon class includes a number of examples that should provide a starting point.

@exceptionfactory
Copy link
Contributor

@agreenburg Updates to InvokeHTTP in NIFI-8304 included cleaning up several methods, such as this one. Other recent updates include upgrading OkHttp to version 4.9.1. Based on these updates, I'm not sure if this PR is still applicable. It would be helpful if you could attempt to verify that the problem reported in NIFI-6920 still exists in recent versions of the processor.

@github-actions
Copy link

We're marking this PR as stale due to lack of updates in the past few months. If after another couple of weeks the stale label has not been removed this PR will be closed. This stale marker and eventual auto close does not indicate a judgement of the PR just lack of reviewer bandwidth and helps us keep the PR queue more manageable. If you would like this PR re-opened you can do so and a committer can remove the stale tag. Or you can open a new PR. Try to help review other PRs to increase PR review bandwidth which in turn helps yours.

@github-actions github-actions bot added the Stale label Jul 29, 2021
@github-actions github-actions bot closed this Aug 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants