diff options
| -rw-r--r-- | lib/internal/quic/core.js | 8 | ||||
| -rw-r--r-- | src/quic/node_quic_session.cc | 44 | ||||
| -rw-r--r-- | test/parallel/test-quic-simple-client-migrate.js | 14 |
3 files changed, 41 insertions, 25 deletions
diff --git a/lib/internal/quic/core.js b/lib/internal/quic/core.js index d311852d950..93849d0dcd8 100644 --- a/lib/internal/quic/core.js +++ b/lib/internal/quic/core.js @@ -1803,8 +1803,10 @@ class QuicSession extends EventEmitter { }, depth, options); } - [kSetSocket](socket) { + [kSetSocket](socket, natRebinding = false) { this[kInternalState].socket = socket; + if (socket !== undefined) + this[kHandle].setSocket(socket[kHandle], natRebinding); } // Called at the completion of the TLS handshake for the local peer @@ -2432,7 +2434,7 @@ class QuicClientSession extends QuicSession { {}; } - async setSocket(socket) { + async setSocket(socket, natRebinding = false) { if (this.destroyed) throw new ERR_INVALID_STATE('QuicClientSession is already destroyed'); if (this.closing) @@ -2460,7 +2462,7 @@ class QuicClientSession extends QuicSession { this[kSetSocket](undefined); } socket[kAddSession](this); - this[kSetSocket](socket); + this[kSetSocket](socket, natRebinding); } } diff --git a/src/quic/node_quic_session.cc b/src/quic/node_quic_session.cc index 999dbd71fda..e75d33e636e 100644 --- a/src/quic/node_quic_session.cc +++ b/src/quic/node_quic_session.cc @@ -2479,7 +2479,6 @@ int QuicSession::set_session(SSL_SESSION* session) { } // A client QuicSession can be migrated to a different QuicSocket instance. -// TODO(@jasnell): This will be revisited. bool QuicSession::set_socket(QuicSocket* socket, bool nat_rebinding) { CHECK(!is_server()); CHECK(!is_destroyed()); @@ -2492,8 +2491,6 @@ bool QuicSession::set_socket(QuicSocket* socket, bool nat_rebinding) { Debug(this, "Migrating to %s", socket->diagnostic_name()); - SendSessionScope send(this); - // Ensure that we maintain a reference to keep this from being // destroyed while we are starting the migration. BaseObjectPtr<QuicSession> ptr(this); @@ -2511,22 +2508,31 @@ bool QuicSession::set_socket(QuicSocket* socket, bool nat_rebinding) { // Step 4: Update ngtcp2 auto local_address = socket->local_address(); - if (nat_rebinding) { - ngtcp2_addr addr; - ngtcp2_addr_init( - &addr, - local_address.data(), - local_address.length(), - nullptr); - ngtcp2_conn_set_local_addr(connection(), &addr); - } else { + + // The nat_rebinding option here should rarely, if ever + // be used in a real application. It is intended to serve + // as a way of simulating a silent local address change, + // such as when the NAT binding changes. Currently, Node.js + // does not really have an effective way of detecting that. + // Manual user code intervention to handle the migration + // to the new QuicSocket is required, which should always + // trigger path validation using the ngtcp2_conn_initiate_migration. + if (LIKELY(!nat_rebinding)) { + SendSessionScope send(this); QuicPath path(local_address, remote_address_); - if (ngtcp2_conn_initiate_migration( - connection(), - &path, - uv_hrtime()) != 0) { - return false; - } + return ngtcp2_conn_initiate_migration( + connection(), + &path, + uv_hrtime()) == 0; + } else { + ngtcp2_addr addr; + ngtcp2_conn_set_local_addr( + connection(), + ngtcp2_addr_init( + &addr, + local_address.data(), + local_address.length(), + nullptr)); } return true; @@ -3671,7 +3677,7 @@ void QuicSessionSetSocket(const FunctionCallbackInfo<Value>& args) { ASSIGN_OR_RETURN_UNWRAP(&session, args.Holder()); CHECK(args[0]->IsObject()); ASSIGN_OR_RETURN_UNWRAP(&socket, args[0].As<Object>()); - args.GetReturnValue().Set(session->set_socket(socket)); + args.GetReturnValue().Set(session->set_socket(socket, args[1]->IsTrue())); } // GracefulClose flips a flag that prevents new local streams diff --git a/test/parallel/test-quic-simple-client-migrate.js b/test/parallel/test-quic-simple-client-migrate.js index d40a693ba3b..af15456c855 100644 --- a/test/parallel/test-quic-simple-client-migrate.js +++ b/test/parallel/test-quic-simple-client-migrate.js @@ -5,10 +5,9 @@ const common = require('../common'); if (!common.hasQuic) common.skip('missing quic'); -common.skip('Not working correct yet... need to refactor'); - const assert = require('assert'); const { key, cert, ca } = require('../common/quic'); + const { once } = require('events'); const { createQuicSocket } = require('net'); const { pipeline } = require('stream'); @@ -58,14 +57,23 @@ const server = createQuicSocket({ server: options }); stream.on('end', common.mustCall(() => { assert.strictEqual(data, 'Hello from the client'); })); - stream.on('close', common.mustCall()); + stream.on('close', common.mustCall(() => { + req.close(); + })); // Send some data on one connection... stream.write('Hello '); // Wait just a bit, then migrate to a different // QuicSocket and continue sending. setTimeout(common.mustCall(async () => { + const s1 = req.socket; + const a1 = req.socket.endpoints[0].address; + await req.setSocket(client2); + + // Verify that it is using a different network endpoint + assert.notStrictEqual(s1, req.socket); + assert.notDeepStrictEqual(a1, req.socket.endpoints[0].address); client.close(); stream.end('from the client'); }), common.platformTimeout(100)); |