fix: make ztls handshake timeout interruptible

[drmabus]

/claim #819
Closes #819

Handshake() was previously executed inline inside a select, which prevented ctx.Done() from interrupting the TLS handshake and made timeouts ineffective.

This change moves the handshake into a goroutine and properly races ctx.Done() against the handshake result.

Changes

• Run Handshake() asynchronously in a goroutine
• Properly handle timeout via ctx.Done()
• Remove unnecessary channel close
• Preserve ErrCertsOnly handling

Notes

A test failure was observed due to a nil pointer dereference after a handshake error in the existing test logic, which appears unrelated to this change.

Summary by CodeRabbit

• Refactor
  • Improved TLS handshake timeout handling by running the handshake asynchronously and simplifying error flow. This makes connection timeouts more reliable and the handshake path clearer.
  • Maintains existing certificate-only behavior so observable TLS outcomes remain unchanged for users.

[neo-by-projectdiscovery-dev]

Neo - PR Security Review

No security issues found

Highlights

• Removes unnecessary channel close operation from tlsHandshakeWithTimeout, improving code safety
• Goroutine and channel handling remain correct with buffered channel preventing leaks
• No exploitable security vulnerabilities introduced in this commit

_{Comment @pdneo help for available commands. · Open in Neo}

[coderabbitai]

Walkthrough

tlsHandshakeWithTimeout now runs the TLS handshake in a goroutine, sending the handshake error to a channel and returning directly from the select branches; deferred channel close and the extra post-select receive were removed. tls.ErrCertsOnly is still mapped to nil before returning.

Changes

Cohort / File(s)	Summary
TLS Handshake Timeout Refactoring pkg/tlsx/ztls/ztls.go	Handshake is performed in a separate goroutine with results sent to an error channel; select branches return directly. Removed deferred close(errChan) and the second receive after the select. Preserves mapping of tls.ErrCertsOnly to nil.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 I nudged the handshake into a little run,
A goroutine hop — quick work, lots of fun.
Channels hum softly, timeouts tap the drum,
The rabbit grins: concurrent chores are done. 🥕

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check	✅ Passed	The title 'fix: make ztls handshake timeout interruptible' clearly and concisely describes the main change: moving the TLS handshake to a goroutine to allow proper timeout interruption via context cancellation.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

📝 Coding Plan

• Generate coding plan for human review comments

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

pkg/tlsx/ztls/ztls.go (1)

327-338: Fix is correct; prefer errors.Is() for error comparison.

The change correctly moves the handshake into a goroutine, allowing ctx.Done() to properly race against the handshake result. The buffered channel (size 1) ensures the goroutine won't block when timeout occurs.

However, using direct equality for error comparison at line 335 is less idiomatic and won't catch wrapped errors. Consider using errors.Is():

♻️ Suggested improvement

 	case err := <-errChan:
-		if err == tls.ErrCertsOnly {
+		if errors.Is(err, tls.ErrCertsOnly) {
 			err = nil
 		}
 		return err

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/tlsx/ztls/ztls.go` around lines 327 - 338, Replace the direct error
equality check against tls.ErrsOnly in the handshake goroutine result with
errors.Is to correctly detect wrapped errors: inside the select case that
receives err from errChan (the handshake launched with tlsConn.Handshake()),
change the comparison `if err == tls.ErrCertsOnly` to `if errors.Is(err,
tls.ErrCertsOnly)` and ensure the package imports the standard "errors" package
if not already present so wrapped errors are handled properly.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@pkg/tlsx/ztls/ztls.go`:
- Around line 327-338: Replace the direct error equality check against
tls.ErrsOnly in the handshake goroutine result with errors.Is to correctly
detect wrapped errors: inside the select case that receives err from errChan
(the handshake launched with tlsConn.Handshake()), change the comparison `if err
== tls.ErrCertsOnly` to `if errors.Is(err, tls.ErrCertsOnly)` and ensure the
package imports the standard "errors" package if not already present so wrapped
errors are handled properly.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 10059aa3-b6f3-4f3c-98f6-cc8a20fe2195

📥 Commits

Reviewing files that changed from the base of the PR and between d13b67f and 5547d4a.

📒 Files selected for processing (1)

• pkg/tlsx/ztls/ztls.go

[drmabus]

updated to use errors.Is for proper wrapped error handling.

[Mzack9999]

Thanks for the contribution! This has been resolved by #949 which was merged into dev. Your fix correctly identified the synchronous handshake issue, but didn't address the goroutine leak on timeout (needs rawConn.Close() to unblock the stuck handshake) or the missing timeout contexts in EnumerateCiphers. Closing as superseded.