Skip to content

A couple of HTTP compression fixes #1632

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
Merged
merged 2 commits into from
Aug 20, 2025
Merged

A couple of HTTP compression fixes #1632

merged 2 commits into from
Aug 20, 2025

Conversation

ac000
Copy link
Member

@ac000 ac000 commented Aug 11, 2025 

This pull-request comprises two commits

1) http: compression: Set the temporary file name in n_h_c_c_s_r()

The sets the temporary compressed file name in the nxt_file_t structure.

While we don't actually need it ourselves, it's used in the case that
nxt_file_close() fails.

2) http: compression: Add a missed nxt_http_comp_compress() return check

This adds a missing check for compression errors when compressing static
responses.

----------------------------------------------------------------
Andrew Clayton (2):
      http: compression: Set the temporary file name in n_h_c_c_s_r()
      http: compression: Add a missed nxt_http_comp_compress() return check

 src/nxt_http_compression.c | 26 ++++++++++++++++----------
 src/nxt_http_compression.h |  4 ++--
 src/nxt_http_static.c      |  2 +-
 3 files changed, 19 insertions(+), 13 deletions(-)

@ac000 ac000 marked this pull request as ready for review August 11, 2025 23:42
@ac000 ac000 requested a review from hongzhidao August 11, 2025 23:43
hongzhidao
hongzhidao approved these changes Aug 20, 2025
View reviewed changes
Copy link
Contributor

@hongzhidao hongzhidao left a comment

Choose a reason for hiding this comment

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

Look good.

ac000 added 2 commits August 20, 2025 13:55
@ac000
http: compression: Set the temporary file name in n_h_c_c_s_r()
6482e46 
When creating a new nxt_file_t structure in
nxt_http_comp_compress_static_response() for the temporary compressed
file be sure to set the *name* member.

We don't generally need it, but I failed to notice that when calling
nxt_file_close() if the close(2) fails then we log an error message
containing the file name, which at best would have just printed junk.

So set the file name for this particular error case...

This issue was reported by coverity.

Signed-off-by: Andrew Clayton <[email protected]>
@ac000
http: compression: Add a missed nxt_http_comp_compress() return check
5e97e44 
In nxt_http_comp_compress_static_response() we should check the return
value of the call to nxt_http_comp_compress() in case of error.

Signed-off-by: Andrew Clayton <[email protected]>
@ac000
Copy link
Member Author

ac000 commented Aug 20, 2025

Rebased with master

@ac000 ac000 merged commit 5e97e44 into nginx:master Aug 20, 2025
24 checks passed
@ac000 ac000 deleted the compr-fixes branch August 20, 2025 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hongzhidao hongzhidao hongzhidao approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ac000 @hongzhidao