From 63545c5d185c4d88f111eb84194d38f12b493b39 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Wed, 5 Jun 2019 15:39:56 -0700 Subject: [PATCH] quic: linting fixups PR-URL: https://github.com/nodejs/quic/pull/31 --- doc/api/errors.md | 5 ++++ lib/internal/errors.js | 38 ++++++++++++------------ lib/internal/quic/core.js | 28 +++++++++-------- lib/internal/quic/util.js | 3 +- src/node_quic_crypto.h | 6 ++-- src/node_quic_session.cc | 10 +++---- src/node_quic_socket.cc | 10 ++++--- src/node_quic_stream.cc | 2 +- test/parallel/test-quic-client-server.js | 6 +++- 9 files changed, 60 insertions(+), 48 deletions(-) diff --git a/doc/api/errors.md b/doc/api/errors.md index 0ad961d3b2..830f9b734b 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -1599,6 +1599,11 @@ compiled with ICU support. A given value is out of the accepted range. + +### ERR_QUIC_ERROR + +TBD + ### ERR_QUICCLIENTSESSION_FAILED diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 2373163efe..b556da34c6 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1104,6 +1104,25 @@ E('ERR_OUT_OF_RANGE', msg += ` It must be ${range}. Received ${received}`; return msg; }, RangeError); +E('ERR_QUICCLIENTSESSION_FAILED', + 'Failed to create a new QuicClientSession: %s', Error); +E('ERR_QUICCLIENTSESSION_FAILED_SETSOCKET', + 'Failed to set the QuicSocket: %d', Error); +E('ERR_QUICSESSION_DESTROYED', + 'Cannot call %s after a QuicSession has been destroyed', Error); +E('ERR_QUICSESSION_INVALID_DCID', 'Invalid DCID value: %s', Error); +E('ERR_QUICSESSION_UNABLE_TO_MIGRATE', + 'The QuicClientSession is currently unable to be migrated to a ' + + 'different QuicSocket', + Error); +E('ERR_QUICSOCKET_CLOSING', + 'Cannot call %s while a QuicSocket is closing', Error); +E('ERR_QUICSOCKET_DESTROYED', + 'Cannot call %s after a QuicSocket has been destroyed', Error); +E('ERR_QUICSOCKET_LISTENING', + 'This QuicSocket is already listening', Error); +E('ERR_QUICSOCKET_UNBOUND', + 'Cannot call %s before a QuicSocket has been bound', Error); E('ERR_QUIC_ERROR', function(code, family) { const { constants: { @@ -1126,25 +1145,6 @@ E('ERR_QUIC_ERROR', function(code, family) { } return `QUIC session closed with ${familyType} error code ${code}`; }, Error); -E('ERR_QUICCLIENTSESSION_FAILED', - 'Failed to create a new QuicClientSession: %s', Error); -E('ERR_QUICCLIENTSESSION_FAILED_SETSOCKET', - 'Failed to set the QuicSocket: %d', Error); -E('ERR_QUICSESSION_DESTROYED', - 'Cannot call %s after a QuicSession has been destroyed', Error); -E('ERR_QUICSESSION_INVALID_DCID', 'Invalid DCID value: %s', Error); -E('ERR_QUICSESSION_UNABLE_TO_MIGRATE', - 'The QuicClientSession is currently unable to be migrated to a ' + - 'different QuicSocket', - Error); -E('ERR_QUICSOCKET_CLOSING', - 'Cannot call %s while a QuicSocket is closing', Error); -E('ERR_QUICSOCKET_DESTROYED', - 'Cannot call %s after a QuicSocket has been destroyed', Error); -E('ERR_QUICSOCKET_LISTENING', - 'This QuicSocket is already listening', Error); -E('ERR_QUICSOCKET_UNBOUND', - 'Cannot call %s before a QuicSocket has been bound', Error); E('ERR_QUIC_TLS13_REQUIRED', 'QUIC requires TLS version 1.3', Error); E('ERR_REQUIRE_ESM', 'Must use import to load ES Module: %s', Error); E('ERR_SCRIPT_EXECUTION_INTERRUPTED', diff --git a/lib/internal/quic/core.js b/lib/internal/quic/core.js index 0abf6a62d0..b57a1cdd8a 100644 --- a/lib/internal/quic/core.js +++ b/lib/internal/quic/core.js @@ -10,6 +10,7 @@ const { assertCrypto(); const { Error } = primordials; +const { Buffer } = require('buffer'); const { isArrayBufferView } = require('internal/util/types'); const { getAllowUnauthorized, @@ -61,6 +62,7 @@ const { ERR_INVALID_ARG_TYPE, ERR_INVALID_ARG_VALUE, ERR_INVALID_CALLBACK, + ERR_QUIC_ERROR, ERR_QUICSESSION_DESTROYED, ERR_QUICSOCKET_CLOSING, ERR_QUICSOCKET_DESTROYED, @@ -526,8 +528,8 @@ function onRemoveListener(event) { this[kHandle].state[IDX_QUIC_SESSION_STATE_CLIENT_HELLO_ENABLED] = 0; break; case 'OCSPRequest': - this[kHandle].state[IDX_QUIC_SESSION_STATE_CERT_ENABLED] = 0; - break; + this[kHandle].state[IDX_QUIC_SESSION_STATE_CERT_ENABLED] = 0; + break; } } @@ -1147,7 +1149,7 @@ class QuicSession extends EventEmitter { if (typeof error === 'number' || (error != null && - typeof error === 'Object' && + typeof error === 'object' && !(error instanceof Error))) { const { closeCode, @@ -1276,7 +1278,7 @@ class QuicServerSession extends QuicSession { } [kClientHello](alpn, servername, ciphers, callback) { - this.emit( + this.emit( 'clientHello', alpn, servername, @@ -1374,8 +1376,10 @@ class QuicClientSession extends QuicSession { this.#preferredAddressPolicy = preferredAddressPolicy; this.#remoteTransportParams = remoteTransportParams; this.#requestOCSP = requestOCSP; - this.#secureContext = createSecureContext(sc_options, - initSecureContextClient); + this.#secureContext = + createSecureContext( + sc_options, + initSecureContextClient); this.#sessionTicket = sessionTicket; this.#transportParams = validateTransportParams( @@ -1553,14 +1557,12 @@ class QuicStream extends Duplex { // Close the writable side this.end(); } + } else if (this.serverInitiated) { + // Close the writable side + this.end(); } else { - if (this.serverInitiated) { - // Close the writable side - this.end(); - } else { - this.push(null); - this.read(); - } + this.push(null); + this.read(); } } } diff --git a/lib/internal/quic/util.js b/lib/internal/quic/util.js index 95e676c050..32b1da072c 100644 --- a/lib/internal/quic/util.js +++ b/lib/internal/quic/util.js @@ -32,6 +32,7 @@ const { IDX_QUIC_SESSION_MAX_PACKET_SIZE_DEFAULT, MAX_RETRYTOKEN_EXPIRATION, MIN_RETRYTOKEN_EXPIRATION, + NGTCP2_NO_ERROR, NGTCP2_DEFAULT_MAX_ACK_DELAY, NGTCP2_MAX_CIDLEN, NGTCP2_MIN_CIDLEN, @@ -92,7 +93,7 @@ function validateCloseCode(code) { closeCode = code; closeFamily = QUIC_ERROR_APPLICATION; } else { - throw new ERR_INVALID_ARG_TYPE('code', ['number', 'Object'], code) + throw new ERR_INVALID_ARG_TYPE('code', ['number', 'Object'], code); } return { closeCode, diff --git a/src/node_quic_crypto.h b/src/node_quic_crypto.h index 62885af03d..0e91b4bb4f 100644 --- a/src/node_quic_crypto.h +++ b/src/node_quic_crypto.h @@ -896,7 +896,7 @@ inline void MessageCB( } } -inline std::string ToHex(const uint8_t *s, size_t len) { +inline std::string ToHex(const uint8_t* s, size_t len) { static constexpr char LOWER_XDIGITS[] = "0123456789abcdef"; std::string res; res.resize(len * 2); @@ -909,9 +909,9 @@ inline std::string ToHex(const uint8_t *s, size_t len) { } inline void LogSecret( - SSL *ssl, + SSL* ssl, int name, - const unsigned char *secret, + const unsigned char* secret, size_t secretlen) { if (auto keylog_cb = SSL_CTX_get_keylog_callback(SSL_get_SSL_CTX(ssl))) { unsigned char crandom[32]; diff --git a/src/node_quic_session.cc b/src/node_quic_session.cc index bf4d25439d..eb5eb9d190 100644 --- a/src/node_quic_session.cc +++ b/src/node_quic_session.cc @@ -29,7 +29,6 @@ using crypto::SecureContext; using v8::Array; using v8::ArrayBufferView; -using v8::Boolean; using v8::Context; using v8::Float64Array; using v8::Function; @@ -1822,9 +1821,9 @@ int QuicSession::TLSHandshake() { CHECK(!IsDestroyed()); Debug(this, "TLS handshake %s", initial_ ? "starting" : "continuing"); - if (initial_) + if (initial_) { session_stats_.handshake_start_at = uv_hrtime(); - else { + } else { // TODO(@jasnell): Check handshake_continue_at to guard against slow // handshake attack } @@ -2091,7 +2090,6 @@ int QuicServerSession::OnClientHello() { arraysize(argv), argv); OPENSSL_free(exts); - //return 0; return client_hello_cb_running_ ? -1 : 0; } @@ -3036,9 +3034,9 @@ int QuicClientSession::OnTLSStatus() { int len = SSL_get_tlsext_status_ocsp_resp(ssl(), &resp); Debug(this, "An OCSP Response of %d bytes has been received.", len); Local arg; - if (resp == nullptr) + if (resp == nullptr) { arg = Undefined(env()->isolate()); - else { + } else { arg = Buffer::Copy(env(), reinterpret_cast(resp), len) .ToLocalChecked(); } diff --git a/src/node_quic_socket.cc b/src/node_quic_socket.cc index e332e307da..dd6810c545 100644 --- a/src/node_quic_socket.cc +++ b/src/node_quic_socket.cc @@ -802,10 +802,12 @@ void QuicSocketListen(const FunctionCallbackInfo& args) { alpn += *val; } - bool reject_unauthorized = args[5]->IsTrue(); - bool request_cert = args[6]->IsTrue(); - - socket->Listen(sc, preferred_address, alpn, reject_unauthorized, request_cert); + socket->Listen( + sc, + preferred_address, + alpn, + args[5]->IsTrue(), // reject_unauthorized + args[6]->IsTrue()); // request_cert } void QuicSocketReceiveStart(const FunctionCallbackInfo& args) { diff --git a/src/node_quic_stream.cc b/src/node_quic_stream.cc index 89c9451597..6c92433be3 100644 --- a/src/node_quic_stream.cc +++ b/src/node_quic_stream.cc @@ -330,7 +330,7 @@ void QuicStream::ReceiveData(int fin, const uint8_t* data, size_t datalen) { inbound_consumed_data_while_paused_ += avail; else session_->ExtendStreamOffset(this, avail); - }; + } // When fin != 0, we've received that last chunk of data for this // stream, indicating that the stream is no longer readable. diff --git a/test/parallel/test-quic-client-server.js b/test/parallel/test-quic-client-server.js index 93fe1a4941..4a19b692a1 100644 --- a/test/parallel/test-quic-client-server.js +++ b/test/parallel/test-quic-client-server.js @@ -88,9 +88,13 @@ server.on('session', common.mustCall((session) => { // TODO(@jasnell): Using setImmediate here causes the test // to fail, but it shouldn't. Investigate why. process.nextTick(() => { + // The first argument is a potential error + // The second is an optional new SecureContext + // The third is the ocsp response. + // All arguments are optional cb(null, null, Buffer.from('hello')); }); - })); + })); session.on('keylog', common.mustCall((line) => { assert(kKeylogs.shift().test(line));