cpp-netlib-devel

[cpp-netlib-devel] [Proxy] The great forgotten feature :)

[Amoriello H. <amo...@gm...>]

Hi Dean, Contributors,


First of all, I would like to thanks you and the contributors of the
cpp-netlib for their awsome work!
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.

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>())



* 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.

----- I don't really think this "if" is what you expect in a "modern"
library design.




* 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".




* 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.

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?

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.

-- 
Amoriello Hutti

Re: [cpp-netlib-devel] [Proxy] The great forgotten feature :)

[Dean M. B. <mik...@gm...>]

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