Skip to content

Add generic TCP connection handler hook#3

Merged
abnegate merged 4 commits into
mainfrom
codex/postgres-sslrequest-reject
Jun 7, 2026
Merged

Add generic TCP connection handler hook#3
abnegate merged 4 commits into
mainfrom
codex/postgres-sslrequest-reject

Conversation

@abnegate
Copy link
Copy Markdown
Member

@abnegate abnegate commented Jun 6, 2026

Summary

  • remove PostgreSQL/MySQL protocol handling from the generic TCP proxy core
  • add a generic coroutine connection handler hook for callers that need protocol negotiation before routing
  • expose TCPAdapter::connect() so custom handlers can reuse proxy backend dialing/socket options after resolving a route

Verification

  • composer lint
  • composer check
  • composer test

Copilot AI review requested due to automatic review settings June 6, 2026 12:41
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Jun 6, 2026

Greptile Summary

This PR replaces the hardcoded PostgreSQL SSLRequest interception in both the event-driven and coroutine TCP servers with a generic connectionHandler hook in Config, letting protocol-aware callers implement their own negotiation before the proxy routes a backend. The TLS class is stripped of all protocol-detection constants and helpers, and TLSContext gains toSwooleProtocolConfig() for mid-connection socket upgrades.

  • Coroutine.php: listener is now always plaintext; connectionHandler is invoked per connection and may return true to take full ownership, otherwise the standard recv/forward loop proceeds. Adds startTLS() (protected helper for subclasses), stats(), shutdown(), and a connection counter wrapped in finally.
  • TCP.php: backend-dialing logic extracted into a new public connect() method so handlers that resolve routing themselves can reuse the same connection-cache path as getConnection().
  • TLS.php / Connection.php: pendingTls, PG_SSL_REQUEST, PG_SSL_RESPONSE_*, MYSQL_CLIENT_SSL_FLAG, and the two detection helpers are removed; their protocol-specific tests are replaced by config/reflection tests and a source-string assertion in TLSTest.php.

Confidence Score: 5/5

Safe to merge; the architectural shift is clean and consistent across both server implementations.

The core forwarding and TLS-upgrade paths are correctly refactored. The removed SSLRequest handling in the event-driven server was unused in practice (Swoole's SWOOLE_SSL mode does TLS immediately on accept, before any PostgreSQL wire bytes arrive). The only open items are documentation and test-quality nits that do not affect runtime correctness.

tests/TLSTest.php relies on source-code string matching rather than runtime assertions; src/Server/TCP/Config.php and src/Server/TCP/Swoole/Coroutine.php would benefit from a docblock describing the connectionHandler return-value contract and the accessibility of startTLS().

Important Files Changed

Filename Overview
src/Server/TCP/Swoole/Coroutine.php Refactored to always use a plaintext listener; PostgreSQL SSLRequest handling removed in favour of a generic connectionHandler hook. Adds connection counter, GC timer tracking, stats(), startTLS() helper (protected, not callable from handler closures), and shutdown().
src/Server/TCP/Config.php Adds connectionHandler closure property with no docblock describing the required return-value contract.
src/Adapter/TCP.php Extracts backend-dialing logic from getConnection() into a new public connect() method so protocol-aware handlers can route first then reuse the same connection-cache path.
src/Server/TCP/TLS.php Removes PostgreSQL/MySQL protocol constants and detection helpers that are now the caller's responsibility.
src/Server/TCP/TLSContext.php Adds toSwooleProtocolConfig() for mid-connection TLS upgrade via Socket::setProtocol() + Socket::sslHandshake(); cleanly separates immediate-accept TLS from negotiate-then-upgrade TLS.
src/Server/TCP/Swoole.php Removes PostgreSQL STARTTLS interception from onReceive; adds a stats() helper wrapping Server::stats() with safe key normalisation.
src/Server/TCP/Connection.php Drops the pendingTls flag that is no longer needed after removing inline SSLRequest handling.
tests/TLSTest.php Replaces protocol-detection unit tests with source-code string assertions and lightweight config/reflection tests; one new test inspects raw PHP source bytes rather than runtime behaviour.
tests/TLSContextTest.php Adds a focused unit test for toSwooleProtocolConfig() verifying open_ssl is set alongside cert/key paths.
tests/ConnectionStructTest.php Removes pendingTls assertions to match the dropped field in Connection.

Reviews (5): Last reviewed commit: "fix: move tcp protocol handling behind h..." | Re-trigger Greptile

Comment thread tests/TLSTest.php Outdated
@abnegate abnegate force-pushed the codex/postgres-sslrequest-reject branch from aef9d5d to 4fbf604 Compare June 6, 2026 22:58
Comment thread src/Server/TCP/Swoole/Coroutine.php Outdated
@abnegate abnegate changed the title Fix PostgreSQL SSLRequest fallback without TLS Add generic TCP connection handler hook Jun 7, 2026
@abnegate abnegate merged commit 3c94ed7 into main Jun 7, 2026
6 checks passed
@abnegate abnegate deleted the codex/postgres-sslrequest-reject branch June 7, 2026 01:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants