Tcp timeout #3407
Tcp timeout #3407
Conversation
harshitashankar
requested review from
Sarthak160 and
gouravkrosx
as code owners
|
CLA Assistant Lite bot: I have read the CLA Document and I hereby sign the CLA 1 out of 2 committers have signed the CLA. |
|
The CLA check failed. Please ensure you have:
After fixing these issues, comment 'recheck' to trigger the workflow again. |
|
The CLA check failed. Please ensure you have:
After fixing these issues, comment 'recheck' to trigger the workflow again. |
| if err != nil { | ||
| utils.LogError(logger, err, "failed to dial the conn to destination server", zap.Uint32("proxy port", p.Port), zap.String("server address", dstAddr)) | ||
| // Send HTTP error response if we've already read an HTTP request | ||
| p.sendHTTPErrorResponse(srcConn, initialBuf, logger, 500, "Internal Server Error") |
There was a problem hiding this comment.
We can’t simply send an HTTP response to the srcConn without knowing whether the client is speaking HTTP, as this can cause an immediate protocol violation. A non-HTTP client (like SSH or a database client) will not understand the response, leading to a connection failure with a cryptic error like “Connection reset by peer” or “Protocol error.”
|
@harshitashankar Instead of adding an extra port-based check, we should refactor the logic so that we start listening to both the client and the server concurrently. Whichever side sends the first initial buffer can then be used to detect the protocol. Since in MySQL the server sends the first packet, we can inspect that buffer — if it’s MySQL, proceed with the MySQL flow; otherwise, fall back to the generic parser. |
2 participants
Describe the changes that are made
Links & References
Closes: #[issue number that will be closed through this PR]
🔗 Related PRs
🐞 Related Issues
📄 Related Documents
What type of PR is this? (check all applicable)
Added e2e test pipeline?
Added comments for hard-to-understand areas?
Added to documentation?
Are there any sample code or steps to test the changes?
Self Review done?
Any relevant screenshots, recordings or logs?
🧠 Semantics for PR Title & Branch Name
Please ensure your PR title and branch name follow the Keploy semantics:
📌 PR Semantics Guide
📌 Branch Semantics Guide
Examples:
fix: patch MongoDB document update bugfeat/#1-login-flow(You may skip mentioning the issue number in the branch name if the change is small and the PR description clearly explains it.)Additional checklist: