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 #2019

Open
jano42 opened this issue Nov 29, 2019 · 3 comments · May be fixed by #2022
Open

Proxy support #2019

jano42 opened this issue Nov 29, 2019 · 3 comments · May be fixed by #2022
Assignees
Milestone

Comments

@jano42
Copy link
Contributor

jano42 commented Nov 29, 2019

I'm wondering if you plan to support proxies ? (for downloading config files)

As this might be complex, one other solution would be for calling code to provide an httpClient (there's already a method in Driver class bool Driver::setHttpClient(Internal::i_HttpClient *client)
but parameter is internal (and not accessible from Manager public api).
For example in my app, I already have my http layer which handle proxies, http redirections,.... (quite huge work).

But for that the i_HttpClient should be exposed...

I think giving ability of external code to provide an HttpClient could be a good solution; which allow to handle any custom behavior (proxy w/o authentication, redirections, ,...)

Would you like I make a PR about that ?

@Fishwaldo
Copy link
Member

Fishwaldo commented Dec 1, 2019

SSL and Proxy are on the todo....

SSL - I don't like pulling in external dependencies though... there is support for SSL in the platform/HttpClient.cpp/h files for PolarSSL.

Proxies - Support would have to be added to HttpSocket::SendRequest

You can call the Driver::setHttpClient directly. Its accessable, just not the prefered way to do this at the moment. (but given no other option..)

I'm not sure if via the Manager Class is the right place to Set a HTTP Client implementation though. (we do a similar implementation for the Log class and that's not exposed via the Manager class either).

But for that the i_HttpClient should be exposed...

We could move it from the OpenZWave::Internal to the OpenZWave namespace I guess... But.... that breaks the API. So maybe future major revision?

@Fishwaldo
Copy link
Member

We could move it from the OpenZWave::Internal to the OpenZWave namespace I guess... But.... that breaks the API. So maybe future major revision?

Actually, given you are the first to ask about this... there might not be any users of that setHttpClient (and thus i_HttpClient) so maybe we can do it... let me think about it and where to put the setHttpClient method...

@Fishwaldo Fishwaldo self-assigned this Dec 1, 2019
@jano42
Copy link
Contributor Author

jano42 commented Dec 2, 2019

If you want, I've done a small quick PR for that.

Just another words of the integration of the PolarSSL library. You should have a look at (PocoProject)[https://pocoproject.org]. This is a complete framework which manage lots of things, and one is Poco::Net (associate with Poco::NetSSL, based on OpenSSL) mange everything around HTTP (request, webserver, ssl, proxies,...). This is a quite good library, stable, opensource, which build on any platform. This is just an example. I use this library for 5 years and this allow me to concentrate my developement on my own job, withtout losting my time on writing something which is already validated by others...

@jano42 jano42 linked a pull request Dec 2, 2019 that will close this issue
@Fishwaldo Fishwaldo added this to the 1.7 milestone Mar 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants