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

proxy support for websocket #542

Closed
wants to merge 7 commits into from
Closed

proxy support for websocket #542

wants to merge 7 commits into from

Conversation

tkanoh
Copy link
Contributor

@tkanoh tkanoh commented Sep 15, 2018

I wrote a patch that support to websoket via proxy.
Processing for exceptional behavior such as connection timeout can not be described,
but I confirmd that It can connect to AWS IoT and Azure IoT Hub via Squid chache with
synchronous connection.
Asynchronous and none SSL connection test is not done.
It will be helpful if you can support proxy in future version.

@icraggs
Copy link
Contributor

icraggs commented Oct 12, 2018

Hi. I see that you've signed the ECA, that's good. You also need to sign your commits.

I don't know too much about HTTP proxies, so probably would need some help in testing this. (@lt-holman thoughts?)

@lt-holman
Copy link
Contributor

lt-holman commented Oct 15, 2018 via email

@tkanoh
Copy link
Contributor Author

tkanoh commented Oct 16, 2018

If it want to implement only Basic Authentication for Proxy,
we can extend the environment variable (e.g. https: //[user[:passwoed]]@hostname[:port])
and pass it to Proxy-authorization header.
(paho.mqtt.c already has a Base64 encoding function, and it is probably easy.)
However, when trying to deal with other authentication methods such as Disgest Authentication,
It will be difficult to pass by environment variable and I think that other future function expansion (e.i. SOCKS protocol) will be affected.
Therefore,I think that it is better to set parameters in MQTTClient_connectOptions structure like MQTTClient_SSLOptions instead of environment variables,
What do you think?

If you want to test this patch with the SQUID of the local machine,
please make some connection using the Websocket protocol by setting "https_proxy=http://127.0.0.1[:port]" ([:port] is port number) as environment variable.

It should have added PGP key,
I am not sure why continuous-integration/appveyor/pr and ip-validation fail ?
This is my first pull request, so it may be that something was wrong. Sorry.

@windbender
Copy link

Ping. I'm finding this would be a useful feature. Where does this PR stand? It seems as though it's been idle for > 6 months despite lots of work and a being pretty useful feature.

@icraggs icraggs force-pushed the master branch 2 times, most recently from 6c3912a to d34c512 Compare August 6, 2019 22:25
Copy link
Contributor

@icraggs icraggs left a comment

Choose a reason for hiding this comment

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

Thanks. The commits need to use the git signed-off-by option to be able to pass the IP check.

Copy link
Contributor

@icraggs icraggs left a comment

Choose a reason for hiding this comment

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

Also, how can I try it out, and add one or more tests?

@icraggs
Copy link
Contributor

icraggs commented Dec 5, 2019

@windbender I did comment in October 2018 that the commits needed to be (git) signed-off-by, but they still haven't. It looks like I'll have to find another way...

@icraggs icraggs mentioned this pull request Dec 5, 2019
Signed-off-by: Tamotsu Kanoh <kanoh@plathome.co.jp>
Signed-off-by: Tamotsu Kanoh <kanoh@plathome.co.jp>
Signed-off-by: Tamotsu Kanoh <kanoh@plathome.co.jp>
Signed-off-by: Tamotsu Kanoh <kanoh@plathome.co.jp>
Signed-off-by: Tamotsu Kanoh <kanoh@plathome.co.jp>
Signed-off-by: Tamotsu Kanoh <kanoh@plathome.co.jp>
@tkanoh
Copy link
Contributor Author

tkanoh commented Dec 8, 2019

I did 'git rebase -i' to append Signed-off-by and passed eclipsefdn/eca check,
but still something wrong for continuous-integration/appveyor/pr check.
I will work again.

@icraggs
Copy link
Contributor

icraggs commented Dec 10, 2019

Hi @tkanoh . Thanks. I think the CI tests are fine. I updated the develop branch so that the pending-tokens test works better but not master, and that is the only test that is failing on windows.

@icraggs
Copy link
Contributor

icraggs commented Dec 10, 2019

I've merged this PR into the develop branch. Thanks!

@icraggs icraggs closed this Dec 10, 2019
@tkanoh
Copy link
Contributor Author

tkanoh commented Dec 10, 2019

@icraggs Tanks to merged my PR and too many advice!!

@keysight-daryl
Copy link
Contributor

Trying to use some of this now... isn't entirely clear if it actually works or not.

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.

5 participants