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

Adding user specified HTTP headers to HTTP JSON data adapter. #4094

Merged
merged 3 commits into from Aug 25, 2017

Conversation

Projects
None yet
2 participants
@dennisoelkers
Member

dennisoelkers commented Aug 16, 2017

Description

Motivation and Context

This allows the user to specify and manage HTTP headers for data
adapters. These are required for all kinds of purposes, especially for
access tokens required to access certain external services.

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
Adding user specified HTTP headers to HTTP JSON data adapter.
This allows the user to specify and manage HTTP headers for data
adapters. These are required for all kinds of purposes, especially for
access tokens required to access certain external services.
@joschi

The current implementation doesn't allow overriding the Accept or User-Agent HTTP request headers when defining them as additional HTTP headers.

Instead of replacing the original headers, it adds an additional one.

Example with custom Accept: text/plain HTTP request header:

GET / HTTP/1.1
User-Agent: Graylog Lookup - https://www.graylog.org/
Accept: application/json
Foo: Bar
Accept: text/plain
Host: 127.0.0.1:12345
Connection: Keep-Alive
Accept-Encoding: gzip

The implementation should either replace the original headers or don't allow to override the Accept and User-Agent HTTP request headers in the "HTTP Headers" section.

@joschi

This comment has been minimized.

Contributor

joschi commented Aug 21, 2017

Another minor nitpick:
The implementation doesn't allow setting multiple values for the same HTTP request header, but that can be worked around by using a comma-separated list (instead of Foo: Bar and Foo: Baz simply use Foo: Bar, Baz, according to RFC 2616, section 4.2).

This should be properly documented.

@joschi joschi self-assigned this Aug 21, 2017

@dennisoelkers

This comment has been minimized.

Member

dennisoelkers commented Aug 22, 2017

@joschi Thanks for the very helpful comments, will address them.

dennisoelkers added some commits Aug 24, 2017

@dennisoelkers

This comment has been minimized.

Member

dennisoelkers commented Aug 24, 2017

@joschi: Addressed both. HTTP Headers will now be overwritten, added note about comma-separating multiple header values. Thanks!

@joschi

joschi approved these changes Aug 25, 2017

@joschi joschi merged commit f4fd0a1 into master Aug 25, 2017

5 checks passed

ci-web-linter Jenkins build graylog-pr-linter-check 1882 has succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
graylog-project/pr Jenkins build graylog-project-pr-snapshot 412 has succeeded
Details
license/cla Contributor License Agreement is signed.
Details

@joschi joschi deleted the add-headers-to-http-json-path-adapter branch Aug 25, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment