Add TLS support (upgrade-after-INFO) by ChrisFrizza6969 · Pull Request #57 · thesis-php/nats

ChrisFrizza6969 · 2026-07-02T18:18:27Z

Add TLS support (upgrade-after-INFO)

Summary

Adds client TLS so the driver can connect to TLS-required servers such as
Synadia NGS (tls://connect.ngs.global). Currently
the client only speaks plaintext — it always sends CONNECT with
tls_required: false and never performs a TLS handshake — so any server that
advertises tls_required: true resets the connection.

Motivation

NGS (and any NATS server with tls: {...}) sends a plaintext INFO advertising
tls_required: true, then expects the client to run the TLS handshake before
CONNECT. Without TLS there is no way to use this driver against those servers.

What changed

Config: new optional ?Amp\Socket\ClientTlsContext $tls (default null
= existing plaintext behavior, fully backward compatible). fromURI enables
TLS from a tls:// / nats+tls:// / ssl:// scheme, defaulting the peer name
to the host for SNI/verification; fromArray accepts a tls key.
SocketConnectionFactory: attaches the TLS context to the ConnectContext
so the socket can be upgraded (it does not connect TLS-first).
Framer: when TLS is configured, the single reader fiber reads the
plaintext INFO line, runs setupTls(), and then pushes INFO to the
parser. Doing the upgrade inside the sole socket-reader fiber preserves the
single-reader invariant (amphp forbids concurrent reads on a socket); pushing
INFO only after the handshake completes prevents startup() (a different
fiber) from writing CONNECT mid-handshake and corrupting the stream.
SocketConnection::startup() is unchanged (no TLS logic).

Compatibility

The non-TLS code path is byte-for-byte unchanged: when $tls is null,
$upgradeTls is false and the Framer behaves exactly as before. Existing
tests are unaffected.

Scope / limitation

Implements the standard NATS upgrade-after-INFO flow. Servers configured with
tls { handshake_first: true } (TLS handshake before INFO) are not covered by
this PR.

Tests

Added ConfigTest cases: tls:// / nats+tls:// / ssl:// produce a
ClientTlsContext with the host as peer name; a non-TLS scheme leaves tls
null; fromArray accepts a tls key.
Verified locally: php-cs-fixer (no diff), phpunit ConfigTest green,
phpstan level max / strict clean on the changed files.
Manually verified end-to-end against real NGS: plaintext INFO →
setupTls() → CONNECT → PONG, plus JetStream object store list/get/put
over the resulting TLS connection.

Adds client TLS so the driver can connect to TLS-required servers such as Synadia NGS (tls://connect.ngs.global). Previously the client only spoke plaintext (CONNECT always sent tls_required:false) and any TLS-required server reset the connection. Design: - Config: new optional `?ClientTlsContext $tls` (null = current plaintext behavior, fully backward compatible). `fromURI` enables TLS from a tls:// / nats+tls:// / ssl:// scheme (peer name defaults to the host for SNI/verification); `fromArray` accepts a `tls` key. - SocketConnectionFactory: attaches the TLS context to the ConnectContext so the socket can be upgraded (does NOT connect TLS-first). - Framer: when TLS is configured, the single reader fiber reads the plaintext INFO line, runs setupTls(), THEN pushes INFO to the parser. Doing the upgrade inside the sole socket-reader fiber preserves the single-reader invariant (amphp forbids concurrent reads); pushing INFO only after the handshake completes prevents startup() from writing CONNECT mid-handshake. - SocketConnection::startup() is unchanged (no TLS logic). Scope/limitation: implements the standard NATS upgrade-after-INFO flow (server sends plaintext INFO advertising tls_required, client then upgrades). Servers configured with `tls { handshake_first: true }` are not covered. Non-TLS code path is byte-for-byte unchanged; existing tests unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

- fromURI tls://, nats+tls://, ssl:// -> ClientTlsContext with host as peer name - non-tls scheme leaves tls null (backward compat) - fromArray accepts a tls key Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

kafkiansky · 2026-07-28T17:46:04Z

+        $scheme = strtolower($components['scheme'] ?? '');
+        if (($scheme === 'tls' || $scheme === 'nats+tls' || $scheme === 'ssl') && ($components['host'] ?? '') !== '') {
+            $firstHost = explode(':', explode(',', $components['host'])[0])[0];
+            $tls = (new \Amp\Socket\ClientTlsContext($firstHost));


Suggested change

$tls = (new \Amp\Socket\ClientTlsContext($firstHost));

$tls = new ClientTlsContext($firstHost);

Done imported it. Also caught a third FQCN in the fromArray docblock.

kafkiansky · 2026-07-28T17:46:29Z

+        // set, the connection upgrades to TLS after the server's plaintext INFO
+        // (the standard NATS tls_required flow, e.g. Synadia NGS). The upgrade is
+        // performed by the socket's single reader fiber — see Framer.
+        public ?\Amp\Socket\ClientTlsContext $tls = null,


Suggested change

public ?\Amp\Socket\ClientTlsContext $tls = null,

public ?ClientTlsContext $tls = null,

kafkiansky · 2026-07-28T17:48:04Z

+        // SBC firmware fetcher) can't read an object whose digest we wrote
+        // unpadded. base64encode() strips padding (correct for subject/name
+        // segments), so pad the digest explicitly here.
+        $encodedDigest = strtr(base64_encode($digest->finish()), '+/', '-_');


These changes should be made in another PR.

kafkiansky · 2026-07-28T17:48:57Z

     */
    public function startup(): void
    {
+        // The TLS upgrade (when configured) happens inside the Framer's reader


Unnecessary AI comment, imho.

- import ClientTlsContext instead of using the FQCN inline - drop the explanatory comment in SocketConnection::startup() Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Co-authored-by: Vadim Zanfir <vadimzanfir@gmail.com>

The accepted suggestion was authored against the pre-import file and added a second identical use statement, which is a fatal error. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

kafkiansky · 2026-07-30T05:53:47Z

+                // below reads the encrypted stream exactly as normal.
+                if ($upgradeTls) {
+                    $preamble = '';
+                    while (!str_contains($preamble, "\r\n")) {


The read loop stops as soon as $preamble contains \r\n, but nothing guarantees a single read() ends exactly at the delimiter: TCP can hand us more than the INFO line. If any bytes land after the first \r\n, we push them to the parser as plaintext (which then tries to parse post-upgrade bytes as a NATS frame) and setupTls() never sees them. Should we guard this explicitly (fail if there's anything after the first \r\n) rather than rely on it silently? Or is there a reason the tail can't happen that I'm missing?

kafkiansky · 2026-07-30T05:55:39Z

@ChrisFrizza6969, run make fixer locally, please.

cgf-afk and others added 3 commits July 2, 2026 13:49

docs(tls): document TLS connections in README

a24cbe2

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

kafkiansky reviewed Jul 28, 2026

View reviewed changes

refactor: address review feedback

67cf895

- import ClientTlsContext instead of using the FQCN inline - drop the explanatory comment in SocketConnection::startup() Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

ChrisFrizza6969 force-pushed the feat/tls-support branch from 23b85d6 to 67cf895 Compare July 28, 2026 19:22

ChrisFrizza6969 and others added 2 commits July 28, 2026 15:24

Update src/Config.php

6963f33

Co-authored-by: Vadim Zanfir <vadimzanfir@gmail.com>

fix: remove duplicated ClientTlsContext import

62c582b

The accepted suggestion was authored against the pre-import file and added a second identical use statement, which is a fatal error. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

kafkiansky reviewed Jul 30, 2026

View reviewed changes

Navigation Menu

Uh oh!

Uh oh!

Add TLS support (upgrade-after-INFO) - #57

Add TLS support (upgrade-after-INFO)#57
ChrisFrizza6969 wants to merge 6 commits into
thesis-php:0.4.xfrom
ChrisFrizza6969:feat/tls-support

ChrisFrizza6969 commented Jul 2, 2026

Uh oh!

kafkiansky Jul 28, 2026

Uh oh!

ChrisFrizza6969 Jul 28, 2026

Uh oh!

kafkiansky Jul 28, 2026

Uh oh!

ChrisFrizza6969 Jul 28, 2026

Uh oh!

Uh oh!

kafkiansky Jul 28, 2026

Uh oh!

kafkiansky Jul 28, 2026

Uh oh!

kafkiansky Jul 30, 2026

Uh oh!

kafkiansky commented Jul 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	$tls = (new \Amp\Socket\ClientTlsContext($firstHost));
	$tls = new ClientTlsContext($firstHost);

	public ?\Amp\Socket\ClientTlsContext $tls = null,
	public ?ClientTlsContext $tls = null,

Uh oh!

Uh oh!

Conversation

ChrisFrizza6969 commented Jul 2, 2026

Add TLS support (upgrade-after-INFO)

Summary

Motivation

What changed

Compatibility

Scope / limitation

Tests

Uh oh!

kafkiansky Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

ChrisFrizza6969 Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

kafkiansky Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

ChrisFrizza6969 Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kafkiansky Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

kafkiansky Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

kafkiansky Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

kafkiansky commented Jul 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants