cpp-netlib-devel

[cpp-netlib-devel] Review of http::message

[Dean M. C. B. <dmb...@fr...>]

Hi Everyone,

I took some time out to look at the implementation of the http::message
-- and right off the bat I see a few issues.

- I don't see unit test(s) for the implementation(s). I know that this
code has been adapted from an already existing library, and it would be
nice to see it in action first even in a measy unit test. I'll try
working on some later just to verify the implementation of the
http::message. Mike, can you give me (or everyone for that matter) an
idea as to how we should be using the http::message in particular
independent of the client? I can write some unit tests to check if it
still complies with the basic_message<> interface. Please let me know if
you want me to do this.

- I don't particularly like 'parser_traits' because it gives me the idea
that it contains nested types instead of constants. I don't have any
better suggestion than 'parser_constants', but somehow we need to
resolve this naming issue one way or another.

- I was suggesting earlier to have instead a tag type which
differentiated an http::message from a basic_message, because precisely
to avoid the glob traits facility that we see in message_traits (and
like the standard iterator traits that glob values together). The
approach would be similar to:

	namespace http { namespace tags {
		struct message_tag { };
	}; };

	namespace http { namespace traits {
		template <typename Tag>
		struct delimiters;

		template <>
		struct delimiters<tags::message_tag> {
			static char const * const STRING_CRLF =
"\x0D\x0A";
			...
		};

		template <typename Tag>
		struct header_names;

		template <>
		struct header<tags::message_tag> {
			static char const * const HEADER_HOST = "Host";
		};
	}; };

In the above approach, we can then even support wide character strings,
by using a different tag to support wide characters.

	namespace http { 
		namespace tags {
			struct message_tag_w { };
		};

		namespace traits {
			template <> struct
delimiters<tags::message_tag_w> {
				static wchar_t const * const CRLF =
L"\x0D\x0A";
				...
			};
		};
	};

The technique is almost like tag dispatching for methods, only this time
it's tag dispatching for type traits.

I can help with this conversion if there are no objections.

Mike, do you see a problem with libpion if we do it the way above?

- If we go with the tag dispatching approach, we can then have a
specialization on basic_message<> instead that has structural details
unique to that specialization.

Please let me know what you think guys, it looks like I'll be going to
need this _real soon_ so I now have a personal stake at making sure this
goes through as planned. ;)

--
Dean Michael Berris
Software Engineer, Friendster, Inc.
<dmb...@fr...>
+639287291459

Re: [cpp-netlib-devel] Review of http::message

[Glyn M. <gly...@gm...>]

Hi Dean,

On 15/04/2008, Dean Michael C. Berris <dmb...@fr...> wrote:
>
>
> - I was suggesting earlier to have instead a tag type which
> differentiated an http::message from a basic_message, because precisely
> to avoid the glob traits facility that we see in message_traits (and
> like the standard iterator traits that glob values together). The
> approach would be similar to:
>
>         namespace http { namespace tags {
>                 struct message_tag { };
>         }; };
>
>         namespace http { namespace traits {
>                 template <typename Tag>
>                 struct delimiters;
>
>                 template <>
>                 struct delimiters<tags::message_tag> {
>                         static char const * const STRING_CRLF =
> "\x0D\x0A";
>                         ...
>                 };
>
>                 template <typename Tag>
>                 struct header_names;
>
>                 template <>
>                 struct header<tags::message_tag> {
>                         static char const * const HEADER_HOST = "Host";
>                 };
>         }; };
>
> In the above approach, we can then even support wide character strings,
> by using a different tag to support wide characters.
>
>         namespace http {
>                 namespace tags {
>                         struct message_tag_w { };
>                 };
>
>                 namespace traits {
>                         template <> struct
> delimiters<tags::message_tag_w> {
>                                 static wchar_t const * const CRLF =
> L"\x0D\x0A";
>                                 ...
>                         };
>                 };
>         };


Maybe I'm missing something here, but isn't it an error to initialize a
static non-integral type inside a struct definition?
Regards,
Glyn

Re: [cpp-netlib-devel] Review of http::message

[Dean M. C. B. <dmb...@fr...>]

Hi Glyn,

> -----Original Message-----
> From: cpp...@li... 
> [mailto:cpp...@li...] On 
> Behalf Of Glyn Matthews
> Sent: Tuesday, April 15, 2008 7:24 PM
> To: C++ Networking Library Developers Mailing List
> Subject: Re: [cpp-netlib-devel] Review of http::message
> 
[snip code]
> 
> 
> Maybe I'm missing something here, but isn't it an error to 
> initialize a static non-integral type inside a struct definition?

I actually am not yet sure myself, but given 'char const * const' is
essentially a pointer type (which can be considered an integral type)
this just might work. This is one reason I'd love to have unit tests
too. ;)

Let me get back to you on this, I can spike it with a compiler.

--
Dean Michael Berris
Software Engineer, Friendster, Inc.
<dmb...@fr...>
+639287291459
 

Re: [cpp-netlib-devel] Review of http::message

[Dean M. C. B. <dmb...@fr...>]

I'm back...

> -----Original Message-----
> From: cpp...@li... 
> [mailto:cpp...@li...] On 
> Behalf Of Dean Michael C. Berris
> Sent: Tuesday, April 15, 2008 7:39 PM
> To: C++ Networking Library Developers Mailing List
> Subject: Re: [cpp-netlib-devel] Review of http::message
> 
> Hi Glyn,
> 
> > -----Original Message-----
> > From: cpp...@li...
> > [mailto:cpp...@li...] On 
> Behalf Of 
> > Glyn Matthews
> > Sent: Tuesday, April 15, 2008 7:24 PM
> > To: C++ Networking Library Developers Mailing List
> > Subject: Re: [cpp-netlib-devel] Review of http::message
> > 
> [snip code]
> > 
> > 
> > Maybe I'm missing something here, but isn't it an error to 
> initialize 
> > a static non-integral type inside a struct definition?
> 
> I actually am not yet sure myself, but given 'char const * 
> const' is essentially a pointer type (which can be considered 
> an integral type) this just might work. This is one reason 
> I'd love to have unit tests too. ;)
> 
> Let me get back to you on this, I can spike it with a compiler.
>

EEP. Yes, you're right.

So this will have to come into some unnamed namespace to be included in
every translation unit. Any other approach you can suggest? 

--
Dean Michael Berris
Software Engineer, Friendster, Inc.
<dmb...@fr...>
+639287291459
 

Re: [cpp-netlib-devel] Review of http::message

[Glyn M. <gly...@gm...>]

On 15/04/2008, Dean Michael C. Berris <dmb...@fr...> wrote:
>
>
> So this will have to come into some unnamed namespace to be included in
> every translation unit. Any other approach you can suggest?


Would it be possible to do something like the following:
namespace http {
namespace tags {
struct message_tag {};
struct message_tag_w {};
}
}

namespace http {
namespace traits {
template <
    typename Tag
    >
struct delimiters;


template <>
struct delimiters<tags::message_tag> {
    static char const *crlf() {
        static const char *const crlf = "\x0D\x0A";
        return crlf;
    }
};


template <>
struct delimiters<tags::message_tag_w> {
    static wchar_t const *crlf() {
        static const wchar_t *const crlf =  L"\x0D\x0A";
        return crlf;
    }
};
}
}

Re: [cpp-netlib-devel] Review of http::message

[Dean M. C. B. <dmb...@fr...>]

> -----Original Message-----
> From: cpp...@li... 
> [mailto:cpp...@li...] On 
> Behalf Of Glyn Matthews
> Sent: Tuesday, April 15, 2008 8:06 PM
> To: C++ Networking Library Developers Mailing List
> Subject: Re: [cpp-netlib-devel] Review of http::message
> 
> 
> Would it be possible to do something like the following:
> namespace http {
> namespace tags {
> struct message_tag {};
> struct message_tag_w {};
> }
> }
> 
> namespace http {
> namespace traits {
> template <
>     typename Tag
>     >
> struct delimiters;
> 
> 
> template <>
> struct delimiters<tags::message_tag> {
>     static char const *crlf() {
>         static const char *const crlf = "\x0D\x0A";
>         return crlf;
>     }
> };
> 
> 
> template <>
> struct delimiters<tags::message_tag_w> {
>     static wchar_t const *crlf() {
>         static const wchar_t *const crlf =  L"\x0D\x0A";
>         return crlf;
>     }
> };
> }
> }
>  

I like it. Thanks Glyn. :)

It looks like stuff that the compiler will inline anyway because it has
full type information with everything being static and all. However, it
will change the way these are accessed -- instead of just doing values,
we need to make function calls...

Perhaps it may work.

Mike, would that be too much of a trouble? I as much as possible would
like to avoid anonymous namespaces because of the ODR.

--
Dean Michael Berris
Software Engineer, Friendster, Inc.
<dmb...@fr...>
+639287291459
 

Re: [cpp-netlib-devel] Review of http::message

[Michael D. <mi...@mi...>]

The new approach sounds (and now, looks) good to me.

Although I noticed that parser_traits.hpp appears to be just a copy of  
the traits.hpp file (the parser constants seem to have been lost  
somewhere in the update).  Whether these are called 'parser_traits',  
'parser_consts' or anything else is fine with me =)

-Mike


On Apr 15, 2008, at 5:18 AM, Dean Michael C. Berris wrote:

>> -----Original Message-----
>> From: cpp...@li...
>> [mailto:cpp...@li...] On
>> Behalf Of Glyn Matthews
>> Sent: Tuesday, April 15, 2008 8:06 PM
>> To: C++ Networking Library Developers Mailing List
>> Subject: Re: [cpp-netlib-devel] Review of http::message
>>
>>
>> Would it be possible to do something like the following:
>> namespace http {
>> namespace tags {
>> struct message_tag {};
>> struct message_tag_w {};
>> }
>> }
>>
>> namespace http {
>> namespace traits {
>> template <
>>    typename Tag
>>>
>> struct delimiters;
>>
>>
>> template <>
>> struct delimiters<tags::message_tag> {
>>    static char const *crlf() {
>>        static const char *const crlf = "\x0D\x0A";
>>        return crlf;
>>    }
>> };
>>
>>
>> template <>
>> struct delimiters<tags::message_tag_w> {
>>    static wchar_t const *crlf() {
>>        static const wchar_t *const crlf =  L"\x0D\x0A";
>>        return crlf;
>>    }
>> };
>> }
>> }
>>
>
> I like it. Thanks Glyn. :)
>
> It looks like stuff that the compiler will inline anyway because it  
> has
> full type information with everything being static and all. However,  
> it
> will change the way these are accessed -- instead of just doing  
> values,
> we need to make function calls...
>
> Perhaps it may work.
>
> Mike, would that be too much of a trouble? I as much as possible would
> like to avoid anonymous namespaces because of the ODR.
>
> --
> Dean Michael Berris
> Software Engineer, Friendster, Inc.
> <dmb...@fr...>
> +639287291459
>
>
> -------------------------------------------------------------------------
> This SF.net email is sponsored by the 2008 JavaOne(SM) Conference
> Don't miss this year's exciting event. There's still time to save  
> $100.
> Use priority code J8TL2D2.
> http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
> _______________________________________________
> Cpp-netlib-devel mailing list
> Cpp...@li...
> https://lists.sourceforge.net/lists/listinfo/cpp-netlib-devel
>

Re: [cpp-netlib-devel] Review of http::message

[Dean M. C. B. <dmb...@fr...>]

> -----Original Message-----
> From: cpp...@li... 
> [mailto:cpp...@li...] On 
> Behalf Of Michael Dickey
> Sent: Monday, April 21, 2008 4:20 AM
> To: C++ Networking Library Developers Mailing List
> Subject: Re: [cpp-netlib-devel] Review of http::message
> 
> The new approach sounds (and now, looks) good to me.
>

Nice. That's good to know Mike. :)
 
> Although I noticed that parser_traits.hpp appears to be just 
> a copy of the traits.hpp file (the parser constants seem to 
> have been lost somewhere in the update).  Whether these are 
> called 'parser_traits', 'parser_consts' or anything else is 
> fine with me =)
> 

Yeah, I haven't gotten to chopping that file up just yet. I'm still
mulling around the name(s), but I think I'll just go first with
parser_consts or a more fine-grained approach at decomposing the various
constants contained in parser_traits. I'll let you know when I get
something in soon. :)

Thanks Mike!

--
Dean Michael Berris
Software Engineer, Friendster, Inc.
<dmb...@fr...>
+639287291459
 

Re: [cpp-netlib-devel] Review of http::message

[Dean M. C. B. <dmb...@fr...>]

Okay, so just now I chopped up parser_traits into the following files:
	
network/protocol/http/traits/impl/cookie_name.ipp
network/protocol/http/traits/impl/cookie_value.ipp
network/protocol/http/traits/impl/header_name.ipp
network/protocol/http/traits/impl/header_value.ipp
network/protocol/http/traits/impl/method.ipp
network/protocol/http/traits/impl/post_content.ipp
network/protocol/http/traits/impl/query_name.ipp
network/protocol/http/traits/impl/query_string.ipp
network/protocol/http/traits/impl/query_value.ipp
network/protocol/http/traits/impl/resource.ipp
network/protocol/http/traits/impl/status_message.ipp
network/protocol/http/traits/parser_traits.hpp

In this approach, we then use the tag dispatch to get the appropriate
typedef for the MAX nested static value:

	http::cookie_name<http::message_tag>::MAX; // 1024u

Mike, please let me know when you think we can start changing the parser
implementation(s) to use the newer traits mechanisms.

I like this rate that we're proceeding. I'll try writing more tests to
satisfy some of Divye's curiousity -- or if you're already trying your
hand at those Divye, please let me know so we can coordinate
accordingly.

Have a good day guys!

--
Dean Michael Berris
Software Engineer, Friendster, Inc.
<dmb...@fr...>
+639287291459
 

> -----Original Message-----
> From: cpp...@li... 
> [mailto:cpp...@li...] On 
> Behalf Of Dean Michael C. Berris
> Sent: Monday, April 21, 2008 2:15 PM
> To: C++ Networking Library Developers Mailing List
> Subject: Re: [cpp-netlib-devel] Review of http::message
> 
> > -----Original Message-----
> > From: cpp...@li...
> > [mailto:cpp...@li...] On 
> Behalf Of 
> > Michael Dickey
> > Sent: Monday, April 21, 2008 4:20 AM
> > To: C++ Networking Library Developers Mailing List
> > Subject: Re: [cpp-netlib-devel] Review of http::message
> > 
> > The new approach sounds (and now, looks) good to me.
> >
> 
> Nice. That's good to know Mike. :)
>  
> > Although I noticed that parser_traits.hpp appears to be 
> just a copy of 
> > the traits.hpp file (the parser constants seem to have been lost 
> > somewhere in the update).  Whether these are called 
> 'parser_traits', 
> > 'parser_consts' or anything else is fine with me =)
> > 
> 
> Yeah, I haven't gotten to chopping that file up just yet. I'm 
> still mulling around the name(s), but I think I'll just go 
> first with parser_consts or a more fine-grained approach at 
> decomposing the various constants contained in parser_traits. 
> I'll let you know when I get something in soon. :)
> 
> Thanks Mike!
> 
> --
> Dean Michael Berris
> Software Engineer, Friendster, Inc.
> <dmb...@fr...>
> +639287291459
>  
> 
> --------------------------------------------------------------
> -----------
> This SF.net email is sponsored by the 2008 JavaOne(SM) 
> Conference Don't miss this year's exciting event. There's 
> still time to save $100. 
> Use priority code J8TL2D2. 
> http://ad.doubleclick.net/clk;198757673;13503038;p?http://java
> .sun.com/javaone
> _______________________________________________
> Cpp-netlib-devel mailing list
> Cpp...@li...
> https://lists.sourceforge.net/lists/listinfo/cpp-netlib-devel
> 

Re: [cpp-netlib-devel] Review of http::message

[Divye K. <div...@gm...>]

Hi,

On Mon, Apr 21, 2008 at 2:06 PM, Dean Michael C. Berris <
dmb...@fr...> wrote:

> I like this rate that we're proceeding. I'll try writing more tests to
> satisfy some of Divye's curiousity -- or if you're already trying your
> hand at those Divye, please let me know so we can coordinate
> accordingly.
>

  Exams are fast approaching and regular quizzes are keeping me busy. I'm
currently looking only at what's being checked in and viewing it as a
potential user :-) Don't mind my passiveness for now. I'll get down to
writing black box tests for verification of the HTTP protocol specifications
only after my exams (thats after the 13th of May. But I'll definitely be
keeping track of progress and put in a line of code or two. Thanks to all
this, I'm learning about Boost Tests.

A note: The tests that were recently checked in do not time out even after 5
minutes on a linux system. I'll try those tests on Windows and post any
differences.

Regards,
Divye

-- 
Whether the chicken crossed the road or the road moved beneath the chicken
depends upon your frame of reference.
My official web site:
http://www.iitr.ernet.in/studenthomepages/homepages/061305

Re: [cpp-netlib-devel] Review of http::message

[Dean M. C. B. <dmb...@fr...>]

> -----Original Message-----
> From: cpp...@li... 
> [mailto:cpp...@li...] On 
> Behalf Of Divye Kapoor
> Sent: Monday, April 21, 2008 9:52 PM
> To: C++ Networking Library Developers Mailing List
> Subject: Re: [cpp-netlib-devel] Review of http::message
> 
> Hi,
> 
> 
> On Mon, Apr 21, 2008 at 2:06 PM, Dean Michael C. Berris 
> <dmb...@fr...> wrote:
> 
> 
> 	I like this rate that we're proceeding. I'll try 
> writing more tests to
> 	satisfy some of Divye's curiousity -- or if you're 
> already trying your
> 	hand at those Divye, please let me know so we can coordinate
> 	accordingly.
> 	
> 
>  
>   Exams are fast approaching and regular quizzes are keeping 
> me busy. I'm currently looking only at what's being checked 
> in and viewing it as a potential user :-) Don't mind my 
> passiveness for now. I'll get down to writing black box tests 
> for verification of the HTTP protocol specifications only 
> after my exams (thats after the 13th of May. But I'll 
> definitely be keeping track of progress and put in a line of 
> code or two. Thanks to all this, I'm learning about Boost Tests. 
> 

Take your time, it's nice to know that you're still interested. :)

> A note: The tests that were recently checked in do not time 
> out even after 5 minutes on a linux system. I'll try those 
> tests on Windows and post any differences.
> 

Yup. And I'll have to move away from Boost.Asio's iostream interface --
unless it actually implements a timeout on the stream that I don't know
about yet.

This is actually high time for the libPion HTTP client implementation to
move into the http::client implementation. I think if we can actually
unit-test parts of the parser(s), and then move the implementation into
the HTTP client we already have, we should be able to move forward at a
better pace -- with error condition checking, etc.

As for that, I'll have to wait for Mike's moving the files into the
repository to get me started on this. :)

--
Dean Michael Berris
Software Engineer, Friendster, Inc.
<dmb...@fr...>
+639287291459
 

Re: [cpp-netlib-devel] Review of http::message

[Michael D. <mi...@mi...>]

I like it!  Makes me wonder though, if maybe we should just make all  
these part of the http "message traits," and do away with the "parser  
traits" concept altogether?

-Mike

On Apr 21, 2008, at 1:36 AM, Dean Michael C. Berris wrote:

> Okay, so just now I chopped up parser_traits into the following files:
> 	
> network/protocol/http/traits/impl/cookie_name.ipp
> network/protocol/http/traits/impl/cookie_value.ipp
> network/protocol/http/traits/impl/header_name.ipp
> network/protocol/http/traits/impl/header_value.ipp
> network/protocol/http/traits/impl/method.ipp
> network/protocol/http/traits/impl/post_content.ipp
> network/protocol/http/traits/impl/query_name.ipp
> network/protocol/http/traits/impl/query_string.ipp
> network/protocol/http/traits/impl/query_value.ipp
> network/protocol/http/traits/impl/resource.ipp
> network/protocol/http/traits/impl/status_message.ipp
> network/protocol/http/traits/parser_traits.hpp
>
> In this approach, we then use the tag dispatch to get the appropriate
> typedef for the MAX nested static value:
>
> 	http::cookie_name<http::message_tag>::MAX; // 1024u
>
> Mike, please let me know when you think we can start changing the  
> parser
> implementation(s) to use the newer traits mechanisms.
>
> I like this rate that we're proceeding. I'll try writing more tests to
> satisfy some of Divye's curiousity -- or if you're already trying your
> hand at those Divye, please let me know so we can coordinate
> accordingly.
>
> Have a good day guys!
>
> --
> Dean Michael Berris
> Software Engineer, Friendster, Inc.
> <dmb...@fr...>
> +639287291459
>
>
>> -----Original Message-----
>> From: cpp...@li...
>> [mailto:cpp...@li...] On
>> Behalf Of Dean Michael C. Berris
>> Sent: Monday, April 21, 2008 2:15 PM
>> To: C++ Networking Library Developers Mailing List
>> Subject: Re: [cpp-netlib-devel] Review of http::message
>>
>>> -----Original Message-----
>>> From: cpp...@li...
>>> [mailto:cpp...@li...] On
>> Behalf Of
>>> Michael Dickey
>>> Sent: Monday, April 21, 2008 4:20 AM
>>> To: C++ Networking Library Developers Mailing List
>>> Subject: Re: [cpp-netlib-devel] Review of http::message
>>>
>>> The new approach sounds (and now, looks) good to me.
>>>
>>
>> Nice. That's good to know Mike. :)
>>
>>> Although I noticed that parser_traits.hpp appears to be
>> just a copy of
>>> the traits.hpp file (the parser constants seem to have been lost
>>> somewhere in the update).  Whether these are called
>> 'parser_traits',
>>> 'parser_consts' or anything else is fine with me =)
>>>
>>
>> Yeah, I haven't gotten to chopping that file up just yet. I'm
>> still mulling around the name(s), but I think I'll just go
>> first with parser_consts or a more fine-grained approach at
>> decomposing the various constants contained in parser_traits.
>> I'll let you know when I get something in soon. :)
>>
>> Thanks Mike!
>>
>> --
>> Dean Michael Berris
>> Software Engineer, Friendster, Inc.
>> <dmb...@fr...>
>> +639287291459
>>
>>
>> --------------------------------------------------------------
>> -----------
>> This SF.net email is sponsored by the 2008 JavaOne(SM)
>> Conference Don't miss this year's exciting event. There's
>> still time to save $100.
>> Use priority code J8TL2D2.
>> http://ad.doubleclick.net/clk;198757673;13503038;p?http://java
>> .sun.com/javaone
>> _______________________________________________
>> Cpp-netlib-devel mailing list
>> Cpp...@li...
>> https://lists.sourceforge.net/lists/listinfo/cpp-netlib-devel
>>
>
> -------------------------------------------------------------------------
> This SF.net email is sponsored by the 2008 JavaOne(SM) Conference
> Don't miss this year's exciting event. There's still time to save  
> $100.
> Use priority code J8TL2D2.
> http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
> _______________________________________________
> Cpp-netlib-devel mailing list
> Cpp...@li...
> https://lists.sourceforge.net/lists/listinfo/cpp-netlib-devel
>

Re: [cpp-netlib-devel] Review of http::message

[Dean M. C. B. <dmb...@fr...>]

Hi Mike,

> -----Original Message-----
> From: cpp...@li... 
> [mailto:cpp...@li...] On 
> Behalf Of Michael Dickey
> Sent: Monday, April 21, 2008 11:43 PM
> To: C++ Networking Library Developers Mailing List
> Subject: Re: [cpp-netlib-devel] Review of http::message
> 
> I like it!  Makes me wonder though, if maybe we should just 
> make all these part of the http "message traits," and do away 
> with the "parser traits" concept altogether?
>

Sounds good to me. ;) Actually, it's just a simple change in the header
includes. I'll save that for later though. :D

Do you have a good estimate Mike on when we can see more libpion parser
code (and tests) getting into the library? I'm itching to get started on
this just so you know... ;)

--
Dean Michael Berris
Software Engineer, Friendster, Inc.
<dmb...@fr...>
+639287291459