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: chunk bodies and process partial #252

Merged
merged 6 commits into from
Jan 4, 2024

Conversation

M4tteoP
Copy link
Member

@M4tteoP M4tteoP commented Dec 22, 2023

The bodySize received by body callbacks is not the size of the chunk, but rather of the whole body that has been buffered up to that point in time (For more context: tetratelabs/proxy-wasm-go-sdk#418). This divergent usage of bodySize is the origin of the misbehaviour we were dealing with here: the body callback might be called more than once with the same bodySize. Trying to read more data from the buffer leads to the error status returned by host: not found error.

With this PR, the overall management of the body callbacks looks way better. Before and after verbose logs receiving a big payload highlight the differences:

  • No more errors are returned, not trying to read data that is not present.
  • Less error-prone code reading exactly the expected size of the body chunk.
  • ProcessRequestBody is not called anymore twice: once the request limit is reached, we can verify if the process partial action did not trigger an interruption. It means that from now on we have to consider that the body has been processed.
  • Thanks to the previous point, further request data is no longer retrieved and sent to Coraza, but rather is directly sent upstream. It should lead to better memory usage and overall improved performance.

Before:

OnHttpRequestBody called: bodySize: 14220, endOfStream: false

Reading 14220 bytes of request body starting from 0, really read: 14220. EOS: false. GetHttpRequestBody error: <nil>

OnHttpRequestBody called: bodySize: 14220, endOfStream: false

Reading 14220 bytes of request body starting from 14220, really read: 0. EOS: false. GetHttpRequestBody error: error status returned by host: not found

OnHttpRequestBody called: bodySize: 71068, endOfStream: false

Reading 71068 bytes of request body starting from 14220, really read: 56848. EOS: false. GetHttpRequestBody error: <nil>

Processing request body whose size reached the configured limit (Action ProcessPartial)

OnHttpRequestBody called: bodySize: 81204, endOfStream: false

Reading 81204 bytes of request body starting from 85288, really read: 0. EOS: false. GetHttpRequestBody error: error status returned by host: not found

OnHttpRequestBody called: bodySize: 156876, endOfStream: false

Reading 156876 bytes of request body starting from 85288, really read: 71588. EOS: false. GetHttpRequestBody error: <nil>

OnHttpRequestBody called: bodySize: 165028, endOfStream: false

Reading 165028 bytes of request body starting from 242164, really read: 0. EOS: false. GetHttpRequestBody error: error status returned by host: not found

OnHttpRequestBody called: bodySize: 256628, endOfStream: false

Reading 256628 bytes of request body starting from 242164, really read: 14464. EOS: false. GetHttpRequestBody error: <nil>

OnHttpRequestBody called: bodySize: 270572, endOfStream: false

Reading 270572 bytes of request body starting from 498792, really read: 0. EOS: false. GetHttpRequestBody error: error status returned by host: not found

OnHttpRequestBody called: bodySize: 300302, endOfStream: true

Reading 300302 bytes of request body starting from 498792, really read: 0. EOS: true. GetHttpRequestBody error: error status returned by host: not found

ProcessRequestBody should have already been called tx_id="KzIrphAtjFDUdcHVhbP"

After:

OnHttpRequestBody called: bodySize: 14220, endOfStream: false

Reading 14220 bytes of request body starting from 0, really read: 14220. EOS: false. GetHttpRequestBody error: <nil>

OnHttpRequestBody called: bodySize: 14220, endOfStream: false

OnHttpRequestBody called: bodySize: 71068, endOfStream: false

Reading 56848 bytes of request body starting from 14220, really read: 56848. EOS: false. GetHttpRequestBody error: <nil>

Processing request body whose size reached the configured limit (Action ProcessPartial)

OnHttpRequestBody called: bodySize: 214754, endOfStream: false

OnHttpRequestBody called: bodySize: 0, endOfStream: true

Waiting for further discussion SDK side tetratelabs/proxy-wasm-go-sdk#418, before updating the dependency and removing the draft status.

@M4tteoP M4tteoP marked this pull request as ready for review January 2, 2024 13:28
@M4tteoP M4tteoP mentioned this pull request Jan 2, 2024
@M4tteoP M4tteoP merged commit 393d4d6 into corazawaf:main Jan 4, 2024
4 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.

2 participants