Skip to content

Add extra state for allow_half_open setting #6038

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

Closed
shinrich wants to merge 1 commit into from
Closed

Add extra state for allow_half_open setting #6038

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions doc/admin-guide/files/records.config.en.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4202,8 +4202,12 @@ Sockets
:reloadable:
:overridable:

Turn on or off support for connection half open for client side. Default is on, so
after client sends FIN, the connection is still there.
Controls whether ATS will continue to send data over a connection when the client side
closes (operating in a half open state). The client would have to be be written to
expect this to process the extra data. A value of 0 disables the half open connection from
consideration. A value of 1 cause |TS| to send data after receiving a FIN on non TLS connections.
A value of 2 will cause |TS| to also send data over TLS connections after the client sends a
FIN.

.. ts:cv:: CONFIG proxy.config.http.wait_for_cache INT 0

Expand Down
2 changes: 1 addition & 1 deletion mgmt/RecordsConfig.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ static const RecordElement RecordsConfig[] =
// # basics #
// ##########
// #
{RECT_CONFIG, "proxy.config.http.allow_half_open", RECD_INT, "1", RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-1]", RECA_NULL}
Copy link
Member

@zizhong zizhong Oct 22, 2019

Choose a reason for hiding this comment

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

do we want to change the default behavior to be half-open disabled?

{RECT_CONFIG, "proxy.config.http.allow_half_open", RECD_INT, "1", RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-2]", RECA_NULL}
,
{RECT_CONFIG, "proxy.config.http.enabled", RECD_INT, "1", RECU_RESTART_TM, RR_NULL, RECC_INT, "[0-1]", RECA_NULL}
,
Expand Down
10 changes: 7 additions & 3 deletions proxy/http/Http1ClientSession.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -523,10 +523,14 @@ Http1ClientSession::start()
}

bool
Http1ClientSession::allow_half_open() const
Http1ClientSession::allow_half_open(bool allow_half_open_tls) const
{
// Only allow half open connections if the not over TLS
return (client_vc && dynamic_cast<SSLNetVConnection *>(client_vc) == nullptr);
if (allow_half_open_tls) {
return true;
} else {
// Only allow half open connections if the not over TLS
return (client_vc && dynamic_cast<SSLNetVConnection *>(client_vc) == nullptr);
}
}

void
Expand Down
2 changes: 1 addition & 1 deletion proxy/http/Http1ClientSession.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ class Http1ClientSession : public ProxySession
void reenable(VIO *vio) override;

// Accessor Methods
bool allow_half_open() const;
bool allow_half_open(bool half_open_tls) const;
void set_half_close_flag(bool flag) override;
bool get_half_close_flag() const override;
bool is_chunked_encoding_supported() const override;
Expand Down
5 changes: 3 additions & 2 deletions proxy/http/Http1Transaction.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,11 @@ Http1Transaction::reenable(VIO *vio)
bool
Http1Transaction::allow_half_open() const
{
bool config_allows_it = (_sm) ? _sm->t_state.txn_conf->allow_half_open > 0 : true;
bool config_allows_it = (_sm) ? _sm->t_state.txn_conf->allow_half_open > 0 : false;
bool config_allows_tls_half_open = (_sm) ? _sm->t_state.txn_conf->allow_half_open > 1 : false;
if (config_allows_it) {
// Check with the session to make sure the underlying transport allows the half open scenario
return static_cast<Http1ClientSession *>(_proxy_ssn)->allow_half_open();
return static_cast<Http1ClientSession *>(_proxy_ssn)->allow_half_open(config_allows_tls_half_open);
}
return false;
}
Expand Down
11 changes: 9 additions & 2 deletions proxy/http/HttpSM.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3250,6 +3250,9 @@ HttpSM::tunnel_handler_ua(int event, HttpTunnelConsumer *c)
close_connection = false;
}
}
// Transaction is done. Clear the half open flag so the client
// connection can close the next time it is asked
ua_txn->set_half_close_flag(false);
break;
case VC_EVENT_WRITE_READY:
case VC_EVENT_READ_READY:
Expand Down Expand Up @@ -3292,12 +3295,16 @@ HttpSM::tunnel_handler_ua(int event, HttpTunnelConsumer *c)
if (close_connection) {
// If the client could be pipelining or is doing a POST, we need to
// set the ua_txn into half close mode
// For POST, this needs to be done in case the POST resulted in an error.
// Must set the half_open here so the POST clean up occurs as described
// in TS-3778. I would think in a success case, the half_open flag is unneeded
// since the transaction has completed.

// only external POSTs should be subject to this logic; ruling out internal POSTs here
bool is_eligible_post_request = ((t_state.method == HTTP_WKSIDX_POST) && !is_internal);

if ((is_eligible_post_request || t_state.client_info.pipeline_possible == true) && c->producer->vc_type != HT_STATIC &&
event == VC_EVENT_WRITE_COMPLETE) {
if ((is_eligible_post_request || t_state.client_info.pipeline_possible == true) && ua_txn->allow_half_open() &&
c->producer->vc_type != HT_STATIC && event == VC_EVENT_WRITE_COMPLETE) {
ua_txn->set_half_close_flag(true);
}

Expand Down