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

Document the 'protocol' configuration option #938

Closed
wants to merge 1 commit into from

Conversation

siliconcat
Copy link

There is an undocumented protocol option that can be used. I was trying to figure a way to use http instead of https (in the older version of the client I was using in my project) for testing and couldn't find it. But I can see it is now there and just not documented in the Readme.

@richardm-stripe
Copy link
Contributor

richardm-stripe commented Jun 30, 2020

Hi @siliconcat, thanks for the pull request.

The protocol configuration option does work to send plain HTTP, but it is not officially supported and we don't want to document it as part of the public interface. We recognize there are a few legitimate uses, but we want to discourage as heavily as possible users from sending their credentials over an insecure channel when it can be avoided.

@siliconcat
Copy link
Author

Hi @richardm-stripe,

Since it is not official I guess it could then be removed at anytime without warning, which leaves me uneasy. IMO it would be better to document it and warn accordingly, and maybe explain its use-cases, rather than obfuscate that it exists. Otherwise I would remove it altogether so that is the best way to "discourage" its use. If it is there, people would find it online (like I did) and even use it in a "bad" way, since there is no warning at all from the library maintainers.

In my case, I came to it after a few hours of trying to find a good solution to run my app against a mock to capture the payloads we are sending to it in our tests, using Docker. Any other good solution for that? Since I use Docker for Mac, it is tricky to access localhost containers on https, hence using http did save the day.

Many thanks!

@richardm-stripe
Copy link
Contributor

Thanks for the feedback and detailing your use case @siliconcat. We're trying the "document it and warn accordingly" as you suggest in #940.

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