From 9f2e00fb9970dbcfcabee268c0a62ea5fb2153ae Mon Sep 17 00:00:00 2001 From: James M Snell Date: Wed, 17 Jun 2020 14:22:14 -0700 Subject: [PATCH] quic: temporarily disable preferred address tests The way the preferred address mechanism is supposed to work is this: A server might be exposed via multiple network interfaces / addresses. The preferred address is the one that clients should use. If a client uses one of the non-preferred addresses to initially connect to the server, the server will include the preferred address in it's initial transport params back to the client over the original connection. The client then can make a choice: it can either choose to ignore the advertised preferred address and continue using the original, or it can transition the in-flight connection to the preferred address without having to restart the connection. In the latter case, the connection will start making use of the preferred address but it might not do so immediately. To test this mechanism properly, we should have our server configured on multiple endpoints with one of those marked as the preferred. The connection should start on one and preceed uninterrupted to completion. If the preferred address policy is "accept", the client will accept and transition to the servers preferred address transparently, without interupting the flow. The current test is deficient because the server is only listening on a single address which is also advertised as the preferred. While the client should get the advertisement, we're not actually making use of the preferred address advertisement and nothing on the connection changes The ipv6 version of the test is also hanging on some platforms and it's not entirely clear why yet. PR-URL: https://github.com/nodejs/node/pull/33934 Reviewed-By: Daniel Bevenius Reviewed-By: Anna Henningsen --- .../test-quic-preferred-address-ipv6.js | 42 ++++++++++--- .../sequential/test-quic-preferred-address.js | 59 +++++++++++++++---- 2 files changed, 81 insertions(+), 20 deletions(-) diff --git a/test/sequential/test-quic-preferred-address-ipv6.js b/test/sequential/test-quic-preferred-address-ipv6.js index 07623a81dfb575..608fe62fa8ecec 100644 --- a/test/sequential/test-quic-preferred-address-ipv6.js +++ b/test/sequential/test-quic-preferred-address-ipv6.js @@ -8,7 +8,38 @@ if (!common.hasQuic) if (!common.hasIPv6) common.skip('missing ipv6'); -const Countdown = require('../common/countdown'); +// TODO(@jasnell): Temporarily disabling the preferred address +// tests because we need to rethink through how exactly this +// mechanism should work and how we should test it. +// +// The way the preferred address mechanism is supposed to work +// is this: A server might be exposed via multiple network +// interfaces / addresses. The preferred address is the one that +// clients should use. If a client uses one of the non-preferred +// addresses to initially connect to the server, the server will +// include the preferred address in it's initial transport params +// back to the client over the original connection. The client +// then can make a choice: it can either choose to ignore the +// advertised preferred address and continue using the original, +// or it can transition the in-flight connection to the preferred +// address without having to restart the connection. In the latter +// case, the connection will start making use of the preferred +// address but it might not do so immediately. +// +// To test this mechanism properly, we should have our server +// configured on multiple endpoints with one of those marked +// as the preferred. The connection should start on one and preceed +// uninterrupted to completion. If the preferred address policy +// is "accept", the client will accept and transition to the servers +// preferred address transparently, without interupting the flow. +// +// The current test is deficient because the server is only listening +// on a single address which is also advertised as the preferred. +// While the client should get the advertisement, we're not actually +// making use of the preferred address advertisement and nothing on +// the connection changes. +common.skip('preferred address test currently disabled'); + const assert = require('assert'); const fixtures = require('../common/fixtures'); const key = fixtures.readKey('agent1-key.pem', 'binary'); @@ -25,12 +56,6 @@ const server = createQuicSocket({ endpoint: { type: 'udp6' } }); const kALPN = 'zzz'; // ALPN can be overriden to whatever we want -const countdown = new Countdown(1, () => { - debug('Countdown expired. Destroying sockets'); - server.close(); - client.close(); -}); - server.listen({ key, cert, ca, alpn: kALPN, preferredAddress: { port: common.PORT, address: '::', @@ -87,7 +112,8 @@ server.on('ready', common.mustCall(() => { stream.resume(); stream.on('close', common.mustCall(() => { - countdown.dec(); + server.close(); + client.close(); })); })); diff --git a/test/sequential/test-quic-preferred-address.js b/test/sequential/test-quic-preferred-address.js index a75aa6284c304c..b2537c636b741a 100644 --- a/test/sequential/test-quic-preferred-address.js +++ b/test/sequential/test-quic-preferred-address.js @@ -5,7 +5,38 @@ const common = require('../common'); if (!common.hasQuic) common.skip('missing quic'); -const Countdown = require('../common/countdown'); +// TODO(@jasnell): Temporarily disabling the preferred address +// tests because we need to rethink through how exactly this +// mechanism should work and how we should test it. +// +// The way the preferred address mechanism is supposed to work +// is this: A server might be exposed via multiple network +// interfaces / addresses. The preferred address is the one that +// clients should use. If a client uses one of the non-preferred +// addresses to initially connect to the server, the server will +// include the preferred address in it's initial transport params +// back to the client over the original connection. The client +// then can make a choice: it can either choose to ignore the +// advertised preferred address and continue using the original, +// or it can transition the in-flight connection to the preferred +// address without having to restart the connection. In the latter +// case, the connection will start making use of the preferred +// address but it might not do so immediately. +// +// To test this mechanism properly, we should have our server +// configured on multiple endpoints with one of those marked +// as the preferred. The connection should start on one and preceed +// uninterrupted to completion. If the preferred address policy +// is "accept", the client will accept and transition to the servers +// preferred address transparently, without interupting the flow. +// +// The current test is deficient because the server is only listening +// on a single address which is also advertised as the preferred. +// While the client should get the advertisement, we're not actually +// making use of the preferred address advertisement and nothing on +// the connection changes. +common.skip('preferred address test currently disabled'); + const assert = require('assert'); const fixtures = require('../common/fixtures'); const key = fixtures.readKey('agent1-key.pem', 'binary'); @@ -22,17 +53,19 @@ const server = createQuicSocket(); const kALPN = 'zzz'; // ALPN can be overriden to whatever we want -const countdown = new Countdown(1, () => { - debug('Countdown expired. Destroying sockets'); - server.close(); - client.close(); -}); - -server.listen({ key, cert, ca, alpn: kALPN, preferredAddress: { +server.listen({ port: common.PORT, address: '0.0.0.0', - type: 'udp4', -} }); + key, + cert, + ca, + alpn: kALPN, + preferredAddress: { + port: common.PORT, + address: '0.0.0.0', + type: 'udp4', + } +}); server.on('session', common.mustCall((session) => { debug('QuicServerSession Created'); @@ -59,7 +92,7 @@ server.on('ready', common.mustCall(() => { cert, ca, alpn: kALPN, - preferredAddressPolicy: 'accept' } }); + } }); client.on('close', common.mustCall()); @@ -67,6 +100,7 @@ server.on('ready', common.mustCall(() => { address: 'localhost', port: endpoint.address.port, servername: 'localhost', + preferredAddressPolicy: 'accept', }); req.on('ready', common.mustCall(() => { @@ -83,7 +117,8 @@ server.on('ready', common.mustCall(() => { stream.resume(); stream.on('close', common.mustCall(() => { - countdown.dec(); + server.close(); + client.close(); })); }));