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.

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

in_forward: Use full width of gzip header for checking whether concatenated or not #9139

Merged
merged 3 commits into from
Aug 5, 2024

Conversation

cosmo0920
Copy link
Contributor

Using for concatenated gzip conformation with magic bytes, compression method and OS flags.

  • 0x1f & 0x8b
  • 8 (deflate)
  • skip 7 bytes
  • OS flags <--- Added in this PR

Follows up for #8665.


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:

  • Example configuration file for the change
  • Debug log output from testing the change
  • 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.

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

Documentation

  • Documentation required for this feature

Backporting

  • 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.

@cosmo0920 cosmo0920 added the ok-package-test Run PR packaging tests label Jul 29, 2024
return count;
}

size_t flb_gzip_concatenated_borders(const char *data, size_t len, size_t **out_borders, size_t border_count)
Copy link
Member

Choose a reason for hiding this comment

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

since this function does pretty much the same than _count plus setting up the borders, I suggest to merge both and make **out_borders optional, e.g: if set, write the borders on it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your suggestion.
Fixed at dcc9ca5.

return FLB_FALSE;
}

size_t flb_gzip_concatenated_count(const char *data, size_t len)
Copy link
Member

Choose a reason for hiding this comment

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

rename to flb_gzip_count() and merge it on flb_gzip.c

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done by dcc9ca5.

@cosmo0920
Copy link
Contributor Author

CentOS 7 failure on CI will be fixed by #9157.

@edsiper
Copy link
Member

edsiper commented Aug 2, 2024

@cosmo0920 to merge this, let's remove the number of commits since half of them are around initial implementation, so we just keep the last ones

@cosmo0920
Copy link
Contributor Author

Sure, I'll concatenated into one commit.

@cosmo0920
Copy link
Contributor Author

I squashed my commits into one commit.

@edsiper
Copy link
Member

edsiper commented Aug 4, 2024

it needs to be splitted in 2 or 3 commits:

image

it should be:

  • in_forward: ...
  • in_gzip: ...

or

  • in_forward: ...
  • in_gzip: ...
  • tests: internal: gzip: ...

@cosmo0920
Copy link
Contributor Author

cosmo0920 commented Aug 4, 2024

I split my commit for meaningful groups of commits.

@edsiper edsiper merged commit c15ca2f into master Aug 5, 2024
77 of 78 checks passed
@edsiper edsiper deleted the cosmo0920-widen-gzip-border-check branch August 5, 2024 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs-required ok-package-test Run PR packaging tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants