add Transport config options to limit the number of handshakes #4248
Conversation
marten-seemann
force-pushed
the
retry-config
branch
from
1b455b5 to
80c52e4
Compare
marten-seemann
force-pushed
the
server-accept-closed-connections
branch
from
908a1ae to
28c730d
Compare
marten-seemann
force-pushed
the
retry-config
branch
from
80c52e4 to
80194b5
Compare
marten-seemann
changed the base branch from
server-accept-closed-connections
to
master
marten-seemann
force-pushed
the
retry-config
branch
from
80194b5 to
6dfc56d
Compare
marten-seemann
force-pushed
the
retry-config
branch
from
6dfc56d to
7951e15
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #4248 +/- ##
==========================================
- Coverage 84.09% 84.08% -0.00%
==========================================
Files 150 150
Lines 15401 15431 +30
==========================================
+ Hits 12950 12975 +25
- Misses 1950 1953 +3
- Partials 501 503 +2 ☔ View full report in Codecov by Sentry. |
transport.go
Outdated
| @@ -18,6 +18,11 @@ import ( | |||
|
|
|||
| var errListenerAlreadySet = errors.New("listener already set") | |||
|
|
|||
| const ( | |||
| defaultMaxNumUnvalidatedHandshakes = 32 | |||
| defaultMaxNumHandshakes = 64 | |||
There was a problem hiding this comment.
These values seem unreasonably low.
There was a problem hiding this comment.
Maybe we should set defaultMaxNumHandshakes to unlimited? It's really hard to pick a limit that fits for all.
There was a problem hiding this comment.
64 feels too low for defaultMaxNumHandshakes. This number is way too easy to DoS for even very small machines. I think a high value > 1000 is better. I think unlimited is also better than this being < 1000.
There was a problem hiding this comment.
Changed to unlimited in 77ede93. Not entirely happy with that, but we can always change it later once we come up with a better value.
marten-seemann
force-pushed
the
retry-config
branch
from
77ede93 to
ded0673
Compare
…go#4248) * add Transport config options to limit the number of handshakes * fix accounting for failed handshakes * increase handshake limits, improve documentation
Fixes #3549.