Lowered the timeout from one minute to a default of 20 seconds - #5
Lowered the timeout from one minute to a default of 20 seconds#5Gijutsu wants to merge 1 commit into
Conversation
| s := &http.Server{ | ||
| Addr: os.Args[1], | ||
| ReadTimeout: 10 * time.Second, | ||
| ReadTimeout: 1 * time.Second, |
There was a problem hiding this comment.
Why change these? It's totally reasonable for a connection to stall for 1 second.
There was a problem hiding this comment.
I agree, this change is not necessary to lower the timeout when running against a host that is not responding. The only change that is actually needed is "deadline := time.Now().Add(2 * time.Second)" I'll amend my pull request to reflect this.
|
Cool, Could you squash? It may be a good idea to add this as a flag, since 2 seconds is in the realms of a TCP connection missing something, and in some cases you really want to avoid a false positive, and other times you are less caring :) |
|
I'm unlikely to put in a default lower than 20 seconds. Some of us have crappy connections :) Please make this a flag if you want to be able to configure something lower. |
by adding the command line flags timeout and address while keeping backwards compability by making the usage of the flags optional.
|
Thanks for the feedback. I can see the benefit of making this a flag, so I have converted the PR to use flags while keeping backwards compability by making the usage of the flags optional. Example usage: time ./CVE-2016-2107 8.8.8.8 real 0m20.002s ./CVE-2016-2107 -address filippo.io -timeout 2 |
which in my tests proved to be around 10 times the required time to get a response from a patched or vulnerable server - but it of course depends on your network latency.