Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test send-notification only endpoint is failing in Node.js 11 #402

Closed
marco-c opened this issue Jan 14, 2019 · 10 comments
Closed

test send-notification only endpoint is failing in Node.js 11 #402

marco-c opened this issue Jan 14, 2019 · 10 comments
Assignees
Labels

Comments

@marco-c
Copy link
Member

marco-c commented Jan 14, 2019

No description provided.

@marco-c marco-c changed the title test send-notification only endpoint is failing test send-notification only endpoint is failing in Node.js 11 Jan 14, 2019
@spikerheado1234
Copy link

Hi, if this bug is still open could I work on it?

@marco-c
Copy link
Member Author

marco-c commented Jan 20, 2019

@spikerheado1234 sure! Issues are assigned when there is a PR to fix them.

@spikerheado1234
Copy link

Ok sure I'll have a crack at it. Since it is my first time contributing to a large open source project I may need a little help.

@sudipt1999
Copy link

@spikerheado1234 if you are not working on the bug can you please comment ! I would like to work on this

@marco-c
Copy link
Member Author

marco-c commented Mar 7, 2019

Anyone can work on any open issue which is not assigned and which has no PR open linked to it.
If there has been no activity for months, you can assume nobody is working on it ;)

@brunodmartins
Copy link

I will try this out =D. Its my first time too!

@brunodmartins
Copy link

@marco-c , I installed node 11.12.0 and saw that the tests pass fine, just the warning is been logged in the middle of the suite. We can add a --no-warnings param at the command. Doing that, this stop. Is this a good solution? If so, how can I open the PR? Should I fork?

@marco-c
Copy link
Member Author

marco-c commented Mar 23, 2019

I don't like ignoring warnings altogether, as otherwise we could miss something important in the future.

I think you'll have to somehow modify

https.request = function(options, listener) {
to make the request ignore the certificate error.

@brunodmartins
Copy link

@marco-c, I made some try here but it seens that we just cant dont log this warning. This is the code which prints the warning:

let warnOnAllowUnauthorized = true;

// Arguments: [port,] [host,] [options,] [cb]
exports.connect = function connect(...args) {
  args = normalizeConnectArgs(args);
  var options = args[0];
  var cb = args[1];
  const allowUnauthorized = process.env.NODE_TLS_REJECT_UNAUTHORIZED === '0';

  if (allowUnauthorized && warnOnAllowUnauthorized) {
    warnOnAllowUnauthorized = false;
    process.emitWarning('Setting the NODE_TLS_REJECT_UNAUTHORIZED ' +
                        'environment variable to \'0\' makes TLS connections ' +
                        'and HTTPS requests insecure by disabling ' +
                        'certificate verification.');
}

Link: https://github.com/nodejs/node/blob/42dbaed4605f44c393a057aad75a31cac1d0e5f5/lib/_tls_wrap.js

This was add on this PR: nodejs/node#21900

It seens to affect only Node 11.12.0, which is the current version.

@marco-c
Copy link
Member Author

marco-c commented Mar 24, 2019

We need to try to stop using NODE_TLS_REJECT_UNAUTHORIZED entirely (so the warning won't apply anymore), by modifying the code I pointed at earlier.

@marco-c marco-c self-assigned this May 11, 2019
marco-c added a commit that referenced this issue May 11, 2019
…t self-signed certificate and use rejectUnauthorized option instead

Fixes #402
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants