Fixes server ignores request to gzip data

[rose2221]

What type of PR is this?

Bug fix

What does this PR do? Why is it needed?

This PR fixes an issue where the Prysm beacon chain server ignores the Accept-Encoding: gzip header in HTTP requests and always returns uncompressed JSON responses.

Fix Details:

• Updated middleware to properly respect the Accept-Encoding header.
• Ensured that responses are compressed when requested.
• Added error handling to prevent silent failures.

Which issue(s) does this PR fix?

Fixes #14593

Other Notes for Review

Acknowledgements

• [✅] I have read CONTRIBUTING.md.
• [✅] I have included a uniquely named changelog fragment file.
• [✅] I have added a description to this PR with sufficient context for reviewers to understand this PR.

[CLAassistant]

All committers have signed the CLA.

[rkapka]

Hey @rose2221 , thanks for the contribution! This PR only adds gzip to the blobs endpoint, but it would make much more sense to have support on all endpoints that return data. Which means all endpoint that use middleware.AcceptHeaderHandler.

Before merging the PR you will also have to sign the Contributor License Agreement and add a changelog file.

[rose2221]

Thank you, @rkapka, for your suggestions! I've implemented the requested changes. Let me know if there's anything else that needs adjustment. Also, should I add tests for this as well?

[rkapka]

Also, should I add tests for this as well?

That would be very helpful. Perhaps you could write a unit test where you:

• have a dummy handler that returns some value
• register the gzip middleware for this handler
• execute the handler and check that the returned data is NOT equal to the value that the handler returns (meaning that it got zipped - this is to prevent a scenario where nothing gets zipped and the next step does nothing, but the whole test passes)
• unzip the returned data and see that it's equal to the value returned from the handler

[rose2221]

Also, should I add tests for this as well?

That would be very helpful. Perhaps you could write a unit test where you:

• have a dummy handler that returns some value
• register the gzip middleware for this handler
• execute the handler and check that the returned data is NOT equal to the value that the handler returns (meaning that it got zipped - this is to prevent a scenario where nothing gets zipped and the next step does nothing, but the whole test passes)
• unzip the returned data and see that it's equal to the value returned from the handler

Thank you, I've implemented the test as suggested and I'm open to any further feedback. @rkapka

[james-prysm]

please fix linting bazel run //:gazelle -- fix

[james-prysm]

There's something wrong with this formatting when I first read it I thought it was on the return above

[james-prysm]

I'm a little worried about the usage of this will need to read up more on this

[rkapka]

@rose2221 Thanks for the test. I would like to approve the PR, but you need to run bazel run //:gazelle -- fix to fix dependencies before this can be merged.

[rkapka]

Merging the PR @rose2221 , thanks a lot for the contribution!

[rkapka]

@rose2221 For some reason our end-to-end tests fail with this PR. I can debug this myself but if you'd like to do it yourself you can run

bazel test //testing/endtoend:go_default_test --test_filter=TestEndToEnd_MinimalConfig --test_env=E2E_EPOCHS=4 --test_timeout=10000 --test_output=streamed --sandbox_debug

[rose2221]

@rose2221 For some reason our end-to-end tests fail with this PR. I can debug this myself but if you'd like to do it yourself you can run

bazel test //testing/endtoend:go_default_test --test_filter=TestEndToEnd_MinimalConfig --test_env=E2E_EPOCHS=4 --test_timeout=10000 --test_output=streamed --sandbox_debug

hey yes i would like to work on it

[rkapka]

Great! When the test fails, it outputs a path where all logs are stored. These are logs from the e2e run as well as beacon node logs (and some other as well but looking at beacon node logs should be enough)

[rose2221]

Great! When the test fails, it outputs a path where all logs are stored. These are logs from the e2e run as well as beacon node logs (and some other as well but looking at beacon node logs should be enough)

Hey @rkapka @james-prysm , sorry for the delay — I've fixed it now. Thanks for pointing out the log path.

[rkapka]

Simply ignoring SSZ is not correct

Ignoring SSZ is fine, but we should do it in a more reliable way. The user can specify two values in the Accept header, in which case even though application/octet-stream is a part of the header, the type of the response can still be application/json

[rkapka]

@rose2221 Do you mind cherry-picking 5dcbb52 into your branch? I believe this is the correct way to ignore SSZ responses

[rose2221]

@rose2221 Do you mind cherry-picking 5dcbb52 into your branch? I believe this is the correct way to ignore SSZ responses

sure ! will complete it in a day or two! thanks for the same!

[rkapka]

Also the PR has some conflicts now @rose2221

[rose2221]

Also the PR has some conflicts now @rose2221

i've applied the changes as suggested! thanks very much for the same!