x-pack/filebeat/input/httpjson: Fix nil pointer deref #37591

chrisberkhout · 2024-01-10T10:34:12Z

Proposed commit message

x-pack/filebeat/input/httpjson: Fix basic auth nil pointer deref (#37591)

For chained requests, setting user and password values for basic
authentication via a pointer to a requestFactory struct was done before
the struct was initialized, resulting in a nil pointer dereference and
runtime panic. Moving it to after the initialization resolved the issue.

Checklist

My code follows the style guidelines of this project
~~I have commented my code, particularly in hard-to-understand areas~~
~~I have made corresponding changes to the documentation~~
~~I have made corresponding change to the default configuration files~~
I have added tests that prove my fix is effective or that my feature works
I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

How to test this PR locally

For manual testing, there's a filebeat.yml file below that will trigger the issue. The input fails before any requests are made.

Run ./filebeat -c filebeat.yml -v and check the logs for the error. When the requests are successfully made they can be seen on that webhook's page.

filebeat.inputs:
- type: httpjson
  id: my-test-httpjson-id
  enabled: true
  interval: 1m
  auth.basic.user: basicuser
  auth.basic.password: basicpass
  request.url: 'https://webhook.site/8d8e2d6d-4a79-4b9c-8b8d-f6641588ac52'
  request.method: POST
  request.body:
    message: "some message"
  request.ssl:
    verification_mode: none
  request.encode_as: application/json
  request.tracer.filename: http-request-trace-*.ndjson
  request.tracer.maxbackups: 5
  response.decode_as: application/json
  publisher_pipeline.disable_host: true
  chain:
    - step:
        request.url: >-
          https://webhook.site/8d8e2d6d-4a79-4b9c-8b8d-f6641588ac52/$.records[:].id
        request.method: GET
        replace: '$.records[:].id'
output.elasticsearch:
  hosts: ["localhost:9200"]

Related issues

Superseds x-pack/filebeat/input/httpjson: Fix nil pointer deref #37586
Bug introduced in [elastic/beats] [HttpJson] - Improves request chaining with new 'while' step & client creation per step #32222

mergify · 2024-01-10T10:34:48Z

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @chrisberkhout? 🙏.
For such, you'll need to label your PR with:

The upcoming major version of the Elastic Stack
The upcoming minor version of the Elastic Stack (if you're not pushing a breaking change)

To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

backport-v8./d.0 is the label to automatically backport to the 8./d branch. /d is the digit

elasticmachine · 2024-01-10T10:53:13Z

❕ Build Aborted

Either there was a build timeout or someone aborted the build.

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Duration: 18 min 30 sec

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.
/package : Generate the packages and run the E2E tests.
/beats-tester : Run the installation tests with beats-tester.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

elasticmachine · 2024-01-10T13:03:29Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2024-01-10T10:47:30.157+0000
Duration: 136 min 53 sec

Test stats 🧪

Test	Results
Failed	0
Passed	3235
Skipped	176
Total	3411

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.
/package : Generate the packages and run the E2E tests.
/beats-tester : Run the installation tests with beats-tester.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

elasticmachine · 2024-01-10T16:26:31Z

Pinging @elastic/security-external-integrations (Team:Security-External Integrations)

elasticmachine · 2024-01-10T18:44:26Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Duration: 137 min 7 sec

❕ Flaky test report

No test was executed to be analysed.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.
/package : Generate the packages and run the E2E tests.
/beats-tester : Run the installation tests with beats-tester.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

elasticmachine · 2024-01-11T10:53:22Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Duration: 140 min 41 sec

❕ Flaky test report

No test was executed to be analysed.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.
/package : Generate the packages and run the E2E tests.
/beats-tester : Run the installation tests with beats-tester.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

x-pack/filebeat/input/httpjson/request.go

bhapas

LGTM

chrisberkhout · 2024-01-12T07:37:21Z

@bhapas Sorry, I was a bit quick there and realized after pushing that I wasn't actually covering both Step and While cases (code under test only does While if there's no Step), so I expanded that test.

elasticmachine · 2024-01-12T07:43:03Z

❕ Build Aborted

There is a new build on-going so the previous on-going builds have been aborted.

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2024-01-12T07:15:01.865+0000
Duration: 28 min 56 sec

Test stats 🧪

Test	Results
Failed	6
Passed	4483
Skipped	188
Total	4677

Test errors

Expand to view the tests failures

`Build&Test / x-pack/filebeat-windows-2016-windows-2016 / TestParquetDecoding – github.com/elastic/beats/v7/x-pack/filebeat/input/awss3`

Expand to view the error details

 Failed

Expand to view the stacktrace

 === RUN   TestParquetDecoding
=== RUN   TestParquetDecoding/test_decoding_of_a_parquet_file_and_compare_the_number_of_events_with_batch_size_1
--- PASS: TestParquetDecoding/test_decoding_of_a_parquet_file_and_compare_the_number_of_events_with_batch_size_1 (3.08s)
=== RUN   TestParquetDecoding/test_decoding_of_a_parquet_file_and_compare_the_number_of_events_with_batch_size_100
--- PASS: TestParquetDecoding/test_decoding_of_a_parquet_file_and_compare_the_number_of_events_with_batch_size_100 (0.20s)
=== RUN   TestParquetDecoding/test_decoding_of_a_parquet_file_and_compare_the_number_of_events_with_default_parquet_config

`Build&Test / x-pack/filebeat-windows-2016-windows-2016 / TestParquetDecoding/test_decoding_of_a_parquet_file_and_compare_the_number_of_events_with_default_parquet_config – github.com/elastic/beats/v7/x-pack/filebeat/input/awss3`

Expand to view the error details

 Failed

Expand to view the stacktrace

 === RUN   TestParquetDecoding/test_decoding_of_a_parquet_file_and_compare_the_number_of_events_with_default_parquet_config

`Build&Test / x-pack/filebeat-windows-2016-windows-2016 / TestInput – github.com/elastic/beats/v7/x-pack/filebeat/input/cel`

Expand to view the error details

 Failed

Expand to view the stacktrace

 === RUN   TestInput
=== RUN   TestInput/hello_world
--- PASS: TestInput/hello_world (0.01s)
=== RUN   TestInput/hello_world_time
--- PASS: TestInput/hello_world_time (0.00s)
=== RUN   TestInput/bad_events_type
--- PASS: TestInput/bad_events_type (0.00s)
=== RUN   TestInput/hello_world_non_nil_state
--- PASS: TestInput/hello_world_non_nil_state (0.00s)
=== RUN   TestInput/what_is_next
--- PASS: TestInput/what_is_next (0.00s)
=== RUN   TestInput/bad_cursor_type
--- PASS: TestInput/bad_cursor_type (0.00s)
=== RUN   TestInput/show_state
--- PASS: TestInput/show_state (0.02s)
=== RUN   TestInput/show_provided_state
--- PASS: TestInput/show_provided_state (0.00s)
=== RUN   TestInput/iterative_state
--- PASS: TestInput/iterative_state (2.01s)
=== RUN   TestInput/iterative_state_implicit_initial_cursor

`Build&Test / x-pack/filebeat-windows-2016-windows-2016 / TestInput/iterative_state_implicit_initial_cursor – github.com/elastic/beats/v7/x-pack/filebeat/input/cel`

Expand to view the error details

 Failed

Expand to view the stacktrace

 === RUN   TestInput/iterative_state_implicit_initial_cursor

`Build&Test / x-pack/filebeat-windows-2016-windows-2016 / TestTransaction_Delete/delete-err – github.com/elastic/beats/v7/x-pack/filebeat/input/entityanalytics/internal/kvstore`

Expand to view the error details

 Failed

Expand to view the stacktrace

 === RUN   TestTransaction_Delete/delete-err
=== PAUSE TestTransaction_Delete/delete-err
=== CONT  TestTransaction_Delete/delete-err

`Build&Test / x-pack/filebeat-windows-2016-windows-2016 / TestTransaction_Delete – github.com/elastic/beats/v7/x-pack/filebeat/input/entityanalytics/internal/kvstore`

Expand to view the error details

 Failed

Expand to view the stacktrace

 === RUN   TestTransaction_Delete
=== RUN   TestTransaction_Delete/delete-ok
=== PAUSE TestTransaction_Delete/delete-ok
=== CONT  TestTransaction_Delete/delete-ok
--- PASS: TestTransaction_Delete/delete-ok (0.01s)
=== RUN   TestTransaction_Delete/delete-no-bucket
=== PAUSE TestTransaction_Delete/delete-no-bucket
=== CONT  TestTransaction_Delete/delete-no-bucket
--- PASS: TestTransaction_Delete/delete-no-bucket (0.05s)
=== RUN   TestTransaction_Delete/delete-err
=== PAUSE TestTransaction_Delete/delete-err
=== CONT  TestTransaction_Delete/delete-err

Steps errors

Expand to view the steps failures

`Error signal`

Took 0 min 0 sec . View more details here
Description: Error 'org.jenkinsci.plugins.workflow.steps.FlowInterruptedException'

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.
/package : Generate the packages and run the E2E tests.
/beats-tester : Run the installation tests with beats-tester.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

elasticmachine · 2024-01-12T09:50:18Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2024-01-12T07:36:02.916+0000
Duration: 134 min 9 sec

Test stats 🧪

Test	Results
Failed	0
Passed	3241
Skipped	176
Total	3417

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.
/package : Generate the packages and run the E2E tests.
/beats-tester : Run the installation tests with beats-tester.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

) For chained requests, setting user and password values for basic authentication via a pointer to a requestFactory struct was done before the struct was initialized, resulting in a nil pointer dereference and runtime panic. Moving it to after the initialization resolved the issue. (cherry picked from commit 0c387c5)

) (#37623) For chained requests, setting user and password values for basic authentication via a pointer to a requestFactory struct was done before the struct was initialized, resulting in a nil pointer dereference and runtime panic. Moving it to after the initialization resolved the issue. (cherry picked from commit 0c387c5) Co-authored-by: Chris Berkhout <[email protected]>

) (#37624) For chained requests, setting user and password values for basic authentication via a pointer to a requestFactory struct was done before the struct was initialized, resulting in a nil pointer dereference and runtime panic. Moving it to after the initialization resolved the issue. (cherry picked from commit 0c387c5)

…stic#37591) For chained requests, setting user and password values for basic authentication via a pointer to a requestFactory struct was done before the struct was initialized, resulting in a nil pointer dereference and runtime panic. Moving it to after the initialization resolved the issue.

botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Jan 10, 2024

chrisberkhout mentioned this pull request Jan 10, 2024

Update SQL and Oracle module docs regarding Oracle DSNs #37590

Merged

6 tasks

mergify bot assigned chrisberkhout Jan 10, 2024

chrisberkhout mentioned this pull request Jan 10, 2024

x-pack/filebeat/input/httpjson: Fix nil pointer deref #37586

Closed

6 tasks

chrisberkhout added Filebeat Filebeat x-pack Issues and pull requests for X-Pack features. Team:Security-External Integrations bugfix and removed needs_team Indicates that the issue/PR needs a Team:* label labels Jan 10, 2024

chrisberkhout added backport-v8.11.0 Automated backport with mergify backport-v8.12.0 Automated backport with mergify labels Jan 10, 2024

chrisberkhout requested a review from ShourieG January 10, 2024 16:26

chrisberkhout marked this pull request as ready for review January 10, 2024 16:26

chrisberkhout requested a review from a team as a code owner January 10, 2024 16:26

chrisberkhout force-pushed the httpjson-input-chain-basic-auth-init-fix branch from 770ab5f to 6252c15 Compare January 10, 2024 16:26

chrisberkhout force-pushed the httpjson-input-chain-basic-auth-init-fix branch from 6252c15 to 07d1199 Compare January 11, 2024 08:33

bhapas reviewed Jan 11, 2024

View reviewed changes

x-pack/filebeat/input/httpjson/request.go Show resolved Hide resolved

chrisberkhout added 3 commits January 12, 2024 08:14

Fix the httpjson input to avoid nil pointer deref.

2228084

CHANGELOG.next update

4f1b5d0

Add test.

3b2d817

chrisberkhout force-pushed the httpjson-input-chain-basic-auth-init-fix branch from 942c178 to 3b2d817 Compare January 12, 2024 07:14

bhapas approved these changes Jan 12, 2024

View reviewed changes

chrisberkhout added 2 commits January 12, 2024 08:23

Check both Step and While cases.

76e5462

Do it in a table.

99a8551

bhapas approved these changes Jan 12, 2024

View reviewed changes

chrisberkhout merged commit 0c387c5 into elastic:main Jan 12, 2024
8 checks passed

chrisberkhout deleted the httpjson-input-chain-basic-auth-init-fix branch January 12, 2024 09:58

This was referenced Jan 12, 2024

[8.11](backport #37591) x-pack/filebeat/input/httpjson: Fix nil pointer deref #37623

Merged

[8.12](backport #37591) x-pack/filebeat/input/httpjson: Fix nil pointer deref #37624

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

x-pack/filebeat/input/httpjson: Fix nil pointer deref #37591

x-pack/filebeat/input/httpjson: Fix nil pointer deref #37591

chrisberkhout commented Jan 10, 2024 •

edited

Loading

mergify bot commented Jan 10, 2024

elasticmachine commented Jan 10, 2024

Build stats

elasticmachine commented Jan 10, 2024

Build stats

Test stats 🧪

elasticmachine commented Jan 10, 2024

elasticmachine commented Jan 10, 2024

Build stats

elasticmachine commented Jan 11, 2024

Build stats

bhapas left a comment

chrisberkhout commented Jan 12, 2024

elasticmachine commented Jan 12, 2024

Build stats

Test stats 🧪

`Build&Test / x-pack/filebeat-windows-2016-windows-2016 / TestParquetDecoding – github.com/elastic/beats/v7/x-pack/filebeat/input/awss3`

`Build&Test / x-pack/filebeat-windows-2016-windows-2016 / TestParquetDecoding/test_decoding_of_a_parquet_file_and_compare_the_number_of_events_with_default_parquet_config – github.com/elastic/beats/v7/x-pack/filebeat/input/awss3`

`Build&Test / x-pack/filebeat-windows-2016-windows-2016 / TestInput – github.com/elastic/beats/v7/x-pack/filebeat/input/cel`

`Build&Test / x-pack/filebeat-windows-2016-windows-2016 / TestInput/iterative_state_implicit_initial_cursor – github.com/elastic/beats/v7/x-pack/filebeat/input/cel`

`Build&Test / x-pack/filebeat-windows-2016-windows-2016 / TestTransaction_Delete/delete-err – github.com/elastic/beats/v7/x-pack/filebeat/input/entityanalytics/internal/kvstore`

`Build&Test / x-pack/filebeat-windows-2016-windows-2016 / TestTransaction_Delete – github.com/elastic/beats/v7/x-pack/filebeat/input/entityanalytics/internal/kvstore`

`Error signal`

elasticmachine commented Jan 12, 2024

Build stats

Test stats 🧪

x-pack/filebeat/input/httpjson: Fix nil pointer deref #37591

x-pack/filebeat/input/httpjson: Fix nil pointer deref #37591

Conversation

chrisberkhout commented Jan 10, 2024 • edited Loading

Proposed commit message

Checklist

How to test this PR locally

Related issues

mergify bot commented Jan 10, 2024

elasticmachine commented Jan 10, 2024

❕ Build Aborted

Build stats

🤖 GitHub comments

elasticmachine commented Jan 10, 2024

💚 Build Succeeded

Build stats

Test stats 🧪

💚 Flaky test report

🤖 GitHub comments

elasticmachine commented Jan 10, 2024

elasticmachine commented Jan 10, 2024

💚 Build Succeeded

Build stats

❕ Flaky test report

🤖 GitHub comments

elasticmachine commented Jan 11, 2024

💚 Build Succeeded

Build stats

❕ Flaky test report

🤖 GitHub comments

bhapas left a comment

Choose a reason for hiding this comment

chrisberkhout commented Jan 12, 2024

elasticmachine commented Jan 12, 2024

❕ Build Aborted

Build stats

Test stats 🧪

Test errors

Build&Test / x-pack/filebeat-windows-2016-windows-2016 / TestParquetDecoding – github.com/elastic/beats/v7/x-pack/filebeat/input/awss3

Build&Test / x-pack/filebeat-windows-2016-windows-2016 / TestParquetDecoding/test_decoding_of_a_parquet_file_and_compare_the_number_of_events_with_default_parquet_config – github.com/elastic/beats/v7/x-pack/filebeat/input/awss3

Build&Test / x-pack/filebeat-windows-2016-windows-2016 / TestInput – github.com/elastic/beats/v7/x-pack/filebeat/input/cel

Build&Test / x-pack/filebeat-windows-2016-windows-2016 / TestInput/iterative_state_implicit_initial_cursor – github.com/elastic/beats/v7/x-pack/filebeat/input/cel

Build&Test / x-pack/filebeat-windows-2016-windows-2016 / TestTransaction_Delete/delete-err – github.com/elastic/beats/v7/x-pack/filebeat/input/entityanalytics/internal/kvstore

Build&Test / x-pack/filebeat-windows-2016-windows-2016 / TestTransaction_Delete – github.com/elastic/beats/v7/x-pack/filebeat/input/entityanalytics/internal/kvstore

Steps errors

Error signal

🤖 GitHub comments

elasticmachine commented Jan 12, 2024

💚 Build Succeeded

Build stats

Test stats 🧪

💚 Flaky test report

🤖 GitHub comments

chrisberkhout commented Jan 10, 2024 •

edited

Loading

`Build&Test / x-pack/filebeat-windows-2016-windows-2016 / TestParquetDecoding – github.com/elastic/beats/v7/x-pack/filebeat/input/awss3`

`Build&Test / x-pack/filebeat-windows-2016-windows-2016 / TestParquetDecoding/test_decoding_of_a_parquet_file_and_compare_the_number_of_events_with_default_parquet_config – github.com/elastic/beats/v7/x-pack/filebeat/input/awss3`

`Build&Test / x-pack/filebeat-windows-2016-windows-2016 / TestInput – github.com/elastic/beats/v7/x-pack/filebeat/input/cel`

`Build&Test / x-pack/filebeat-windows-2016-windows-2016 / TestInput/iterative_state_implicit_initial_cursor – github.com/elastic/beats/v7/x-pack/filebeat/input/cel`

`Build&Test / x-pack/filebeat-windows-2016-windows-2016 / TestTransaction_Delete/delete-err – github.com/elastic/beats/v7/x-pack/filebeat/input/entityanalytics/internal/kvstore`

`Build&Test / x-pack/filebeat-windows-2016-windows-2016 / TestTransaction_Delete – github.com/elastic/beats/v7/x-pack/filebeat/input/entityanalytics/internal/kvstore`

`Error signal`