Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add configurable connection timeout #20

Merged
merged 2 commits into from
Jan 9, 2024
Merged

Conversation

sollie
Copy link
Contributor

@sollie sollie commented Jan 4, 2024

Quick hack to add configurable connection timeout. Useful in scripts when scanning a ramge of hosts.

@TrueSkrillor
Copy link
Contributor

As tscanner is supposed to be an importable library, I'd like to keep the interface stable. As Golang does not support optional parameters (or default values), I'd suggest that we introduce this feature as another function, ScanWithTimeout; Scan will then call ScanWithTimeout internally. Otherwise, I'm happy to merge these changes.

@sollie
Copy link
Contributor Author

sollie commented Jan 8, 2024

As Golang does not support optional parameters (or default values)

One could turn Scan() into a variadic function, and only care about the first argument.

Something like

func Scan(address string, scanMode ScanMode, verbose bool, timeout ...int) (*Report, error) {
	if len(timeout) == 1 {
		// set timeout
	} else if len(timeout) > 1 {
		// error out, more parameters than expected
	} else {
		// use default timeout
	}

For simplicity the default timeout could be specified in Scan().
I don't think this would have any downsides?

@TrueSkrillor
Copy link
Contributor

While this would be possible, it seems overly complicated for what this is trying to achieve. I decided to add ScanWithTimeout as a separate function, converting Scan to a placeholder calling ScanWithTimeout instead. Also, I changed the parameter to be of type int, not *int to allow literal values to be passed to the function.

@TrueSkrillor TrueSkrillor merged commit 2c2dc70 into RUB-NDS:main Jan 9, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants