-
Notifications
You must be signed in to change notification settings - Fork 104
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
Expect header #2343
base: master
Are you sure you want to change the base?
Expect header #2343
Conversation
9dd9a0e
to
a58000b
Compare
a58000b
to
3939451
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just wanted to discuss if it's possible to reuse tfw_h1_send_resp()
.
* request into @seq_queue, to proccess responses in correct order. | ||
*/ | ||
static int | ||
tfw_http_send_continuation(TfwCliConn *cli_conn, TfwHttpReq *req) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why don't we use tfw_h1_send_resp()
? I didn't get which specifics we need for 100-continue, but maybe it's better to make tfw_h1_send_resp()
API more flexible rather than keeping an alternate workflow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tfw_h1_send_resp()
uses tfw_http_resp_fwd()
that can't be used in this case without modifications, when tfw_http_resp_fwd()
called it expects that request already in seq_queue
, however tfw_http_send_continuation()
sends requests immediately if seq_queue
is empty and tfw_http_send_continuation()
doesn't free request paired with response, but tfw_http_resp_fwd()
does.
Modifications of tfw_http_resp_fwd()
will overcomplicate the function, because of this I created a new one, that do only what we need in the case of 100-continue.
However, I think we can call tfw_http_send_continuation()
from tfw_h1_send_resp()
if extend API of tfw_h1_send_resp()
. But it may have some overhead when error occurred during response allocation. Is it worth to do that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I think we're good with the current code. Could you please add the 1st paragraph of your answer as a comment for tfw_http_send_continuation()
Add new special header `Expect` with only one allowed value `101-continue`. See RFC 9110 Section 10.1.1.
Tempesta FW processes `Expect` itself and doesn't forward this header to upstream to avoid unnecessary traffic.
Once `Expect: 100-continue` received and no content received, Tempesta FW return `100-continue` response. When Tempesta received complete request, `Expect` header will be removed from the request during forwarding request to upstream. Tempesta never sends `Expect` to upstream, this header supported only when received from client. In case of pipelined requests, when first request dosn't contain `Expect` header, but second does, Tempesta responds in the same order as requests were received. The first will be sent response to the first request, then `100-continue` will be sent and after final response to the second request will be sent. This is achieved by using connection `seq_queue`, once `Expect` header received, `100-continue` response will be paired with corresponding request in the `seq_queue`, however if for some reason `100-continue` has not been sent and part of the body or whole body was received, 100-continue will be unpaired from request and freed, the request will be deleted from the `seq_queue` until it's not be processed completely. Therefore in this case only final response will be sent.
3939451
to
738ceca
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* request into @seq_queue, to proccess responses in correct order. | ||
*/ | ||
static int | ||
tfw_http_send_continuation(TfwCliConn *cli_conn, TfwHttpReq *req) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I think we're good with the current code. Could you please add the 1st paragraph of your answer as a comment for tfw_http_send_continuation()
No description provided.