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: #9018
Other issue related to this PR fix #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: e66f29d

[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]

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

[metalfork]

Should this be moved to Milestone 3.1.4?

[pwhelan]

LGTM

Awesome work getting the tests working.

[leonardo-albertovich]

Could you please revert all file changes caused by line encoding switches?

That would be even more important than the code change I requested because this PR should only touch the code file you changed and the test file where you added your test in order to be merged.

Regards.

[leonardo-albertovich]

According to RFC 2616, media types can have parameters which should be separated from the main type and subtype with a colon (and I think optionally a space) which means, in the case of a value longer than 16 characters you would have to verify that the seventeenth character is either a space a colon in order to avoid matching values like "application/json_but_with_some_twist_that_we_do_not_want_to_match".

[metalfork]

@leonardo-albertovich Did you mean semi-colon? ;?

There are things that need to be fixed before this can be merged.

[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).