Menu
▾
▴
Re: [cpp-netlib-devel] [Proxy] The great forgotten feature :)
|
From: Dean M. B. <mik...@gm...> - 2011-04-17 14:04:23
|
Hi Amoriello! First off I'd like to thank you for taking the time and detailing your thoughts. The least I can do is thank you. :D That said, please see my thoughts in-lined below: On Sat, Apr 16, 2011 at 7:38 PM, Amoriello Hutti <amo...@gm...> wrote: > Hi Dean, Contributors, > > > First of all, I would like to thanks you and the contributors of the > cpp-netlib for their awsome work! Thank you, I'm glad you think it's awesome. :) > I would like to use you library, but it misses a key feature for the > use I would like to make of this lib. > > This feature is described as "Use O/S described proxy facilites" > https://github.com/cpp-netlib/cpp-netlib/issues/2 > > I am a cpp fan, but not as experienced as you and the contributors, so > I can't submit any pull request yet. > > But I have red the docs (html, slide, paper and even the boostcon talk > :) and I finaly came to open the code. > All I can share (now) to help this request to be done, is to share > some points I saw while reading the code. > Cool! Thanks for going through all the effort. ;) > Actualy, I tried to imagine where to start if I wanted to implement a > "simple" http-proxy configuration : proxy name and port (no > authentication) in the sync_client impl. > (and because this is the only thing I need yet :) > > > * as you said in the request comment, I also think the best place to > configure the proxy information is the client ctor. > ------------------------------------------------------------------------------------------------ > > So we can imagine we add have a > > sync_client::sync_client (bool cache_resolved, > bool follow_redirect, > optional<string_type> const& > certificate_path = optional<string_type>(), > optional<string_type> const& > verify_path = optional<string_type>(), > optional<string_type> const& > proxy_host = optional<string_type>(), > optional<string_type> const& > proxy_port = optional<string_type>()) > > Well... actually you'd like to look at the Boost.Parameter based constructors that the server and clients already support in 0.9-beta (soon to be 0.9.0). That would make it a lot easier to extend the constructor options that would be supported -- along with adding Boost.Parameter options to the other API calls. > > * Then we have to change the sync_client:request_skeleton<>(...) > ------------------------------------------------------------------------------------------------ > > In this function, you get the connection_object from the > connection_base::get_connection. > Here, we have to pass to the get_connection function the proxy_host > and proxy_port optionals string. That would lead to the following > prototype : > > connection_ptr get_connection(resolver_type& resolver, > basic_request<Tag> const& request_, > optional<string_type> > const & certificate_file = optional<string_type>(), > optional<string_type> > const & verify_file = optional<string_type>(), > optional<string_type> > const & proxy_host = optional<string_type>(), > optional<string_type> > const & proxy_port = optional<string_type>()) > > > and then, if (proxy_host.size != 0), then I initialize the > connection_impl not on the request.host, and port, but on the > proxy_host and port. > You can also use the Boost.Parameters based approach here, or if you're looking at hard-coding it as an internal detail (like how the options for the SSL-specific parts are handled) then it would look like this is the way to go. > ----- I don't really think this "if" is what you expect in a "modern" > library design. > Well, it being a runtime option, we can't really do anything about that. The 'if' statement is still part of the language the last time I checked, no reason for us not to use it. :D > > > > * Then the connection_base::send_request > ------------------------------------------------------------------------------------------------ > > basic_response<Tag> > send_request(string_type const & method, basic_request<Tag> request_, > bool get_body) > > Here, we loose the trace of the proxy_host and proxy_port optional > string (maybe they should be added to the connection_policy's private > members). > > I think, there, we are close to the end of the "simple proxy support > implementation". > Yeah, actually what should really happen is that there should be a way to isolate the factory for connections and have the factories be constructed based on runtime options passed into the constructors. This will be an internal detail of the library which will make it easier to extend the implementation in the future, without having to burden the user with dependency injection mumbo jumbo from the interface. I'll implement this and write a paper about it when I get the time to do that -- between me moving to Sydney, Australia and starting work at Google Australia in a few hours, there's not a lot of time for me to get work done. But that's another issue that needs to be addressed in a different email (following shortly). :D > > * Last, but not least. The linearize algorithm. > ------------------------------------------------------------------------------------------------ > > When we connect to a proxy, (using GET for example), the GET parameter > MUST be the entire uri, not only the uri.path(). > > if (request.path().empty() || request.path()[0] != consts::slash_char()) > *oi = consts::slash_char(); > boost::copy(request.path(), oi); > > > But there, we completely loose the trace of the proxy_host and > proxy_port information, so there is no way to know if it is the > complete uri that should be written, or only the uri.path(). > > I am not really friendly with boost::concept yet, so the linearize.hpp > code is quite obfuscated to me. > I don't know how to help the linearize algorithm to be aware of the > fact that it is behind a proxy or not. > This can be done with a predicate or function object. We can have a path transformer function object that takes a uri and turns it into the appropriate linearized request. This also means we can then use the linearize algorithm for things like body compression, encoding, etc. by providing points of customization, which would be similar to how the STL algorithms do it -- with function objects passed as parameters to the call to the linearize algorithm. Aside from the tag dispatch, we can then set up a set of default transformers and implementations that "do nothing" basically behaving as they do at the moment. At the point where we'd call the linearize algorithm, we then determine externally whether the path transformer needs to know about the proxy stuff along with whether the headers transformer would need to add more details to the headers being linearized. There's a lot of leeway for the algorithm to evolve, and it being an implementation detail that just so happens to be generic enough to be lifted, we're in good shape with changing the implementation and interface. :) > Maybe a new tag "proxy" and some meta like "is_behind_proxy" to > provide a static dispatch based on the tag and avoid the ugly "if" in > the connection object, and help the linearize algo? > If you want to encode that information statically then that would be an option. However I don't think there is any reason to make that a static option when it can very well be just a runtime configuration option. > Thanks for this lib, I hope futur boost reviewer will see the > potential of this piece of master code and design, and will accept it > to be integrated in the boost trunk. > Sorry if I can't submit any pull request yet, and sorry for my bad english. > No problem and thanks for the encouragement. I'll try to see if I can get things going to meet some of the requirements for the proxy implementation, but in the meantime please feel free to just play around with the code and submit any (and al) pull requests that you might want to share with me. Have a good one and I definitely do hope I can attend to this in the coming days/weeks. Cheers! -- Dean Michael Berris http://about.me/deanberris |
