in_http: Content-Type rejected for application/json when encoding value is present.

[metalfork]

Updated application/json Content-Type header detect to allow detecting application/json when length is longer than 16 character because of addition encoding in the header value, aka Content-type: application/json; charset=utf-8

---

Enter [N/A] in the box, if an item is not applicable to your change.

Testing Before we can approve your change; please submit the following in a comment:

• [N/A] Example configuration file for the change
• [N/A] Debug log output from testing the change

• [N/A] Attached Valgrind output that shows no leaks or memory corruption was found

If this is a change to packaging of containers or native binaries then please confirm it works for all targets.

• [N/A] Run local packaging test showing all targets (including any new ones) build.
• [N/A] Set ok-package-test label to test for all targets (requires maintainer to do).

Documentation

• [N/A] Documentation required for this feature

Backporting

• [N/A] Backport to latest stable release.

---

Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.

[metalfork]

Current issue: https://github.com/fluent/fluent-bit/issues/9018 Other issue related to this PR fix https://github.com/fluent/fluent-bit/issues/5062

[patrick-stephens]

It would probably be good to add some simple tests as well to prevent any future regressions?

[patrick-stephens]

Can you update your commit messages to follow the contribution guide format?

[metalfork]

It would probably be good to add some simple tests as well to prevent any future regressions?

@patrick-stephens I wasn't able to get my local environment setup to correctly produce tests but based on the docs I took a stab at it: https://github.com/fluent/fluent-bit/pull/9023/commits/e66f29da653660cd2ade4e7508e374e3fd09eae7

[pwhelan]

I managed to mostly get the test running with this patch:

diff --git a/tests/runtime/in_http.c b/tests/runtime/in_http.c
index 5bb440265..4fc4e342f 100644
--- a/tests/runtime/in_http.c
+++ b/tests/runtime/in_http.c
@@ -351,8 +351,9 @@ void flb_test_http_successful_response_code(char *response_code)
     test_ctx_destroy(ctx);
 }
 
-void flb_test_http_json_charset_header_successful_response_code(char *response_code)
+void flb_test_http_json_charset_header()
 {
+    char *response_code = "200";
     struct flb_lib_out_cb cb_data;
     struct test_ctx *ctx;
     struct flb_http_client *c;
@@ -360,7 +361,7 @@ void flb_test_http_json_charset_header_successful_response_code(char *response_c
     int num;
     size_t b_sent;
 
-    char *buf = "{\"test\":\"msg\"}";
+    char *buf = "[{\"test\":\"msg\"}]";
 
     clear_output_num();
 
@@ -391,6 +392,8 @@ void flb_test_http_json_charset_header_successful_response_code(char *response_c
     ctx->httpc = http_client_ctx_create();
     TEST_CHECK(ctx->httpc != NULL);
 
+    flb_time_msleep(1500);
+
     c = flb_http_client(ctx->httpc->u_conn, FLB_HTTP_POST, "/", buf, strlen(buf),
                         "127.0.0.1", 9880, NULL, 0);
     ret = flb_http_add_header(c, FLB_HTTP_HEADER_CONTENT_TYPE, strlen(FLB_HTTP_HEADER_CONTENT_TYPE),
@@ -662,6 +665,7 @@ TEST_LIST = {
     {"successful_response_code_204", flb_test_http_successful_response_code_204},
     {"failure_response_code_400_bad_json", flb_test_http_failure_400_bad_json},
     {"failure_response_code_400_bad_disk_write", flb_test_http_failure_400_bad_disk_write},
+    {"json_charset_header", flb_test_http_json_charset_header},
     {"tag_key", flb_test_http_tag_key},
     {NULL, NULL}
 };

The test runs but fails since the http service is responding with 400.

The code works when I test it directly via the command line so there must be something the test code is not doing correctly.

[metalfork]

The test runs but fails since the http service is responding with 400.

@pwhelan Is it possible the test server is just rejecting the request based on the content type header? Is it checking length to ensure it 16 characters long? Encoding should default to utf-8 so I would assume test http server would not perform any additional validation.

[patrick-stephens]

Note the change is failing to compile on the older compiler for CentOS 7 which is a hard block on merging.

[metalfork]

Note the change is failing to compile on the older compiler for CentOS 7 which is a hard block on merging.

This issue looks to be a DNS issue.

[patrick-stephens]

Note the change is failing to compile on the older compiler for CentOS 7 which is a hard block on merging.

This issue looks to be a DNS issue.

Ah no, CentOS 7 is EOL now so that's why. The repos are all unavailable and likely we have to switch over to the vault ones.

[metalfork]

Note the change is failing to compile on the older compiler for CentOS 7 which is a hard block on merging.

This issue looks to be a DNS issue.

Ah no, CentOS 7 is EOL now so that's why. The repos are all unavailable and likely we have to switch over to the vault ones.

@patrick-stephens is there someone specific on the team that needs to be notified of this ops change?

[patrick-stephens]

Note the change is failing to compile on the older compiler for CentOS 7 which is a hard block on merging.

This issue looks to be a DNS issue.

Ah no, CentOS 7 is EOL now so that's why. The repos are all unavailable and likely we have to switch over to the vault ones.

@patrick-stephens is there someone specific on the team that needs to be notified of this ops change?

See #9043, it's being worked on.

[pwhelan]

The test runs but fails since the http service is responding with 400.

@pwhelan Is it possible the test server is just rejecting the request based on the content type header? Is it checking length to ensure it 16 characters long? Encoding should default to utf-8 so I would assume test http server would not perform any additional validation.

I was hoping you might pick up my test fixes and see why it was failing. I think I can give it another look right now.

*** edit ***

My test was not setting http2=off for the in_http plugin so it was failing due to the code not being implemented for http2 yet.

it works now:

╭─pwhelan@plaptop ~/fluent-bit/build ‹fix_issue_9018●›
╰─$ ./bin/flb-rt-in_http json_charset_header
Test json_charset_header...                     [ OK ]
SUCCESS: All unit tests have passed.

Here is the updated patch:

diff --git a/tests/runtime/in_http.c b/tests/runtime/in_http.c
index 5bb440265..69a4fad28 100644
--- a/tests/runtime/in_http.c
+++ b/tests/runtime/in_http.c
@@ -351,8 +351,9 @@ void flb_test_http_successful_response_code(char *response_code)
     test_ctx_destroy(ctx);
 }

-void flb_test_http_json_charset_header_successful_response_code(char *response_code)
+void flb_test_http_json_charset_header()
 {
+    char *response_code = "200";
     struct flb_lib_out_cb cb_data;
     struct test_ctx *ctx;
     struct flb_http_client *c;
@@ -360,7 +361,7 @@ void flb_test_http_json_charset_header_successful_response_code(char *response_c
     int num;
     size_t b_sent;

-    char *buf = "{\"test\":\"msg\"}";
+    char *buf = "[{\"test\":\"msg\"}]";

     clear_output_num();

@@ -374,6 +375,7 @@ void flb_test_http_json_charset_header_successful_response_code(char *response_c
     }

     ret = flb_input_set(ctx->flb, ctx->i_ffd,
+                        "http2", "off",
                         "successful_response_code", response_code,
                          NULL);
     TEST_CHECK(ret == 0);
@@ -391,6 +393,8 @@ void flb_test_http_json_charset_header_successful_response_code(char *response_c
     ctx->httpc = http_client_ctx_create();
     TEST_CHECK(ctx->httpc != NULL);

+    flb_time_msleep(1500);
+
     c = flb_http_client(ctx->httpc->u_conn, FLB_HTTP_POST, "/", buf, strlen(buf),
                         "127.0.0.1", 9880, NULL, 0);
     ret = flb_http_add_header(c, FLB_HTTP_HEADER_CONTENT_TYPE, strlen(FLB_HTTP_HEADER_CONTENT_TYPE),
@@ -662,6 +666,7 @@ TEST_LIST = {
     {"successful_response_code_204", flb_test_http_successful_response_code_204},
     {"failure_response_code_400_bad_json", flb_test_http_failure_400_bad_json},
     {"failure_response_code_400_bad_disk_write", flb_test_http_failure_400_bad_disk_write},
+    {"json_charset_header", flb_test_http_json_charset_header},
     {"tag_key", flb_test_http_tag_key},
     {NULL, NULL}
 };

[metalfork]

Here is the updated patch:

@pwhelan thank you. I applied your test patch https://github.com/fluent/fluent-bit/pull/9023/commits/daee63ed0f745f89f272d75241ed4278be68743f

[edsiper]

@metalfork please signoff the commits (DCO error)

[edsiper]

unit tests are failing, moving it to the next milestone

[metalfork]

unit tests are failing, moving it to the next milestone

@edsiper I believe I corrected the failing test https://github.com/fluent/fluent-bit/pull/9023/commits/48ebaad055fc2984f7d091af7e88feba4c1cfa82 https://github.com/fluent/fluent-bit/pull/9023/commits/ad1fd07c5c404c509b4ba31a1b316ed0cc8c0466

CC @pwhelan

[metalfork]

@pwhelan can you please kick the unit test workflow https://github.com/fluent/fluent-bit/actions/runs/9858806052 ?

[metalfork]

@pwhelan not being able to run the unit tests locally is definitely a problem on my end. Sorry for the churn.

[metalfork]

@pwhelan can you please kick the Unit tests workflow when you get a chance.

[metalfork]

@patrick-stephens @pwhelan any follow-up from me still needed on this PR?

[metalfork]

Should this be moved to Milestone 3.1.4?

[leonardo-albertovich]

Hi @metalfork, do you think you'll be able to make those changes?

[metalfork]

Hi @metalfork, do you think you'll be able to make those changes?

Updated

[pwhelan]

The failing test happened due to github, as far as I can tell.

There are still changes in the files outside of the in_http plugin. If those changes cannot be reverted in the commit history this PR pretty much cannot be merged.

The easiest way forward might be to clone a fresh repository, add the changed files on top, commit again and push with --force (or just open a new PR instead).