Skip to content

set a liberal default handshake timeout #612

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

Open
covener wants to merge 4 commits into
base: trunk
Choose a base branch
from
Open

set a liberal default handshake timeout #612

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f53797f
set a liberal default handshake timeout
Mar 17, 2026
c57cabd
Update compatibility note for handshake stage
covener Mar 17, 2026
5bf8b37
undo guardrails on handshake defaults
Mar 17, 2026
ba82742
fix docs re: 0/0/0
Mar 17, 2026
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
2 changes: 2 additions & 0 deletions changes-entries/reqtimeout-defaults.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
*) Add add a default handshake timeout (10s) to mod_reqtimeouts internal
defaults and the the default configuration. [Eric Covener]
2 changes: 1 addition & 1 deletion docs/conf/extra/httpd-default.conf.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,5 +86,5 @@ HostnameLookups Off
# To disable, set to header=0 body=0
#
<IfModule reqtimeout_module>
RequestReadTimeout header=20-40,MinRate=500 body=20,MinRate=500
RequestReadTimeout handshake=10,MinRate=250 header=20-40,MinRate=500 body=20,MinRate=500
</IfModule>
5 changes: 3 additions & 2 deletions docs/manual/mod/mod_reqtimeout.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,12 @@ the request headers and/or body from client.
[header=<var>timeout</var>[-<var>maxtimeout</var>][,MinRate=<var>rate</var>]
[body=<var>timeout</var>[-<var>maxtimeout</var>][,MinRate=<var>rate</var>]
</syntax>
<default>RequestReadTimeout handshake=0 header=20-40,MinRate=500 body=20,MinRate=500</default>
<default>RequestReadTimeout handshake=10,MinRate=250 header=20-40,MinRate=500 body=20,MinRate=500</default>
<contextlist><context>server config</context><context>virtual host</context>
</contextlist>
<compatibility>Defaulted to disabled in version 2.3.14 and earlier. The
<code>handshake</code> stage is available since version 2.4.39.
<code>handshake</code> stage is available since version 2.4.39, with non-zero
defaults added in 2.4.67.
</compatibility>

<usage>
Expand Down
11 changes: 4 additions & 7 deletions modules/filters/mod_reqtimeout.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@
module AP_MODULE_DECLARE_DATA reqtimeout_module;

#define UNSET -1
#define MRT_DEFAULT_handshake_TIMEOUT 0 /* disabled */
#define MRT_DEFAULT_handshake_TIMEOUT 10
#define MRT_DEFAULT_handshake_MAX_TIMEOUT 0
#define MRT_DEFAULT_handshake_MIN_RATE 0
#define MRT_DEFAULT_handshake_MIN_RATE 250
#define MRT_DEFAULT_header_TIMEOUT 20
#define MRT_DEFAULT_header_MAX_TIMEOUT 40
#define MRT_DEFAULT_header_MIN_RATE 500
Expand Down Expand Up @@ -375,8 +375,7 @@ static int reqtimeout_init(conn_rec *c)
&reqtimeout_module);
AP_DEBUG_ASSERT(cfg != NULL);

/* For compatibility, handshake timeout is disabled when UNSET (< 0) */
if (cfg->handshake.timeout <= 0
if (cfg->handshake.timeout == 0
&& cfg->header.timeout == 0
&& cfg->body.timeout == 0) {
/* disabled for this vhost */
Expand All @@ -391,9 +390,7 @@ static int reqtimeout_init(conn_rec *c)
ap_add_input_filter(reqtimeout_filter_name, ccfg, NULL, c);

ccfg->type = "handshake";
if (cfg->handshake.timeout > 0) {
INIT_STAGE(cfg, ccfg, handshake);
}
INIT_STAGE(cfg, ccfg, handshake);
Comment on lines -394 to +393
Copy link
Copy Markdown
Collaborator

@rpluem rpluem Mar 17, 2026

Choose a reason for hiding this comment

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

This seems wrong in the case that someone explicitly disabled a handshake timeout via configuration by setting it to 0, but has at least another timeout set.

Copy link
Copy Markdown
Collaborator

@rpluem rpluem Mar 17, 2026

Choose a reason for hiding this comment

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

Scratch this. I don't see now why the if was needed at all.

}

/* we are not handling the connection, we just do initialization */
Expand Down
Loading