Repository navigation
ALPNProtocols option of TLSSocket constructor is broken. #16643
Description
- Version: v8.8.1
- Platform: Darwin sprite.local 17.0.0 Darwin Kernel Version 17.0.0: Thu Aug 24 21:48:19 PDT 2017; root:xnu-4570.1.46~2/RELEASE_X86_64 x86_64
- Subsystem: _tls_wrap.js
Documentation for the ALPNProtocols option (found here) indicates that an array of strings should be acceptable:
\\ Broken
var socket = new tls.TLSSocket(null, { ALPNProtocols: ['http/1.1'] });However, the example above produces this error:
TypeError: Must give a Buffer as first argument
at TLSSocket._init (_tls_wrap.js:516:9)
at new TLSSocket (_tls_wrap.js:304:8)
at repl:1:14
at ContextifyScript.Script.runInThisContext (vm.js:50:33)
at REPLServer.defaultEval (repl.js:240:29)
at bound (domain.js:301:14)
at REPLServer.runBound [as eval] (domain.js:314:12)
at REPLServer.onLine (repl.js:441:10)
at emitOne (events.js:120:20)
at REPLServer.emit (events.js:210:7)
From which I inferred that I had to use a buffer:
\\ Working
var buffer = Buffer.from([8, ...Buffer.from('http/1.1')]);
var socket = new tls.TLSSocket(null, { ALPNProtocols: buffer });If this is indeed a documentation bug, I'd be happy to open a pull request. I'll take a shot at a code bug if the issue is determined to be there instead.
Activity
bnoordhuis commentedon Oct 31, 2017
It sounds like the constructor should call
tls.convertALPNProtocols()andtls.convertNPNProtocols()instead of lettingtls.connect()andtls.createServer()do that before passing it on tonew TLSSocket().qubyte commentedon Oct 31, 2017
I wasn't aware of these methods, thanks for the tip. I'll probably have to apply them within the
TLSSocketconstructor though. Do you mean these should be applied before calling theSocketconstructor?addaleax commentedon Oct 31, 2017
@qubyte I think it’s more like, the
TLSSocketconstructor should call them, not userland code.But for now calling them yourself should be fine as a workaround.
qubyte commentedon Oct 31, 2017
@addaleax Hmm. I think I wasn't making myself clear. I mean to call these within the
TLSSocketconstructor, as you say. I think we're on the same page though. Do you mind if I assign myself? It'll be a nice way to ease myself into the codebase.addaleax commentedon Oct 31, 2017
@qubyte Yes, I think we’re on the same page. Go for it if you like!
qubyte commentedon Oct 31, 2017
Thanks! Looks like I can't self-assign though. Would you mind doing that for me please?
tls: accept array of protocols in TLSSocket
apapirovski commentedon Nov 4, 2017
Fixed by 291ff72