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

fix(brotli): brotli partial response fix #11087

Merged

Conversation

SilentEntity
Copy link
Contributor

@SilentEntity SilentEntity commented Mar 25, 2024

Description

When there is a chunk in the eof, brotli plugin overwrite the response arg[1]. Which results into partial response.

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

@shreemaan-abhishek
Copy link
Contributor

#11079 seems like an error raised due to incompatibility between HTTP headers and HTTP version. Could you please explain what the need for this fix?

And test cases are also required.

@SilentEntity
Copy link
Contributor Author

When client connection is slow, and page is loading in chunks[streaming], the last chuck is getting dropped/overwritten. If one see the code last chuck will contain eof [end of file], and according to the lib brotli-ffi the last chuck should be appended with compressor:finish(). But in the code if the last chunk is present, it's getting overwritten :

local chunk, eof = ngx.arg[1], ngx.arg[2]

if type(chunk) == "string" and chunk ~= "" then
    local encode_chunk = ctx.compressor:compress(chunk)
    ngx.arg[1] = encode_chunk .. ctx.compressor:flush()
end

if eof then
    ngx.arg[1] = ctx.compressor:finish()    <--- Problem

The fix should be to append the compressed ngx.arg[1] in the eof.

if eof then
        -- overwriting the arg[1], results into partial response
        ngx.arg[1] = ngx.arg[1] .. ctx.compressor:finish()   <--- fix

Even if the ngx.arg[1] is not present in eof, it won't impact the last chuck.

Test-case of this case is not producible, it will require network throttling with large upstream payload for real scenario.

@shreemaan-abhishek
Copy link
Contributor

okay makes sense now, thanks for the apt explanation.

@shreemaan-abhishek shreemaan-abhishek merged commit 5319503 into apache:master Mar 29, 2024
56 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: Compression plugin brotli is sending partial response
3 participants