Skip to content
This repository has been archived by the owner on Aug 11, 2020. It is now read-only.

quic: client should work when receives invalid preferred address #155

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions src/node_quic_session.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2800,7 +2800,11 @@ bool QuicClientSession::SendConnectionClose() {
sendbuf_.Cancel();
QuicError error = GetLastError();

if (!WritePackets("client connection close - write packets"))
// If we're not already in the closing period,
// first attempt to write any pending packets, then
// start the closing period.
if (!IsInClosingPeriod() &&
!WritePackets("client connection close - write packets"))
return false;

ssize_t nwrite =
Expand Down Expand Up @@ -3222,8 +3226,8 @@ int QuicSession::OnSelectPreferredAddress(
void* user_data) {
QuicSession* session = static_cast<QuicSession*>(user_data);
QuicSession::Ngtcp2CallbackScope callback_scope(session);
return session->SelectPreferredAddress(dest, paddr) ?
0 : NGTCP2_ERR_CALLBACK_FAILURE;
session->SelectPreferredAddress(dest, paddr);
return 0;
}

// Called by ngtcp2 when a stream has been closed for any reason.
Expand Down
2 changes: 1 addition & 1 deletion src/node_quic_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ class SocketAddress {
}

char host[NI_MAXHOST];
if (uv_inet_ntop(af, binaddr, host, sizeof(host)) == 0)
if (uv_inet_ntop(af, binaddr, host, sizeof(host)) != 0)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This seems to be missed from the last rebasing. Refs: https://github.com/nodejs/quic/pull/139/files

Copy link
Member

Choose a reason for hiding this comment

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

how odd... thanks for catching

return false;

addrinfo hints{};
Expand Down
66 changes: 66 additions & 0 deletions test/parallel/test-quic-client-empty-preferred-address.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
'use strict';

// This test ensures that when we don't define `preferredAddress`
// on the server while the `preferredAddressPolicy` on the client
// is `accpet`, it works as expected.

const common = require('../common');
if (!common.hasQuic)
common.skip('missing quic');

const assert = require('assert');
const fixtures = require('../common/fixtures');
const key = fixtures.readKey('agent1-key.pem', 'binary');
const cert = fixtures.readKey('agent1-cert.pem', 'binary');
const ca = fixtures.readKey('ca1-cert.pem', 'binary');
const { createSocket } = require('quic');

const kALPN = 'zzz';
const kServerName = 'agent2';
const server = createSocket({ port: 0, validateAddress: true });

let client;
server.listen({
key,
cert,
ca,
alpn: kALPN,
});

server.on('session', common.mustCall((serverSession) => {
serverSession.on('stream', common.mustCall((stream) => {
stream.on('data', common.mustCall((data) => {
assert.strictEqual(data.toString('utf8'), 'hello');
}));
stream.on('end', common.mustCall(() => {
stream.close();
client.close();
server.close();
}));
}));
}));

server.on('ready', common.mustCall(() => {
client = createSocket({
port: 0,
});

const clientSession = client.connect({
address: 'localhost',
key,
cert,
ca,
alpn: kALPN,
port: server.address.port,
servername: kServerName,
preferredAddressPolicy: 'accept',
});

clientSession.on('close', common.mustCall());

clientSession.on('secure', common.mustCall(() => {
const stream = clientSession.openStream();
stream.end('hello');
stream.on('close', common.mustCall());
}));
}));