Allow request headers in HttpInputSource in native and MSQ Ingestion by pranavbhole · Pull Request #16974 · apache/druid

pranavbhole · 2024-08-28T21:32:48Z

Description

PR for adding the request headers in http input source. we can now pass the additional headers as json in both native and MSQ.
Examples below.

"inputSource" : {
      "type" : "http",
      "uris" : [ "http://example.com/foo.csv", "http://example.com/bar.csv" ],
      "httpAuthenticationUsername" : "bob",
      "httpAuthenticationPassword" : {
        "type" : "default",
        "password" : "secret"
      },
      "requestHeaders" : {
        "Accept" : "application/ndjson",
        "a" : "b"
      }
    },

INSERT INTO w000
SELECT
  TIME_PARSE("timestamp") AS __time,
  isRobot
FROM TABLE(http(
  userName => 'bob',
  password => 'secret',
  uris => ARRAY['http://example.com/foo.csv', 'http://example.com/bar.csv'],
  format => 'csv',
  headers=> '{"Accept":"application/ndjson", "a": "b" }'
  )
) EXTEND ("timestamp" VARCHAR, isRobot VARCHAR)
PARTITIONED BY HOUR

Release note

This PR has:

pranavbhole · 2024-08-28T21:33:00Z

Adding more tests and coverage.

abhishekagarwal87 · 2024-09-05T08:43:37Z

Please also add a corresponding runtime property to whitelist what header keys are allowed. The default can be empty and thus no header is allowed. These free-form property maps can create security holes.

abhishekagarwal87 · 2024-09-05T08:45:34Z

That runtime property should be added to HttpInputSourceConfig similar to allowedProtocols

abhishekagarwal87 · 2024-09-05T07:32:37Z

      @JsonProperty("httpAuthenticationUsername") @Nullable String httpAuthenticationUsername,
      @JsonProperty("httpAuthenticationPassword") @Nullable PasswordProvider httpAuthenticationPasswordProvider,
      @JsonProperty(SYSTEM_FIELDS_PROPERTY) @Nullable SystemFields systemFields,
+      @JsonProperty("additionalHeaders") @Nullable Map<String, String> headersMap,


we should rename this to requestHeaders everywhere.

abhishekagarwal87

Minor comments. Looks good otherwise.

abhishekagarwal87 · 2024-09-11T07:36:35Z

      throws IOException
  {
    final URLConnection urlConnection = object.toURL().openConnection();
+    if (requestHeaders.size() > 0) {


also need to check that requestHeaders is not null.

if not, then requestHeaders is not nullable.

abhishekagarwal87 · 2024-09-11T07:36:53Z

  private final PasswordProvider httpAuthenticationPasswordProvider;
  private final SystemFields systemFields;
  private final HttpInputSourceConfig config;
+  private final Map<String, String> headersMap;


Suggested change

private final Map<String, String> headersMap;

private final Map<String, String> requestHeaders;

abhishekagarwal87 · 2024-09-11T07:39:34Z

+    if (!config.getAllowedHeaders().isEmpty() && headersMap.size() > 0) {
+      Set<String> forbiddenHeaderSet = headersMap.keySet()
+                                                 .stream()
+                                                 .map(StringUtils::toLowerCase)
+                                                 .filter(h -> !config.getAllowedHeaders().contains(h))
+                                                 .collect(Collectors.toSet());
+      if (!forbiddenHeaderSet.isEmpty()) {
+        throw new IAE("Got forbidden headers %s, allowed headers are only %s ",
+                      forbiddenHeaderSet, config.getAllowedHeaders());
+      }
+    }


this could be simplified to

for key in headersMap if (!config.allowedHeaders.contains(key)) throw new IAE(" Header [%s] is not allowed to be set. Only headers are allowed are [%s]. You can allow the headers by changing property <insert property name> ", key, config.getAllowedHeaders());

Also please use InvalidInput.exception.

abhishekagarwal87 · 2024-09-11T07:41:38Z

can you add one test with non-empty headers map?

-      inputStreamPartial = HttpEntity.openInputStream(url, "", null, 5);
+      inputStream = HttpEntity.openInputStream(url, "", null, 0, Collections.emptyMap());
+      inputStreamPartial = HttpEntity.openInputStream(url, "", null, 5, Collections.emptyMap());
+      inputStream.skip(5);


abhishekagarwal87 · 2024-09-12T02:54:29Z

+  {
+    if (config.getAllowedHeaders().size() > 0) {
+      for (Map.Entry<String, String> entry : requestHeaders.entrySet()) {
+        if (!config.getAllowedHeaders().contains(StringUtils.toLowerCase(entry.getKey()))) {


are the keys in allowedHeaders always lower case?

pranavbhole · 2024-09-12T03:11:50Z

Yes, they are mapped as lowercase and stored in maps

…

On Wed, Sep 11, 2024, 7:56 PM Abhishek Agarwal ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In processing/src/main/java/org/apache/druid/data/input/impl/HttpInputSource.java <#16974 (comment)>: > @@ -100,6 +98,27 @@ public static void throwIfInvalidProtocols(HttpInputSourceConfig config, List<UR } } + public static void throwIfForbiddenHeaders(HttpInputSourceConfig config, Map<String, String> requestHeaders) + { + if (config.getAllowedHeaders().size() > 0) { + for (Map.Entry<String, String> entry : requestHeaders.entrySet()) { + if (!config.getAllowedHeaders().contains(StringUtils.toLowerCase(entry.getKey()))) { are the keys in allowedHeaders always lower case? — Reply to this email directly, view it on GitHub <#16974 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABFU6HWTHE5CVEUTEYTOB5LZWD7GTAVCNFSM6AAAAABNJEAG2WVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDEOJZGEYTSMZVGA> . You are receiving this because you authored the thread.Message ID: ***@***.***>

…pache#16974) Support for adding the request headers in http input source. we can now pass the additional headers as json in both native and MSQ.

Additional headers in HttpInputSource in native and MSQ

759eb45

github-actions Bot added Area - Querying Area - Ingestion labels Aug 28, 2024

pranavbhole changed the title ~~Additional headers in HttpInputSource in native and MSQ~~ Additional headers in HttpInputSource in native and MSQ (Wip) Aug 28, 2024

pranavbhole added 3 commits September 10, 2024 16:32

Adding allowedHeaders config

aaea5d1

fix check

2e78497

fix check

996c087

github-advanced-security AI found potential problems Sep 11, 2024

View reviewed changes

Comment thread processing/src/main/java/org/apache/druid/data/input/impl/HttpEntity.java Fixed

pranavbhole added 4 commits September 10, 2024 18:50

Fixing tests

877b734

More tests

cb93ada

More tests

c0219ff

fixing tests

9d4b32d

abhishekagarwal87 reviewed Sep 11, 2024

View reviewed changes

rename to requestHeaders

2613238

abhishekagarwal87 reviewed Sep 11, 2024

View reviewed changes

pranavbhole added 2 commits September 11, 2024 13:48

addressing comments

52a7dfb

addressing comments

afe9a33

github-advanced-security AI found potential problems Sep 11, 2024

View reviewed changes

addressing comments

831c3bc

abhishekagarwal87 reviewed Sep 12, 2024

View reviewed changes

abhishekagarwal87 approved these changes Sep 12, 2024

View reviewed changes

abhishekagarwal87 changed the title ~~Additional headers in HttpInputSource in native and MSQ (Wip)~~ Allow request headers in HttpInputSource in native and MSQ Ingestion Sep 12, 2024

abhishekagarwal87 merged commit a95397e into apache:master Sep 12, 2024

pranavbhole mentioned this pull request Sep 12, 2024

Update doc for allowedHeaders #17045

Merged

10 tasks

adarshsanjeev added this to the 32.0.0 milestone Jan 16, 2025

adarshsanjeev mentioned this pull request Jan 28, 2025

[DRAFT] 32.0.0 release notes #17677

Closed

	private final Map<String, String> headersMap;
	private final Map<String, String> requestHeaders;

Conversation

pranavbhole commented Aug 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Release note

Uh oh!

pranavbhole commented Aug 28, 2024

Uh oh!

abhishekagarwal87 commented Sep 5, 2024

Uh oh!

abhishekagarwal87 commented Sep 5, 2024

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abhishekagarwal87 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Check notice

Choose a reason for hiding this comment

Uh oh!

pranavbhole commented Sep 12, 2024 via email

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

pranavbhole commented Aug 28, 2024 •

edited

Loading