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

fix bug: opts.rejectUnauthorized for SSL option #171

Merged
merged 5 commits into from
Sep 5, 2023

Conversation

nasyarobby
Copy link
Contributor

@nasyarobby nasyarobby commented Sep 1, 2023

Using cli args

the options passed to the client does not include rejectUnauthorized because the value is added after the client is create.
So, I moved the code right before the client creation.

Using configuration file

the check for allowed keys will skip the key if the value is false, so the options will not include the key. To fix this, the code should only skip if the value is undefined.

allowedProps.forEach(key => {
      if (config[key] !== undefined) { flags[key] = config[key] }
    })

@mcollina
Copy link
Member

mcollina commented Sep 1, 2023

Could you add a test for this?

@nasyarobby
Copy link
Contributor Author

sorry for late reply. Sure, can I just write a unit test to make sure the option passed to the client? Or I need to test using the ES (I assume so when I see those docker-compose files) ?

@mcollina
Copy link
Member

mcollina commented Sep 4, 2023

the former is ok

@nasyarobby
Copy link
Contributor Author

@mcollina
Added a new test file for the cli and made some changes to file cli.js so I can make unit tests for it.
I only write some tests for starter, but I hope that is enough to cover the issue I try to raise in this PR.

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm thanks!

@mcollina mcollina merged commit 93bb4c2 into pinojs:master Sep 5, 2023
5 checks passed
@nasyarobby nasyarobby deleted the fix-reject-unauthorized-flag branch September 5, 2023 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants