bzp2010 opened a new pull request, #11988: URL: https://github.com/apache/apisix/pull/11988
### Description This PR is intended to address the following issues: - Sometimes grpc-web requests incorrectly contain two trailer blocks, which according to grpc's protocol requirements must exist and exist only once. (https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-WEB.md#protocol-differences-vs-grpc-over-http2 Message framing) The reason for this is easy to explain, the old code used `status` to assert whether the response body had finished. But this is unreliable, and it can go wrong in the following cases: Since the response is chunked, the trailer chunk from upstream may not be fully included in the first response block, so the upstream_trailer_grpc_message variable is not available in the first body_filter, which causes APISIX to trigger the body_filter call a second time and incorrectly add a duplicate trailer block. ``` RESP HEADER: balabala --- first chunk, eof = false RESP BODY: balabala grpc-status: 0 --- second chunk, eof = true RESP BODY: grpc-message: "demo" --- done ``` Based on the pseudo-code above, you'll see that when this happens, the `upstream_trailer_grpc_status` value is already available to us at the first `body_filter` call, which causes the plugin code to send a trailer chunk. When the next upstream chunk is received, `body_filter` is called a second time, and `upstream_trailer_grpc_status` still has the value it had before, so the plugin code sends the trailer block again. This results in multiple duplicate trailers and accidentally breaks the grpc-web js client in the browser. This PR changes the end of the response asserted using the `status` value to use the `ngx.arg[2]` value, which will be the most reliable and trigger the code to send the trailer only on the last chunk. It does not add or modify any existing behavior, so it does not result in test case modifications either. ### Checklist - [x] 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 - [x] I have verified that this change is backward compatible (If not, please discuss on the [APISIX mailing list](https://github.com/apache/apisix/tree/master#community) first) <!-- Note 1. Mark the PR as draft until it's ready to be reviewed. 2. Always add/update tests for any changes unless you have a good reason. 3. Always update the documentation to reflect the changes made in the PR. 4. Make a new commit to resolve conversations instead of `push -f`. 5. To resolve merge conflicts, merge master instead of rebasing. 6. Use "request review" to notify the reviewer after making changes. 7. Only a reviewer can mark a conversation as resolved. --> -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
