Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

tls: Provide default object for _tlsOptions when options are undefined #7218

Closed
wants to merge 1 commit into from

Conversation

adammw
Copy link

@adammw adammw commented Mar 1, 2014

Prevents this error when options is not set.

_tls_wrap.js:205
  var credentials = options.credentials || crypto.createCredentials();
                           ^
TypeError: Cannot read property 'credentials' of undefined
    at TLSSocket._init (_tls_wrap.js:205:28)
    at new TLSSocket (_tls_wrap.js:186:10)

@Nodejs-Jenkins
Copy link

Thank you for contributing this pull request! Here are a few pointers to make sure your submission will be considered for inclusion.

Commit adammw/node@e8d9bae has the following error(s):

  • Commit message must indicate the subsystem this commit changes

The following commiters were not found in the CLA:

  • Adam Malcontenti-Wilson

You can fix all these things without opening another issue.

Please see CONTRIBUTING.md for more information

@adammw
Copy link
Author

adammw commented Mar 1, 2014

I just realised that TLSSocket also requires isServer, ideally it should have a default (probably false).

@indutny
Copy link
Member

indutny commented Mar 1, 2014

Looks good! Could you please provide a test?

@adammw
Copy link
Author

adammw commented Mar 2, 2014

I could and would be happy to, however I'd feel more comfortable doing so if I knew more about TLSSocket - I'm having a lot of trouble getting it to work.

For example, my script I was trying to write with TLSSocket doesn't work at all, and I'm not sure if it's because I'm not using it correctly, I'm misunderstanding the purpose of TLSSocket, or some unknown bug.

@indutny
Copy link
Member

indutny commented Mar 2, 2014

Could you please paste what comes into upgradeHead once upgrade event handler is called? I'm pretty sure that it is just parsing ClientHello (should be something like 0x160303....). If this is the case - I'd suggest you to initiate TLS only after the server will send Continue response.

@adammw
Copy link
Author

adammw commented Mar 2, 2014

On the server, head is <Buffer 30 0d 0a 0d 0a> which is '0\r\n\r\n' (the last couple of bytes of the header).
On the client, upgradeHead is <Buffer >. I think the client isn't sending a hello correctly, and that's why I'm a bit unsure over how this API is supposed to work.

@indutny
Copy link
Member

indutny commented Mar 2, 2014

Oh, right. You should try calling ._start() on TLSSocket. Sorry, I think it needs to be exposed and documented.

@jasnell
Copy link
Member

jasnell commented Aug 3, 2015

@adammw ... I know it's been a while, but are you still interested in pursuing this?

@jasnell
Copy link
Member

jasnell commented Aug 16, 2015

New issue opened in nodejs/node to keep track of this. Closing this PR as it will not be able to land as is.

@jasnell jasnell closed this Aug 16, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants