Skip to content
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.

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

FTP detector ignores context timeout #3714

Open
rgmz opened this issue Dec 2, 2024 · 4 comments
Open

FTP detector ignores context timeout #3714

rgmz opened this issue Dec 2, 2024 · 4 comments
Labels

Comments

@rgmz
Copy link
Contributor

rgmz commented Dec 2, 2024

2024-12-02T12:08:21-05:00 error trufflehog a detector ignored the context timeout {"detector_worker_id": "J5m7Y", "detector": {"type":"FTP"}, "timeout": 60, "link": "https://github.com/Samsung/ChromiumGStreamerBackend/blob/29c79f177ee954858d754c0faf6d237b4a8a7fbd/net/base/net_util_unittest.cc#L1"}}

@rgmz rgmz added the bug label Dec 2, 2024
@kashifkhan0771
Copy link
Contributor

The FTP detector does not use context for handling timeouts. Instead, it relies on this logic for setting timeouts. The issue comes because s.verificationTimeout is never set, causing it to always fall back to defaultVerificationTimeout, which is 5 seconds.

To address this, I tried a solution where the timeout is retrieved from the context instead of relying on the current logic.

func getContextTimeout(ctx context.Context) time.Duration {
	deadline, ok := ctx.Deadline()
	if !ok {
		return defaultVerificationTimeout
	}
	return time.Until(deadline)
}

With this change, the scan_duration matches whatever timeout I pass for --detector-timeout, as shown in the screenshot below. However, I noticed that if I pass --detector-timeout greater than 30 seconds, the scan_duration still caps at 30 seconds. It seems like there’s a limit enforced somewhere, but I haven’t checked where that happens yet.

Screenshot from 2024-12-30 18-57-29

@kashifkhan0771
Copy link
Contributor

If this solution looks fine, I can create a pull request.

@rgmz
Copy link
Contributor Author

rgmz commented Dec 30, 2024

The issue comes because s.verificationTimeout is never set, causing it to always fall back to defaultVerificationTimeout, which is 5 seconds.

While that is an issue, it's a bit more complicated. Even if the detector uses defaultVerificationTimeout, which is a reasonable limit, the call toftp.Dial can hang for much longer.

Running a local test with some additional logs we can see that the FTP detector waits 240s before returning an error:

2024-12-30T11:54:08-05:00	info-0	trufflehog	running source	{"source_manager_worker_id": "Azdxr", "with_units": true}
Dialing google.com:80...
...
2024-12-30T11:54:19-05:00	error	trufflehog	a detector ignored the context timeout	{"detector_worker_id": "vdHRj", "detector": {"type":"FTP"}, "timeout": 10}
Dial error: EOF...
2024-12-30T11:58:09-05:00	info-0	trufflehog	Finished verification after	{"detector_worker_id": "vdHRj", "detector": {"type":"FTP"}, "timeout": 10, "time": 240.179151834}

@kashifkhan0771
Copy link
Contributor

kashifkhan0771 commented Dec 31, 2024

That's weird because we are passing timeout to ftp.Dial here. Whatever timeout is set the ftp.Dial should use the same 🤔

Also, can you share the command you are using to run?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants