Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

x-pack/filebeat/input/http_endpoint: ensure request body is closed when content is gzipped #37091

Merged
merged 2 commits into from
Nov 13, 2023
Merged

x-pack/filebeat/input/http_endpoint: ensure request body is closed when content is gzipped #37091

merged 2 commits into from
Nov 13, 2023

Conversation

efd6
Copy link
Contributor

@efd6 efd6 commented Nov 12, 2023

Proposed commit message

See title.

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.

Author's Checklist

  • [ ]

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

@efd6 efd6 self-assigned this Nov 12, 2023
@botelastic botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Nov 12, 2023
@efd6 efd6 force-pushed the 36957-http_endpoint branch from c78a950 to 47e8ae3 Compare November 12, 2023 21:30
@elasticmachine
Copy link
Collaborator

elasticmachine commented Nov 12, 2023 •
edited by jenkins-beats-ci bot
Loading

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Duration: 137 min 40 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!)

@efd6 efd6 marked this pull request as ready for review November 13, 2023 01:00
@efd6 efd6 requested a review from a team as a code owner November 13, 2023 01:00
@elasticmachine
Copy link
Collaborator

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

bhapas
bhapas reviewed Nov 13, 2023
View reviewed changes
x-pack/filebeat/input/http_endpoint/gzip.go Outdated
// In order for the GZIP checksum to be verified, the reader must be
// fully consumed until the io.EOF.
//
// After this call the reader should not be reused because it is returned to the pool.
func (r *pooledGzipReader) Close() error {
err := r.Reader.Close()
_err := r.closer.Close()
if err == nil {
err = errors.Join(err, _err)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we ignore error from closer.Close() when Reader.Close() returns an error?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't, though the logic here can be simplified (what I have is a Frankenstein's monster that I should clean up). Thank you for raising it.

@efd6 efd6 requested a review from bhapas November 13, 2023 08:24
@efd6 efd6 enabled auto-merge (squash) November 13, 2023 08:36
@mergify Mergify

This comment was marked as outdated.

Sign in to view
@efd6 efd6 force-pushed the 36957-http_endpoint branch from 2a2c4b6 to e9deb31 Compare November 13, 2023 09:49
@efd6 efd6 merged commit 9f93f93 into elastic:main Nov 13, 2023
8 checks passed
mergify bot pushed a commit that referenced this pull request Nov 13, 2023
@efd6 @mergify
x-pack/filebeat/input/http_endpoint: ensure request body is closed wh…
a9aaa49 
…en content is gzipped (#37091)

(cherry picked from commit 9f93f93)
@mergify mergify bot mentioned this pull request Nov 13, 2023
Merged
efd6 added a commit that referenced this pull request Nov 13, 2023
@mergify @efd6
x-pack/filebeat/input/http_endpoint: ensure request body is closed wh…
05cc4f7 
…en content is gzipped (#37091) (#37106)

(cherry picked from commit 9f93f93)

Co-authored-by: Dan Kortschak <90160302+efd6@users.noreply.github.com>
Scholar-Li pushed a commit to Scholar-Li/beats that referenced this pull request Feb 5, 2024
@efd6 @Scholar-Li
x-pack/filebeat/input/http_endpoint: ensure request body is closed wh…
c46e28f 
…en content is gzipped (elastic#37091)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bhapas bhapas bhapas approved these changes

Assignees

@efd6 efd6

Labels
8.12-candidate backport-v8.11.0 Automated backport with mergify bug Filebeat Filebeat
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@efd6 @elasticmachine @bhapas