Allow request headers in HttpInputSource in native and MSQ Ingestion #16974

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

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

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

processing/src/main/java/org/apache/druid/data/input/impl/HttpInputSource.java


  @JsonCreator
  public HttpInputSource(
      @JsonProperty("uris") List<URI> uris,
      @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

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

      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

processing/src/main/java/org/apache/druid/data/input/impl/HttpInputSource.java

@@ -64,13 +66,15 @@ public class HttpInputSource
  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

processing/src/main/java/org/apache/druid/data/input/impl/HttpInputSource.java

+    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

processing/src/test/java/org/apache/druid/data/input/impl/HttpEntityTest.java

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

processing/src/test/java/org/apache/druid/data/input/impl/HttpEntityTest.java

-      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

processing/src/main/java/org/apache/druid/data/input/impl/HttpInputSource.java

+  {
+    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 bot found potential problems Sep 11, 2024

View reviewed changes

processing/src/main/java/org/apache/druid/data/input/impl/HttpEntity.java Fixed Show fixed Hide 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 bot 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
88 of 90 checks passed

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow request headers in HttpInputSource in native and MSQ Ingestion #16974

Allow request headers in HttpInputSource in native and MSQ Ingestion #16974

pranavbhole commented Aug 28, 2024 •

edited

Loading

pranavbhole commented Aug 28, 2024

abhishekagarwal87 commented Sep 5, 2024

abhishekagarwal87 commented Sep 5, 2024

abhishekagarwal87 Sep 5, 2024

abhishekagarwal87 left a comment

abhishekagarwal87 Sep 11, 2024

abhishekagarwal87 Sep 11, 2024

pranavbhole Sep 11, 2024

abhishekagarwal87 Sep 11, 2024

pranavbhole Sep 11, 2024

abhishekagarwal87 Sep 11, 2024

abhishekagarwal87 Sep 11, 2024

pranavbhole Sep 11, 2024

abhishekagarwal87 Sep 11, 2024

pranavbhole Sep 11, 2024

Check notice

abhishekagarwal87 Sep 12, 2024

pranavbhole commented Sep 12, 2024 via email

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

Allow request headers in HttpInputSource in native and MSQ Ingestion #16974

Allow request headers in HttpInputSource in native and MSQ Ingestion #16974

Conversation

pranavbhole commented Aug 28, 2024 • edited Loading

Description

Release note

pranavbhole commented Aug 28, 2024

abhishekagarwal87 commented Sep 5, 2024

abhishekagarwal87 commented Sep 5, 2024

Choose a reason for hiding this comment

abhishekagarwal87 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Check notice

Choose a reason for hiding this comment

pranavbhole commented Sep 12, 2024 via email

pranavbhole commented Aug 28, 2024 •

edited

Loading