[http] Formal review of Boost.Http

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
74 messages Options
1234
Reply | Threaded
Open this post in threaded view
|

[http] Formal review of Boost.Http

Bjorn Reese
Dear Boost and ASIO communities.

The formal review of Vinícius dos Santos Oliveira's Boost.Http library
starts today, August 7th and ends on Sunday August 16th.

Boost.Http is designed for various forms of modern HTTP interaction,
from normal HTTP request, over HTTP chunking and pipelining, to
upgrading to other web protocols like WebSocket. This library builds
on top of Boost.ASIO, and follows the threading model of ASIO.

The two basic building-blocks are http::socket, which is socket that
talks HTTP, and http::message with contains HTTP meta-data and body
information. You can use these building-blocks to build a HTTP server
that fits your exact needs; for instance, an embedded HTTP server for
a ReST API. Boost.Http comes with a light-weight HTTP server and a
static file server.

Currently, Boost.Http is limited to server-side interaction, but the
design principles used extends to client-side as well.

Boost.Http was originally developed as part of GSoC 2014 and Vinícius
has continued to develop and improve the library since then.

The documentation can be found here:

   http://boostgsoc14.github.io/boost.http/

The source code can be found here:

   https://github.com/BoostGSoC14/boost.http

The source code is build using CMake, but Boost.Build is in the pipeline
(already done for documentation.)

Please answer the following questions:

  1. Should Boost.Http be accepted into Boost? Please state all
     conditions for acceptance explicity.

  2. What is your evaluation of the design?

  3. What is your evaluation of the implementation?

  4. What is your evaluation of the documentation?

  5. What is your evaluation of the potential usefulness of the library?

  6. Did you try to use the library? With what compiler? Did you have
     any problems?

  7. How much effort did you put into your evaluation? A glance? A quick
     reading? In-depth study?

  8. Are you knowledgeable about the problem domain?

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [http] Formal review of Boost.Http

Niall Douglas
On 7 Aug 2015 at 9:08, Bjorn Reese wrote:

> The source code is build using CMake, but Boost.Build is in the pipeline
> (already done for documentation.)

I'll give my frank opinion about Http, and note I had expressed this
privately by email some months ago but my opinion then was not heeded
before presenting this library for review. This has unfortunately
forced me to make these opinions public.

I do not believe Http is ready for peer review for these reasons, and
I personally would urge you withdraw it until you have fixed these
issues and then come back to us. If you do not withdraw it, I will
vote for rejection.


1. You require a C++ 11 compiler and are very much an extension of
ASIO throughout, and then don't take advantage of ASIO's extensive
internal C++ 11 vs Boost switching infrastructure. On the second page
of your documentation you state:

"std::error_code, for instance, cannot be transparently replaced by
boost::system::error_code within ASIO-using code"

This is utterly untrue, and I told you that several months ago. ASIO
itself maps some error_code implementation into namespace asio - go
look at its config.hpp. Therefore you simply use whatever error_code
ASIO itself uses, using the same internal STL abstraction
infrastructure ASIO uses. The same goes for every other instance
where you are using Boost dependencies.

I said several months ago that you should properly finish the ASIO
integration by ripping out all your current hard coded usage of Boost
and using ASIO's own STL abstraction layer, and thereby making Http
identical in supported platforms and configurations as ASIO (i.e.
standalone C++ 11 or with-Boost) reusing the same ASIO configuration
macros. I then suggested it would be a very short step to port Http
to C++ 03 as you are not using anything which particularly demands
C++ 11 outside the examples and tests and which ASIO hasn't
abstracted for you.

Http should be a model EXTENSION of ASIO in the cleanest possible
sense in my opinion. You're not far from achieving it, and I really
think you need to finish it.


2. You claim Boost.Build doesn't work outside the Boost tree. This is
untrue - Boost.Build doesn't need Boost. I don't mind cmake for the
unit testing alone as a temporary stopgap until you bite the
Boost.Build bullet before it enters Boost, but I draw the line at
requiring cmake for normal builds. Before presenting for review
normal library builds cannot use cmake OR should be header only like
Hana.


3. ... which leads me to asking why is Http not a header only library
like ASIO? That would eliminate your Boost.Build requirement in the
strictest sense. Standalone ASIO doesn't need Boost.Build. Neither
should Http.

You mention in the FAQ the reason why not header only is mainly due
to the use of a third party HTTP C parser. It could be insufficient
familiarity on my part with the vaguaries of parsing HTTP, but it has
never struck me as difficult to parse - you don't even need to
tokenise, and indeed my first instinct would be it is better to NOT
tokenise HTTP as that is both slower and opens a wider attack surface
to a hand written "stupid" parser.

(BTW static custom error code categories work just fine header only
if correctly set up. Indeed Boost.System can be configured to be
header only)


4. Http is the first line of parsing code in a world full of
malicious exploitation - just this week all Android devices were made
powned with a single malformed MMS message causing a buffer overflow.
Anything parsing untrusted input needs to be regularly input fuzz
tested period, and very especially Http. Until Http is running at
least one of the below fuzz tools (american fuzzy lop or LLVM
libfuzzer) at least weekly on a CI it is not safe to allow into
production code.

https://svn.boost.org/trac/boost/wiki/BestPracticeHandbook#a7.SAFETY:S
tronglyconsideranightlyorweeklyinputfuzzautomatedtestifyourlibraryisab
letoacceptuntrustedinput


Until you address a minimum of items 1 and 4, I am very sorry but I
must vote for rejection.

Other notes:

* I don't understand why you cannot issue more than one async read at
a time nor async write at a time. In something like pipelined HTTP
that seems like a very common pattern. I smell a potential design
flaw, and while the docs mention a "queue_socket" I can't see such a
thing in the reference section.

* Your tutorial examples use "using namespace std;" at global level.
That's a showstopper of bad practice, and you need to purge all of
those. Same for "using namespace boost;" at global level.

* Your reference section is one very long thin list. You might want
to use more horizontal space.

* I personally found the design rationale not useful. I specifically
want to know:

  1. Why did Http choose ASIO as my base over leading competitor X
and Y?
  2. What sacrifices occurred due to that choice? i.e. things I'd
love weren't the case.
  3. What alternatives may be better suited for the user examining
Http for suitability and why? i.e. make your documentation useful to
people with a problem to solve, not just a howto manual.

* Benchmarks, especially latency ones which are important for many
uses of HTTP are missing. I'd particularly like to know confidence
intervals on the statistical distribution so I know worst case
outcomes.

* Personally speaking, I'd like if in your tutorial you took ten
self-contained steps in building a simple HTTP fileserver, each with
a finished working program at the end of each section. Like the ASIO
tutorial does. In particular, I'd like to see layers which add on
etags and other of the fancy facilities in stages.

* I would personally worry about investing on developing code based
on Http until you have HTTP 2.0 support in there, only because I
would suspect you might need to break the API a bit to get the
all-binary format of HTTP 2.0 working as it's such a change from HTTP
1.0. It may merely be paranoia, but I suspect I wouldn't be alone on
this silent assumption.

* You're still missing CI testing with valgrind, thread sanitiser,
coveralls.io coverage testing, etc etc all the stuff from
https://svn.boost.org/trac/boost/wiki/BestPracticeHandbook.


I hope all the above isn't too disheartening Vinícius. If you took
six weeks extra to finish the port of Http to ASIO proper, added a
Jamfile for the main library build, and patched in some input fuzz
testing (I don't mind if the tests fail across the board BTW, I just
want to see the HTTP parser fuzz tested regularly) I would have no
objection to Http being reviewed and I believe you would get a
conditional acceptance from me at least. As Http currently stands, I
don't believe it is currently ready for review.

I should also add that I have been following this project intently
since its
inception, and I am looking forward to integrating a Http Filesystem
backend
into AFIO as soon as Http passes Boost review and gains a client side
API.

Which, for the record, I believe it will. Your overall design is very
solid,
you had an excellent mentor during GSoC, all you need is (quite a
lot) more polish. I don't find any fundamental design mistakes in
Http apart from the bad design smell surrounding lack of concurrency
in async read and write, and even that could just be a documentation
problem.

Niall

--
ned Productions Limited Consulting
http://www.nedproductions.biz/
http://ie.linkedin.com/in/nialldouglas/




_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

SMime.p7s (8K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [http] Formal review of Boost.Http

Glen Fernandes
Hi Vinícius,

I noticed your roadmap includes benchmarks. Do you have any preliminary benchmarks that you could share during the review period?

Perhaps to:
- cpp-netlib
- poco
- casablanca
- proxygen

Best,
Glen
Reply | Threaded
Open this post in threaded view
|

Re: [http] Formal review of Boost.Http

Vinícius dos Santos Oliveira
2015-08-07 15:24 GMT-03:00 Glen Fernandes <[hidden email]>:

> Do you have any preliminary
> benchmarks that you could share during the review period?
>
> Perhaps to:
> - cpp-netlib
> - poco
> - casablanca
> - proxygen
>

I could provide a sloopy benchmark (just static hello world) during this
weekend, but I don't think it'd be too representative (not near too a
real-world application). I use the Node.js's HTTP parser, so performance
should be Node.js without all the JavaScript engine stuff (and the
possibility of multi-threading).

And I'm writing a reply to Niall's email, but his email is so big that I
don't think I'll finish the reply today. Sorry, Niall (and thanks for the
effort you've put into your feedback).
T_T


--
Vinícius dos Santos Oliveira
https://about.me/vinipsmaker

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [http] Formal review of Boost.Http

Vinícius dos Santos Oliveira
In reply to this post by Niall Douglas
2015-08-07 12:53 GMT-03:00 Niall Douglas <[hidden email]>:

> I do not believe Http is ready for peer review for these reasons, and
> I personally would urge you withdraw it until you have fixed these
> issues and then come back to us. If you do not withdraw it, I will
> vote for rejection.
>

Can I persuade you to review the Boost.Http design? I'd reject myself if
you find any fundamental flaw on the design and even if you're going to
reject the library because the lack of tooling (not directly related to
design or documentation, which are the most important items on any
library), it'd be useful to know what needs to be addressed on the design
before I submit again (in the case the submission is rejected).

1. You require a C++ 11 compiler and are very much an extension of
> ASIO throughout, and then don't take advantage of ASIO's extensive
> internal C++ 11 vs Boost switching infrastructure.


It's not my intention to make Boost.Http work with C++. I use Boost as
dependency and I'm not willing to change that (unless the pieces I use from
Boost enter on the C++ standard).

About C++03 support, it's not required for Boost acceptance, so I can add
later. I want to increase tests coverage a lot before porting code to
C++03, so I don't introduce accidental bugs in the process.

In the case the abstractions I need enter on the C++ standard, maybe your
APIbind will help me.

On the second page

> of your documentation you state:
>
> "std::error_code, for instance, cannot be transparently replaced by
> boost::system::error_code within ASIO-using code"
>
> This is utterly untrue, and I told you that several months ago. ASIO
> itself maps some error_code implementation into namespace asio - go
> look at its config.hpp. Therefore you simply use whatever error_code
> ASIO itself uses, using the same internal STL abstraction
> infrastructure ASIO uses. The same goes for every other instance
> where you are using Boost dependencies.
>

I tried to find such macros on Asio and I couldn't find any for error_code:
http://www.boost.org/doc/libs/1_58_0/doc/html/boost_asio/overview/cpp2011.html

Now that you mentioned config.hpp, I used my grep/find skills and I found:
BOOST_ASIO_HAS_STD_SYSTEM_ERROR

But I just don't find any reference to BOOST_ASIO_HAS_STD_SYSTEM_ERROR
outside of config.hpp. Even if it was used, it is undocumented, which means
it is an implementation detail that won't be guaranteed to be there forever
(accidental behaviour versus a documented guarantee).

I said several months ago that you should properly finish the ASIO

> integration by ripping out all your current hard coded usage of Boost
> and using ASIO's own STL abstraction layer, and thereby making Http
> identical in supported platforms and configurations as ASIO (i.e.
> standalone C++ 11 or with-Boost) reusing the same ASIO configuration
> macros. I then suggested it would be a very short step to port Http
> to C++ 03 as you are not using anything which particularly demands
> C++ 11 outside the examples and tests and which ASIO hasn't
> abstracted for you.
>
> Http should be a model EXTENSION of ASIO in the cleanest possible
> sense in my opinion. You're not far from achieving it, and I really
> think you need to finish it.
>

Delay acceptance of the library will only delay its usage. Design of core
abstractions is finished and I fail to see why its acceptance should be
delayed while I improve implementation details.

Standalone Boost.Http isn't as exciting as a WebSocket implementation.
There is a roadmap of the features:
https://boostgsoc14.github.io/boost.http/design_choices/roadmap.html

C++03 is on the roadmap, but standalone Boost.Http is not.

2. You claim Boost.Build doesn't work outside the Boost tree. This is
> untrue - Boost.Build doesn't need Boost. I don't mind cmake for the
> unit testing alone as a temporary stopgap until you bite the
> Boost.Build bullet before it enters Boost, but I draw the line at
> requiring cmake for normal builds. Before presenting for review
> normal library builds cannot use cmake OR should be header only like
> Hana.
>

CMake will be replaced by Boost.Build before any integration.

3. ... which leads me to asking why is Http not a header only library

> like ASIO? That would eliminate your Boost.Build requirement in the
> strictest sense. Standalone ASIO doesn't need Boost.Build. Neither
> should Http.
>
> You mention in the FAQ the reason why not header only is mainly due
> to the use of a third party HTTP C parser. It could be insufficient
> familiarity on my part with the vaguaries of parsing HTTP, but it has
> never struck me as difficult to parse - you don't even need to
> tokenise, and indeed my first instinct would be it is better to NOT
> tokenise HTTP as that is both slower and opens a wider attack surface
> to a hand written "stupid" parser.
>
> (BTW static custom error code categories work just fine header only
> if correctly set up. Indeed Boost.System can be configured to be
> header only)
>

This parser is well tested and largely used. I want to increase test
coverage a lot before replacing the parser with one of my own. HTTP spec
used to be very confusing before RFC7230. Just now it's easy to implement
all corner-cases. Previously, you needed a lot of experience.

4. Http is the first line of parsing code in a world full of

> malicious exploitation - just this week all Android devices were made
> powned with a single malformed MMS message causing a buffer overflow.
> Anything parsing untrusted input needs to be regularly input fuzz
> tested period, and very especially Http. Until Http is running at
> least one of the below fuzz tools (american fuzzy lop or LLVM
> libfuzzer) at least weekly on a CI it is not safe to allow into
> production code.
>
> https://svn.boost.org/trac/boost/wiki/BestPracticeHandbook#a7.SAFETY:S
> tronglyconsideranightlyorweeklyinputfuzzautomatedtestifyourlibraryisab
> letoacceptuntrustedinput
>

That's why this first implementation uses a proven-parser that is very well
tested. All tests from the Boost.Http socket will run with varying buffer
sizes (from 1 to 2047). Of course I do enjoy tests and I'm constantly
implementing more (the last experiment was using sanitizers).

Until you address a minimum of items 1 and 4, I am very sorry but I

> must vote for rejection.
>
> Other notes:
>
> * I don't understand why you cannot issue more than one async read at
> a time nor async write at a time. In something like pipelined HTTP
> that seems like a very common pattern. I smell a potential design
> flaw, and while the docs mention a "queue_socket" I can't see such a
> thing in the reference section.
>

Too much synchronization and buffering.

The pipeline has the following requests:

A => B => C

If I were to allow replies to request B, the design would be more complex,
increasing the difficult to implement alternative backends and everyone's
life. Also, the reply to request B cannot actually be sent while the reply
to request A is on hold. This means that the whole response to request B
would need buffering and if response B is a video live stream, this means
an implicit (the user cannot know) crash after an unfortunate memory
exhaustion. I couldn't claim a **lightweight** server if this excessive
buffering was to occur. I couldn't claim the project is appropriate for
embedded devices if I had chosen the buffering approach. It's a trade-off.
I should write about it in the "design choice" of the documentation.

HTTP/2.0 does have proper multiplexing and it can be more useful to respond
several requests without "head of line blocking".

A queue socket is a concept, I don't provide one. I can look into how make
the documentation less confusing by adding a page exclusively dedicated to
explain the problem it solves. Would that work for you?

* Your tutorial examples use "using namespace std;" at global level.
> That's a showstopper of bad practice, and you need to purge all of
> those. Same for "using namespace boost;" at global level.
>

The tutorials should be as small as possible to not scary the users.

But I can put a comment "you should avoid using namespace". Does it help?

* Your reference section is one very long thin list. You might want
> to use more horizontal space.
>

I'd be happy already if I knew hot to remove the TOC from the reference
page. It'd be much more clean already.

* I personally found the design rationale not useful. I specifically
> want to know:
>
>   1. Why did Http choose ASIO as my base over leading competitor X
> and Y?
>   2. What sacrifices occurred due to that choice? i.e. things I'd
> love weren't the case.
>

I assume I don't need to reply these questions during the review, given the
reviewers may already be experienced on the answers. However, these are
indeed good questions that I could add to the "design choices" page. Thank
you.

  3. What alternatives may be better suited for the user examining
> Http for suitability and why? i.e. make your documentation useful to
> people with a problem to solve, not just a howto manual.
>

I don't quite understand this point.

* Benchmarks, especially latency ones which are important for many
> uses of HTTP are missing. I'd particularly like to know confidence
> intervals on the statistical distribution so I know worst case
> outcomes.
>

These values will change when I replace the parser used now. Also, they can
change based on the buffer sizes and parser options, which the user is/will
be able to change.

Nevertheless, I do want to do some serious benchmarking, specially because
it'll give me a reference point when I write my own parser and want to
compare performance speedup. I can do some poor benchmark this weekend and
show the results.

* Personally speaking, I'd like if in your tutorial you took ten
> self-contained steps in building a simple HTTP fileserver, each with
> a finished working program at the end of each section. Like the ASIO
> tutorial does. In particular, I'd like to see layers which add on
> etags and other of the fancy facilities in stages.
>

That's a good idea, thank you.

I won't replace the current tutorial, though, as it teaches the main
players on HTTP. I can, however, add a second tutorial showing how to
implement a **simple** file server.

* I would personally worry about investing on developing code based
> on Http until you have HTTP 2.0 support in there, only because I
> would suspect you might need to break the API a bit to get the
> all-binary format of HTTP 2.0 working as it's such a change from HTTP
> 1.0. It may merely be paranoia, but I suspect I wouldn't be alone on
> this silent assumption.
>

You're right about being paranoid here. I was too. But I've spent some time
researching HTTP/2.0 and I got confident about the current design. But you
could vote for conditional acceptance based on HTTP/2.0 and I'd think it is
fair.

* You're still missing CI testing with valgrind, thread sanitiser,
> coveralls.io coverage testing, etc etc all the stuff from
> https://svn.boost.org/trac/boost/wiki/BestPracticeHandbook.
>

I already started the experiments with the sanitizers, but I got a problem
with Boost Test and the experiment was put on hold. I'll come back to it
later.

I hope all the above isn't too disheartening Vinícius.


Not at all. I think you're the second person to dedicate more time
reviewing my work and this isn't disheartening at all. Just that your
feedback is usually around tooling and not the design itself (everybody has
its preferences).

If you took
> six weeks extra to finish the port of Http to ASIO proper,


I disagree about this point. Boost.Http is properly using Asio. Just
because it won't support standalone Asio, it doesn't mean it's improperly
using Asio.

added a
> Jamfile for the main library build,


Consider this done. I won't integrate into Boost without removing CMake
first. This was already partially stated in the Bjorn's announcement (but
somehow implicit, to be fair).


--
Vinícius dos Santos Oliveira
https://about.me/vinipsmaker

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [http] Formal review of Boost.Http

Robert Ramey
On 8/7/15 3:28 PM, Vinícius dos Santos Oliveira wrote:

> Delay acceptance of the library will only delay its usage.

I would disagree here.  There is no reason people can't start using it
now.  In fact, it's much easier to get a library accepted if people have
already started to depend on it.

> 2. Before presenting for review
>> normal library builds cannot use cmake OR should be header only like
>> Hana.

I don't think that there is a requirement that boost build be supported
in order to review a library.
>
> CMake will be replaced by Boost.Build before any integration.

I don't think it's necessary to remove CMake support.  No reason you
can't leave it in and have both CMake and Boost Build support.

>
> Until you address a minimum of items 1 and 4, I am very sorry but I
>> must vote for rejection.

Picking a nit here. The review manager is under no obligation to weigh
"votes" equally.  So I think it would be better if we used the word
"recommendation" rather than vote.

> * You're still missing CI testing with valgrind, thread sanitiser,
>> coveralls.io coverage testing, etc etc all the stuff from
>> https://svn.boost.org/trac/boost/wiki/BestPracticeHandbook.

The items listed in
https://svn.boost.org/trac/boost/wiki/BestPracticeHandbook do not
represent any official boost policy nor have they been discussed such
that they represent any consensus.  So though they may be relevant to
Nail's recommendation, they may not be relevant to anyone else's.

Robert Ramey

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [http] Formal review of Boost.Http

Agustín K-ballo Bergé
On 8/7/2015 9:24 PM, Robert Ramey wrote:

> On 8/7/15 3:28 PM, Vinícius dos Santos Oliveira wrote:
>> * You're still missing CI testing with valgrind, thread sanitiser,
>>> coveralls.io coverage testing, etc etc all the stuff from
>>> https://svn.boost.org/trac/boost/wiki/BestPracticeHandbook.
>
> The items listed in
> https://svn.boost.org/trac/boost/wiki/BestPracticeHandbook do not
> represent any official boost policy nor have they been discussed such
> that they represent any consensus.  So though they may be relevant to
> Nail's recommendation, they may not be relevant to anyone else's.

+1

Regards,
--
Agustín K-ballo Bergé.-
http://talesofcpp.fusionfenix.com

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [http] Formal review of Boost.Http

Glen Fernandes
Agustín K-ballo Bergé wrote:
>
> Robert Ramey wrote:
>>
>> Niall Douglas wrote:
>>> all the stuff from https://svn.boost.org/trac/boost/wiki/BestPracticeHandbook.
>>
>> The items listed in https://svn.boost.org/trac/boost/wiki/BestPracticeHandbook do not represent any official boost policy nor have they been discussed such that they represent any consensus.  So though they may be relevant to Nail's recommendation, they may not be relevant to anyone else's.
>
> +1

+1. I'm not really sure why a page called "BestPracticeHandbook" that
lives on http://*.boost.org/* exists when it is not a "Best practice
handbook" and not anything that the Boost community agrees upon or
even acknowledges.

Glen

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [http] Formal review of Boost.Http

Glen Fernandes
In reply to this post by Vinícius dos Santos Oliveira
Vinícius dos Santos Oliveira wrote:
> I'd be happy already if I knew hot to remove the TOC from the reference page. It'd be much more clean already.

I just chose to avoid nesting too many [section] blocks in favor of
[heading] at that level:
- https://github.com/boostorg/align/blob/master/doc/align.qbk#L528
- http://boostorg.github.io/align/align/reference.html

Best,
Glen

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [asio-users] [http] Formal review of Boost.Http

Lee Clagett-2
In reply to this post by Niall Douglas
Sorry for any duplicates and top posting, but I didn't reply-all, so boost
got dropped (which was probably more important). And this likely ruins some
mail clients (again sorry!).

On Fri, Aug 7, 2015 at 11:10 PM, Lee Clagett <[hidden email]> wrote:

> On Fri, Aug 7, 2015 at 6:29 PM, Vinícius dos Santos Oliveira <
> [hidden email]> wrote:
>
>> 2015-08-07 12:53 GMT-03:00 Niall Douglas <[hidden email]>:
>>
>>> I do not believe Http is ready for peer review for these reasons, and
>>> I personally would urge you withdraw it until you have fixed these
>>> issues and then come back to us. If you do not withdraw it, I will
>>> vote for rejection.
>>>
>> ...[large snip]...
>>
>
>>> * I don't understand why you cannot issue more than one async read at
>>> a time nor async write at a time. In something like pipelined HTTP
>>> that seems like a very common pattern. I smell a potential design
>>> flaw, and while the docs mention a "queue_socket" I can't see such a
>>> thing in the reference section.
>>>
>>
>> Too much synchronization and buffering.
>>
>> The pipeline has the following requests:
>>
>> A => B => C
>>
>> If I were to allow replies to request B, the design would be more
>> complex, increasing the difficult to implement alternative backends and
>> everyone's life. Also, the reply to request B cannot actually be sent while
>> the reply to request A is on hold. This means that the whole response to
>> request B would need buffering and if response B is a video live stream,
>> this means an implicit (the user cannot know) crash after an unfortunate
>> memory exhaustion. I couldn't claim a **lightweight** server if this
>> excessive buffering was to occur. I couldn't claim the project is
>> appropriate for embedded devices if I had chosen the buffering approach.
>> It's a trade-off. I should write about it in the "design choice" of the
>> documentation.
>>
>> HTTP/2.0 does have proper multiplexing and it can be more useful to
>> respond several requests without "head of line blocking".
>>
>> A queue socket is a concept, I don't provide one. I can look into how
>> make the documentation less confusing by adding a page exclusively
>> dedicated to explain the problem it solves. Would that work for you?
>>
>>
> FWIW it should be possible to do client and server side pipelining with
> this design right now. The reads and writes of the HTTP Socket concept
> should be (and currently are by `basic_socket`) handled independently.
> Since the notification for a write completion is based on the lower layer
> write completion, and NOT whether a HTTP response was received, multiple
> (pipelined) requests should be possible by a client. Similarly, this design
> should allow servers to handle pipelined requests in parrallel. Providing a
> framework for client or server pipelining will require more work, and
> should probably not be a part of the Socket concept. In fact -
>
> I think the ServerSocket concept should be removed entirely. The
> `http::basic_socket<...>` models both concepts, so theres lots of server
> specific code in it, which seems confusing to me (is this really a "basic"
> socket?). I think standalone functions for typical client and server
> operations can be implemented as "composed" operations similar to
> `asio::async_write`. The documentation will have to be explicit on what the
> composed function is doing so that users know whether the Socket concept
> has outstanding reads or writes (and therefore cannot do certain operations
> until the handler is invoked). In fact, if there is a server operation that
> _cannot_ be done with the Socket concept, then the concept probably needs
> to be re-designed.
>
> Lee
>

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [asio-users] [http] Formal review of Boost.Http

Vinícius dos Santos Oliveira
2015-08-08 0:10 GMT-03:00 Lee Clagett <[hidden email]>:

> FWIW it should be possible to do client and server side pipelining with
> this design right now. The reads and writes of the HTTP Socket concept
> should be (and currently are by `basic_socket`) handled independently.
> Since the notification for a write completion is based on the lower layer
> write completion, and NOT whether a HTTP response was received, multiple
> (pipelined) requests should be possible by a client. Similarly, this design
> should allow servers to handle pipelined requests in parrallel. Providing a
> framework for client or server pipelining will require more work, and
> should probably not be a part of the Socket concept. In fact -
>

The current design **does** support pipelining. Some HTTP clients disable
pipelining because some buggy servers will get confuse. This won't happen
with Boost.Http (there are tests to ensure pipelning is working).

What the current design does **not** support is handling pipelined requests
concurrently. When you starting read a request, the write_state[1] will
change to finished until the fully request is received, so you can't get
multiple request while you don't issue the replies. This behaviour is
explained in the ServerSocket concept[2].

There are just too many ways to allow HTTP concurrent pipeline that would
be transparent for the user, but all of them are heavier. If I was to allow
handling multiple concurrent HTTP pipelined requests, I'd expose user
cooperation, so the abstraction doesn't become so heavy. The user would
need to be aware if the reply for some request can already be delivered or
not. Of course this means the socket will need to remember order and attach
some id to the request messages. It can be problematic in alternative
backends because the id could be of a different type and makes it difficult
to use the same message type with different HTTP backends. What do you
think? The design would just be more complex for not much gain. HTTP/2.0
allow real multiplexing and doesn't show this problem.

I think the ServerSocket concept should be removed entirely. The
> `http::basic_socket<...>` models both concepts, so theres lots of server
> specific code in it, which seems confusing to me (is this really a "basic"
> socket?).
>

basic_socket uses basic_ prefix just like basic_string.

I think standalone functions for typical client and server operations can
> be implemented as "composed" operations similar to `asio::async_write`. The
> documentation will have to be explicit on what the composed function is
> doing so that users know whether the Socket concept has outstanding reads
> or writes (and therefore cannot do certain operations until the handler is
> invoked). In fact, if there is a server operation that _cannot_ be done
> with the Socket concept, then the concept probably needs to be re-designed.
>

About "users know whether..." seems like a bad idea if I want to deliver
multiple backends. Some implementation details should just be abstracted
away.

I don't understand what you mean by "if there is a server operation that
cannot be done with the Socket concept [...]". Boost.Http has two concepts,
ServerSocket and Socket. Socket concept will be useful to client-side HTTP
and that's the reason why there are two concepts.

[1] https://boostgsoc14.github.io/boost.http/reference/write_state.html
[2] "The ServerSocket object MUST prevent the user from issuing new replies
while [...]",
https://boostgsoc14.github.io/boost.http/reference/server_socket_concept.html


--
Vinícius dos Santos Oliveira
https://about.me/vinipsmaker

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [asio-users] [http] Formal review of Boost.Http

Niall Douglas
In reply to this post by Niall Douglas
On 7 Aug 2015 at 19:29, Vinícius dos Santos Oliveira wrote:

> > I do not believe Http is ready for peer review for these reasons, and
> > I personally would urge you withdraw it until you have fixed these
> > issues and then come back to us. If you do not withdraw it, I will
> > vote for rejection.
>
> Can I persuade you to review the Boost.Http design? I'd reject myself if
> you find any fundamental flaw on the design and even if you're going to
> reject the library because the lack of tooling (not directly related to
> design or documentation, which are the most important items on any
> library), it'd be useful to know what needs to be addressed on the design
> before I submit again (in the case the submission is rejected).
I think my earlier email was poorly phrased now I've slept on it. I
made my objections seem like they were about portability, when they
were really about design and intent of Http.

What I should have said was this: Standalone ASIO is the reference
Networking TS implementation for standardisation. Http is currently a
set of extensions to Boost.ASIO specifically when it *should* be a
set of extensions to the reference Networking TS implementation for
C++ standardisation. This is why your current design and architecture
is flawed, but luckily there isn't too much work to fix this by
finishing the port of Http to the Networking TS.

The Internet of Things tends to involve lots of small somewhat
underpowered internet devices where C++ is likely going to play a
much more important role than historically in embedded systems. The
chances are that any Networking TS in C++ will be very extensively
used in such IoT applications. If there were also a high quality set
of extensions adding HTTP support to the Networking TS, I think your
chances are good that your Http library would be seriously considered
by WG21 for standardisation.

*This* is why I very strongly believe that Http needs to cleave
itself tightly to the Networking TS reference implementation, and not
to Boost.ASIO. Moreover, if you make Http an extension of the
Networking TS, you get Boost support with that for free - Http
becomes dual use.

> About C++03 support, it's not required for Boost acceptance, so I can add
> later. I want to increase tests coverage a lot before porting code to
> C++03, so I don't introduce accidental bugs in the process.

As a strong advocate for C++ 11 over 03 I can understand. However,
IoT developers tend to be stuck on some pretty ancient toolsets for
longer than others. 03 support I think would greatly increase your
potential userbase.

> In the case the abstractions I need enter on the C++ standard, maybe your
> APIbind will help me.

I actually would advise you to choose ASIO's STL abstraction
machinery over my own alternative APIBind in this instance.

> But I just don't find any reference to BOOST_ASIO_HAS_STD_SYSTEM_ERROR
> outside of config.hpp. Even if it was used, it is undocumented, which means
> it is an implementation detail that won't be guaranteed to be there forever
> (accidental behaviour versus a documented guarantee).

I'm pretty sure that implementation detail won't change in a hurry.
STL abstraction machinery changes as quickly as a STL does i.e. every
decade or so.

> > https://svn.boost.org/trac/boost/wiki/BestPracticeHandbook#a7.SAFETY:S
> > tronglyconsideranightlyorweeklyinputfuzzautomatedtestifyourlibraryisab
> > letoacceptuntrustedinput
>
> That's why this first implementation uses a proven-parser that is very well
> tested.

The buck always stops with the top most layer, not internally used
third party libraries.

You NEED to fuzz test untrusted inputs when parsing HTTP. For all you
know, your STL implementation could be the thing with the overflow
bug, or the way in which you use it.

> > * I don't understand why you cannot issue more than one async read at
> > a time nor async write at a time. In something like pipelined HTTP
> > that seems like a very common pattern. I smell a potential design
> > flaw, and while the docs mention a "queue_socket" I can't see such a
> > thing in the reference section.
> >
>
> Too much synchronization and buffering.
>
> The pipeline has the following requests:
>
> A => B => C
>
> If I were to allow replies to request B, the design would be more complex,
> increasing the difficult to implement alternative backends and everyone's
> life. Also, the reply to request B cannot actually be sent while the reply
> to request A is on hold. This means that the whole response to request B
> would need buffering and if response B is a video live stream, this means
> an implicit (the user cannot know) crash after an unfortunate memory
> exhaustion. I couldn't claim a **lightweight** server if this excessive
> buffering was to occur. I couldn't claim the project is appropriate for
> embedded devices if I had chosen the buffering approach. It's a trade-off.
> I should write about it in the "design choice" of the documentation.
>
> HTTP/2.0 does have proper multiplexing and it can be more useful to respond
> several requests without "head of line blocking".
Ok, let's accept that HTTP before 2.0 can't multiplex (not my
experience personally, but I agree it's got big gotchas before 2.0).

You need a user facing API for Http which lets user code multiplex
where that is available, and not multiplex when that is not
available. In other words, identical code written against Http must
automatically be optimal in either connection case.

You're probably going to ask how I might implement that right? I
think my first guess is you need two completion handler layers and
therefore two completion dispatcher reactors: the bottom layer is the
same as ASIO's and uses the ASIO reactor, whilst the top layer which
users of Http sees is a reactor operated by Http and is disconnected,
though operated by, the ASIO reactor.

> A queue socket is a concept, I don't provide one. I can look into how make
> the documentation less confusing by adding a page exclusively dedicated to
> explain the problem it solves. Would that work for you?

It could be this queue socket concept of yours is exactly what I just
proposed as two completion handler and reactor layers. In either
case, I don't think queue sockets should be a concept, I think they
need to be the core of your library's user facing API.

If power users wish to bypass that user facing API layer for less
hand holding and less latency, that should also be possible. But I
think HTTP multiplexing should be assumed to be the default starting
point for Http applications. Once HTTP 2.0 is more common, it will
seem madness that Http's user facing API is _not_ 100% multiplexing.

> * Your tutorial examples use "using namespace std;" at global level.
> > That's a showstopper of bad practice, and you need to purge all of
> > those. Same for "using namespace boost;" at global level.
> >
>
> The tutorials should be as small as possible to not scary the users.
>
> But I can put a comment "you should avoid using namespace". Does it help?

using namespace is fine at local scope. It's not fine at global
scope.

Replacing all global using namespace with local using namespace is
all you need to do.

> * Your reference section is one very long thin list. You might want
> > to use more horizontal space.
> >
>
> I'd be happy already if I knew hot to remove the TOC from the reference
> page. It'd be much more clean already.

Personally in AFIO I hack the CSS to make the list into columns. ASIO
employs a table. There are many solutions.

> * I personally found the design rationale not useful. I specifically
> > want to know:
> >
> >   1. Why did Http choose ASIO as my base over leading competitor X
> > and Y?
> >   2. What sacrifices occurred due to that choice? i.e. things I'd
> > love weren't the case.
> >
>
> I assume I don't need to reply these questions during the review, given the
> reviewers may already be experienced on the answers.
Well ... I'm not sure that's actually the case.

It's not we don't have opinions on the answers, it's whether our
answers are different to your answers. You'll see this during the
AFIO review later this month - I have some really weird opinions no
one else shares like choosing my own custom lightweight monadic
future-promise implementation over async_result. That will be the
source of much criticism I'm sure.

>   3. What alternatives may be better suited for the user examining
> > Http for suitability and why? i.e. make your documentation useful to
> > people with a problem to solve, not just a howto manual.
> >
>
> I don't quite understand this point.

What I'm really asking is for details of when before starting Http
you reviewed the options available and the designs they used, and
exactly why you chose the design choices in Http you did with
reference and comparision to prior art.

Like a literature review, but for programming libraries. Does this
make sense? It's partially for us to help us understand your choices,
but also to help those examining Http to see if it's useful to them
to figure out the best solution to their problem (which may be to use
an alternative to Http).

Documentation which is useful over documentation which is reference.

> * Benchmarks, especially latency ones which are important for many
> > uses of HTTP are missing. I'd particularly like to know confidence
> > intervals on the statistical distribution so I know worst case
> > outcomes.
> >
>
> These values will change when I replace the parser used now. Also, they can
> change based on the buffer sizes and parser options, which the user is/will
> be able to change.
>
> Nevertheless, I do want to do some serious benchmarking, specially because
> it'll give me a reference point when I write my own parser and want to
> compare performance speedup. I can do some poor benchmark this weekend and
> show the results.
That would be interesting, but don't go crazy on it.

Also, please do enable the undefined behaviour sanitiser permanently
including in release mode and with full stack smashing protection
before running benchmarks. Anything parsing malicious input should
have all those turned on where available.

> I hope all the above isn't too disheartening Vinícius.
>
> Not at all. I think you're the second person to dedicate more time
> reviewing my work and this isn't disheartening at all. Just that your
> feedback is usually around tooling and not the design itself (everybody has
> its preferences).

That's because I like the design a lot and have little bad to say
about it.

I've also been subscribed to your github since the beginning, and I
have been reviewing your code on and off as you wrote it. You're a
talented programmer.

> > Jamfile for the main library build,
>
> Consider this done. I won't integrate into Boost without removing CMake
> first. This was already partially stated in the Bjorn's announcement (but
> somehow implicit, to be fair).

I have no problem at all with dual build systems if you're happy
keeping both maintained.

Niall

--
ned Productions Limited Consulting
http://www.nedproductions.biz/
http://ie.linkedin.com/in/nialldouglas/




_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

SMime.p7s (8K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [asio-users] [http] Formal review of Boost.Http

Vinícius dos Santos Oliveira
2015-08-08 11:00 GMT-03:00 Niall Douglas <[hidden email]>:

> As a strong advocate for C++ 11 over 03 I can understand. However,
> IoT developers tend to be stuck on some pretty ancient toolsets for
> longer than others. 03 support I think would greatly increase your
> potential userbase.
>

I do care about C++03. That's why it's on the roadmap. But the design
wouldn't change noticeably and that's why I believe Boost.Http is ready for
review. Some of the requirements you're complaining about aren't Boost
requirements. Boost.Http isn't perfect now (and it'll take maybe a few
years before I exhaust my plans for improvements), but it has a strong core
that is already useful by itself. Lots of Boost libraries continued to
improve with time. It's not like you only do bugfixing after a library is
accepted.

The buck always stops with the top most layer, not internally used
> third party libraries.
>
> You NEED to fuzz test untrusted inputs when parsing HTTP. For all you
> know, your STL implementation could be the thing with the overflow
> bug, or the way in which you use it.
>

I opened an issue, so I won't forget about it:
https://github.com/BoostGSoC14/boost.http/issues/12

Ok, let's accept that HTTP before 2.0 can't multiplex (not my
> experience personally, but I agree it's got big gotchas before 2.0).
>
> You need a user facing API for Http which lets user code multiplex
> where that is available, and not multiplex when that is not
> available. In other words, identical code written against Http must
> automatically be optimal in either connection case.
>

Current design already supports concurrent requests. Each socket is a
communication channel with your application and every time you can handle
another request, you open a new socket where you'll do it. That's my plan
for HTTP 2.0. The advantage is that you don't need to associate any id with
every message (that's more lightweight and more portable against different
HTTP backends).

It could be this queue socket concept of yours is exactly what I just
> proposed as two completion handler and reactor layers. In either
> case, I don't think queue sockets should be a concept, I think they
> need to be the core of your library's user facing API.
>

And then you aren't following C++ rule number #1 anymore: You only pay for
what you use. That's why Asio itself doesn't solve this problem for you.
You can use boost::http::basic_socket<queue_socket> if you need to work
around Asio composed operations at this level. All customization points are
there for anyone.

The queue socket is mentioned in the first page (the front) of
documentation: https://boostgsoc14.github.io/boost.http/#boost_http.f0

Interesting links to follow:

   - http://sourceforge.net/p/asio/mailman/message/32259256/
   -
   http://sourceforge.net/p/axiomq/code/ci/master/tree/include/axiomq/basic_queue_socket.hpp


>   3. What alternatives may be better suited for the user examining
> > > Http for suitability and why? i.e. make your documentation useful to
> > > people with a problem to solve, not just a howto manual.
> > >
> >
> > I don't quite understand this point.
>
> What I'm really asking is for details of when before starting Http
> you reviewed the options available and the designs they used, and
> exactly why you chose the design choices in Http you did with
> reference and comparision to prior art.
>
> Like a literature review, but for programming libraries. Does this
> make sense? It's partially for us to help us understand your choices,
> but also to help those examining Http to see if it's useful to them
> to figure out the best solution to their problem (which may be to use
> an alternative to Http).
>
> Documentation which is useful over documentation which is reference.
>

I see.

That's the entire point on the "design choices" chapter:
https://boostgsoc14.github.io/boost.http/design_choices.html

Of course I can always improve this chapter by explaining more and more.
This review is helping me gather more questions to answer, but I'm already
answering here. So you guys can just ask.


--
Vinícius dos Santos Oliveira
https://about.me/vinipsmaker

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [http] Formal review of Boost.Http

Niall Douglas
In reply to this post by Glen Fernandes
On 7 Aug 2015 at 22:18, Glen Fernandes wrote:

> >>> all the stuff from https://svn.boost.org/trac/boost/wiki/BestPracticeHandbook.
> >>
> >> The items listed in https://svn.boost.org/trac/boost/wiki/BestPracticeHandbook do not represent any official boost policy nor have they been discussed such that they represent any consensus.  So though they may be relevant to Nail's recommendation, they may not be relevant to anyone else's.
> >
> > +1
>
> +1. I'm not really sure why a page called "BestPracticeHandbook" that
> lives on http://*.boost.org/* exists when it is not a "Best practice
> handbook" and not anything that the Boost community agrees upon or
> even acknowledges.
It's a wiki document. Anyone with a Trac login can edit it.

If you feel anything in there is technically wrong, fix it.

If you feel anything in there shouldn't be in there, raise the issue
here and if there is consensus it shouldn't be in there, we'll delete
it.

I would also add that library authors will pick and choose from the
handbook as they see fit. It's not a gospel, nor claims to be one.

The key thing Boost needed was a library development resource guide
as we had little which was up to date and especially relevant to C++
11/14. I wrote my personal vision of that, as I was the one who
invested the 120 hours or so of my family time to write it and I
think that investment of my time gave me that right for the first
edition. If you, or anyone else being critical of my substantial
effort invested here, would like to write something better then go
ahead and do it instead of +1 attacking it when you haven't done
anything better yourself.

But I'd prefer if it evolves into something more consensual, and as I
mentioned everybody has editing rights. That indeed was the original
intent - to get the ball rolling on some new shared documentation, as
nobody was investing the effort to improve Boost documentation at
that level in recent years. You may note each section in the wiki
document has a comments section in which people can ask questions, or
point out mistakes or otherwise give feedback per-section.

That feedback system has already proven useful in substantially
improving some of the sections in the guide where what I wrote was
technically wrong, confusing or unclear, so I guess some people are
finding the Handbook useful.

Niall

--
ned Productions Limited Consulting
http://www.nedproductions.biz/ 
http://ie.linkedin.com/in/nialldouglas/




_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

SMime.p7s (8K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [http] Formal review of Boost.Http

Agustín K-ballo Bergé
On 8/8/2015 1:47 PM, Niall Douglas wrote:

> On 7 Aug 2015 at 22:18, Glen Fernandes wrote:
>
>>>>> all the stuff from https://svn.boost.org/trac/boost/wiki/BestPracticeHandbook.
>>>>
>>>> The items listed in https://svn.boost.org/trac/boost/wiki/BestPracticeHandbook do not represent any official boost policy nor have they been discussed such that they represent any consensus.  So though they may be relevant to Nail's recommendation, they may not be relevant to anyone else's.
>>>
>>> +1
>>
>> +1. I'm not really sure why a page called "BestPracticeHandbook" that
>> lives on http://*.boost.org/* exists when it is not a "Best practice
>> handbook" and not anything that the Boost community agrees upon or
>> even acknowledges.
>
> The key thing Boost needed was a library development resource guide
> as we had little which was up to date and especially relevant to C++
> 11/14. I wrote my personal vision of that, as I was the one who
> invested the 120 hours or so of my family time to write it and I
> think that investment of my time gave me that right for the first
> edition. If you, or anyone else being critical of my substantial
> effort invested here, would like to write something better then go
> ahead and do it instead of +1 attacking it when you haven't done
> anything better yourself.

Why don't you submit it for the traditional review instead? After all,
having spent months (or years!) of your personal time on a library don't
automatically entitle you to make it a Boost library.

> But I'd prefer if it evolves into something more consensual, and as I
> mentioned everybody has editing rights. That indeed was the original
> intent - to get the ball rolling on some new shared documentation, as
> nobody was investing the effort to improve Boost documentation at
> that level in recent years. You may note each section in the wiki
> document has a comments section in which people can ask questions, or
> point out mistakes or otherwise give feedback per-section.
>
> That feedback system has already proven useful in substantially
> improving some of the sections in the guide where what I wrote was
> technically wrong, confusing or unclear, so I guess some people are
> finding the Handbook useful.

I can't speak for others, but I don't think anyone claimed the Handbook
isn't useful. For me personally, the implications that this is a Boost
blessed document rubs me the wrong way, just as libraries that call
themselves Boost.Whatever without ever having been reviewed do.

Regards,
--
Agustín K-ballo Bergé.-
http://talesofcpp.fusionfenix.com

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [http] Formal review of Boost.Http

Darren Cook
In reply to this post by Niall Douglas
> What I should have said was this: Standalone ASIO is the reference
> Networking TS implementation for standardisation. Http is currently a
> set of extensions to Boost.ASIO specifically when it *should* be a
> set of extensions to the reference Networking TS implementation for
> C++ standardisation. This is why your current design and architecture
> is flawed, but luckily there isn't too much work to fix this by
> finishing the port of Http to the Networking TS.

(Q. for Niall Douglas)
Does this change the public-facing API of the proposed Boost.http, or
just the implementation?

>>> * I don't understand why you cannot issue more than one async read at
>>> a time nor async write at a time. In something like pipelined HTTP
>>> that seems like a very common pattern. ...
>>
>> Too much synchronization and buffering.
>>
>> The pipeline has the following requests:
>>
>> A => B => C
>>
>> If I were to allow replies to request B, the design would be more complex,...

(Q. for Vinícius)
Using the diagram at http://stackoverflow.com/q/10480122/841830, does
the above mean that Boost.Http only supports the left-most diagram (e.g.
a browser client cannot request image.png until it has received the
entirety of index.html)?

I'd share Niall's concern that moving to HTTP/2 might require API
changes. In addition to the arrows shown on the rightmost diagram in the
above link, HTTP/2 also supports sending back a file that has not been
requested yet. And it supports one of those arrows being a WebSocket or
SSE connection that continues to send data.

It'd be good to see a code example of how an HTTP/2 server would work,
with all these features present. (I.e. even if it won't compile yet.)

Darren


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [http] Formal review of Boost.Http

Robert Ramey
In reply to this post by Agustín K-ballo Bergé
On 8/8/15 10:06 AM, Agustín K-ballo Bergé wrote:
> I can't speak for others, but I don't think anyone claimed the Handbook
> isn't useful. For me personally, the implications that this is a Boost
> blessed document rubs me the wrong way, just as libraries that call
> themselves Boost.Whatever without ever having been reviewed do.

lol - you're speaking for me!

I just wanted to point out the fact that the "best practices" doesn't
speak for boost.  It's not that it says it does - but the way it's
presented might lead casual readers to assume that it does.

I can't really object to Nial's making his personal recommendations
available in this way.  After all, I've done essentially the same thing
in www.blincubator.com.  I'm just concerned that that placement and tone
suggest that it's the result of some consensus when it's not.  Of course
one could argue that no one has had sufficiently strong opinion to
update/comment or whatever - but this is unconvincing to me.  Most of
don't have time to argue against everything we don't agree with.  This
seems like it's an exploit the boost "trademark" to enhance credibility
and audience beyond what it would otherwise have.

I don't want to dwell too much on the "best practices" wiki.  I'm more
interested in looking at the larger picture of "Boost Web Presence".
There are a couple of things going on here.

a) There are many more opportunities to promote the Boost Mission than
there used to be.

b) There exists the ability to better integrate and promote user driven
content (such as Nail's).

c) How do we better integrate issue tracking to make it easier to use
and integrate with Modular Boost

d) there is the isocpp website - how should Boost complement,
collaborate, or perhaps counter this effort.

e) Boost content has grown topsy turvy over the last 15 years.  There's
lot's of stuff that's outdated, lot's of stuff that's really useful but
hard to find.  How do we organize all this information in a more
coherent way?  And what technology can we leverage on to make it easier
to keep upto date.

f) Rene Rivera has been responsible for Boost infrastructure for many
years and has been very, very reliable in this area.  But the
requirements for evolution of boost web presence have increased by a
lot.  I notice that Rene has been taking on more responsibilities on
boost predef which is a not trivial exercise.  Note all this is in
addition to the being responsible for boost build/test infrastructure.
Also I believe that he has a new job as well.  I contacted Rene about
offloading the responsibility for the Boost Web presence and he seemed
receptive to the idea.

g) We had a meeting about this at C++Now and a weak consensus was
reached that we should be looking to evolve Boost Web Presence.  Michael
Caisse generously agreed to lend resources of his organization to help
with this effort.  It seems that his attempts to move in this direction
have been stymied due to forces beyond his control.  I would like to see
progress on this front.

h) I had all this in mind when I made the boost library incubator.  This
might be consider a place where ideas for Boost 2.0 are being
prototyped.  Some ideas have been a disappointment, others seem useful.
  I'm not sure of the future of the incubator - but for now it's not
urgent to decide as, once up and running, it's been surprisingly easy to
maintain.

All in all, the boost web presence looks to me that it's stuck in 1999.

Reading this, I'm not sure it should be posted here - but then I'm not
sure where it should be posted.

Robert Ramey







_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [asio-users] [http] Formal review of Boost.Http

Lee Clagett-2
In reply to this post by Niall Douglas
On Sat, Aug 8, 2015 at 5:16 AM, Vinícius dos Santos Oliveira <
[hidden email]> wrote:

> 2015-08-08 0:10 GMT-03:00 Lee Clagett <[hidden email]>:
>
>> FWIW it should be possible to do client and server side pipelining with
>> this design right now. The reads and writes of the HTTP Socket concept
>> should be (and currently are by `basic_socket`) handled independently.
>> Since the notification for a write completion is based on the lower layer
>> write completion, and NOT whether a HTTP response was received, multiple
>> (pipelined) requests should be possible by a client. Similarly, this design
>> should allow servers to handle pipelined requests in parrallel. Providing a
>> framework for client or server pipelining will require more work, and
>> should probably not be a part of the Socket concept. In fact -
>>
>
> The current design **does** support pipelining. Some HTTP clients disable
> pipelining because some buggy servers will get confuse. This won't happen
> with Boost.Http (there are tests to ensure pipelning is working).
>
> What the current design does **not** support is handling pipelined
> requests concurrently. When you starting read a request, the write_state[1]
> will change to finished until the fully request is received, so you can't
> get multiple request while you don't issue the replies. This behaviour is
> explained in the ServerSocket concept[2].
>
> There are just too many ways to allow HTTP concurrent pipeline that would
> be transparent for the user, but all of them are heavier. If I was to allow
> handling multiple concurrent HTTP pipelined requests, I'd expose user
> cooperation, so the abstraction doesn't become so heavy. The user would
> need to be aware if the reply for some request can already be delivered or
> not. Of course this means the socket will need to remember order and attach
> some id to the request messages. It can be problematic in alternative
> backends because the id could be of a different type and makes it difficult
> to use the same message type with different HTTP backends. What do you
> think? The design would just be more complex for not much gain. HTTP/2.0
> allow real multiplexing and doesn't show this problem.
>

I agree that providing any pipelining framework in the library is probably
unnecessary right now. However, the design should not inhibit such
functionality from being added (which I do not think is a problem with this
design).



> I think the ServerSocket concept should be removed entirely. The
>> `http::basic_socket<...>` models both concepts, so theres lots of server
>> specific code in it, which seems confusing to me (is this really a "basic"
>> socket?).
>>
>
> basic_socket uses basic_ prefix just like basic_string.
>

I was asking whether basic_socket should NOT model the http::ServerSocket
concept. I understood the naming convention, and the decision to provide
the implementation as a configurable template. The problem is the lack of
composability. If a different http::Socket concept is desired (someone not
using ASIO, etc.), then the http::ServerSocket concept has to be
re-implemented also since there is no other implementation. The obvious way
is to achieve composability is to use inheritance with:
`http::basic_server_socket<Socket>` where `Socket` is a http::Socket
concept. Inheritance in this situation has its drawbacks, for sure (a
virtual destructor should be considered).



> I think standalone functions for typical client and server operations can
>> be implemented as "composed" operations similar to `asio::async_write`. The
>> documentation will have to be explicit on what the composed function is
>> doing so that users know whether the Socket concept has outstanding reads
>> or writes (and therefore cannot do certain operations until the handler is
>> invoked). In fact, if there is a server operation that _cannot_ be done
>> with the Socket concept, then the concept probably needs to be re-designed.
>>
>
> About "users know whether..." seems like a bad idea if I want to deliver
> multiple backends. Some implementation details should just be abstracted
> away.
>
> http::SocketServer implementations manipulate the state of the
http::Socket (and indirectly the asio::tcp::socket), making some actions
unavailable after invoking those functions. For example, after invoking
`async_write_response`, no writes on the http::Socket/http::ServerSocket or
asio::ip::tcp::socket can occur until the callback is invoked because it
calls asio::write on the asio::ip::tcp::socket directly. So unless I am
incorrect, it is important for the users to know whats being manipulated
(the effects).


I don't understand what you mean by "if there is a server operation that
> cannot be done with the Socket concept [...]". Boost.Http has two concepts,
> ServerSocket and Socket. Socket concept will be useful to client-side HTTP
> and that's the reason why there are two concepts.
>
>
The http::ServerSocket concept requires a http::Socket to write its
messages, but does the http::ServerSocket concept need to be a refinement
of the http::Socket concept? This implies a strong relationship. The
current specification looks like a http::ServerTraits concept - its
specifying how a http::Socket is being used in situations specific to
servers. There doesn't appear to be any additional state tracking needed
for the http::ServerSocket functions, which is why I suggested standalone
functions (I didn't see the FileServer section). In fact implementation can
be inverted; make all of the current implementations for http::ServerSocket
standalone functions, and then have the default http::ServerTraits
implementation call the standalone versions of the functions. The
http::ServerTraits should then take a http::Socket as an argument for each
of the functions. This complete de-coupling would allow someone to
independently change the behavior of any related server functions, even
while using the same http::Socket object.


[1] https://boostgsoc14.github.io/boost.http/reference/write_state.html
> [2] "The ServerSocket object MUST prevent the user from issuing new
> replies while [...]",
> https://boostgsoc14.github.io/boost.http/reference/server_socket_concept.html
>
>

Lee

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [http] Formal review of Boost.Http

Glen Fernandes
In reply to this post by Niall Douglas
Niall Douglas wrote:
> If you feel anything in there shouldn't be in there, raise the issue here and if there is consensus it shouldn't be in there, we'll delete it.

Why look for consensus to modify it now? Shouldn't that have happened
before the document was even published on the Boost Trac?

> I wrote my personal vision of that, as I was the one who invested the 120 hours or so of my family time to write it and I think that investment of my time gave me that right for the first edition. If you, or anyone else being critical of my substantial effort invested here, would like to write something better then go ahead and do it instead of +1 attacking it when you haven't done anything better yourself.

What nonsense. What would you think would happen if had admin access
to Boost, just committed AFIO without a formal review, and then
suggested that changes happen to it only after consensus? Is this how
you're going to respond during the formal review of AFIO? By crying
"Don't attack AFIO when you haven't written your own asynchronous file
I/O library"?

> But I'd prefer if it evolves into something more consensual, and as I mentioned everybody has editing rights.

How about we rename the page from "Best Practice Handbook" to "Niall
Douglas' Best Practice Handbook" (or"Niall Douglas' Links To Examples
Of Best Practice For C++ 11/14 Libraries" so that page name matches
the title)?

Just so that nobody is in danger of considering it Boost blessed just
because it lives on the Boost Trac. Maybe later when sufficient Boost
community involvement shapes it into a document worthy of that former
title, we can rename it back?

> You may note each section in the wiki document has a comments section in which people can ask questions, or point out mistakes or otherwise give feedback per-section. That feedback system has already proven useful in substantially improving some of the sections in the guide where what I wrote was technically wrong, confusing or unclear, so I guess some people are finding the Handbook useful.

The only piece of feedback on it that I see is one comment and that
questions whether this should be on your blog instead. [1]

[1] https://muut.com/cppbestpractice/#!/strongly-consider-using-git

Best,
Glen

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [asio-users] [http] Formal review of Boost.Http

Vinícius dos Santos Oliveira
In reply to this post by Lee Clagett-2
2015-08-08 16:36 GMT-03:00 Lee Clagett <[hidden email]>:

> [...] The problem is the lack of composability. If a different
> http::Socket concept is desired (someone not using ASIO, etc.), then the
> http::ServerSocket concept has to be re-implemented also since there is no
> other implementation.
>

Exactly.

The obvious way is to achieve composability is to use inheritance with:
> `http::basic_server_socket<Socket>` where `Socket` is a http::Socket
> concept. Inheritance in this situation has its drawbacks, for sure (a
> virtual destructor should be considered).
>

I don't know. The only place that I see that can be abstracted among all
alternative http backends is management of read_state and write_state and
some auxiliary functions to check user intent (close connection...). The
auxiliary functions can be provided without a base class and the read_state
and write_state code is too little to convince me it's worth.

http::SocketServer implementations manipulate the state of the http::Socket
> (and indirectly the asio::tcp::socket), making some actions unavailable
> after invoking those functions. For example, after invoking
> `async_write_response`, no writes on the http::Socket/http::ServerSocket or
> asio::ip::tcp::socket can occur until the callback is invoked because it
> calls asio::write on the asio::ip::tcp::socket directly. So unless I am
> incorrect, it is important for the users to know whats being manipulated
> (the effects).
>

The documentation points the use of composed operations:
https://boostgsoc14.github.io/boost.http/reference/basic_socket.html

It's akin to Asio documentation on composed operations:
http://www.boost.org/doc/libs/1_58_0/doc/html/boost_asio/reference/async_write/overload1.html
(i.e. "the stream performs no other [...] until this operation completes")

But now that you mentioned, I see that documentation can receive some
improvements on the topic. The ServerSocket concept doesn't mention that
some models can make use of this restriction nor there is any trait to
detect if a model makes use of composed operations.

The http::ServerSocket concept requires a http::Socket to write its

> messages, but does the http::ServerSocket concept need to be a refinement
> of the http::Socket concept? This implies a strong relationship. The
> current specification looks like a http::ServerTraits concept - its
> specifying how a http::Socket is being used in situations specific to
> servers. There doesn't appear to be any additional state tracking needed
> for the http::ServerSocket functions, which is why I suggested standalone
> functions (I didn't see the FileServer section). In fact implementation can
> be inverted; make all of the current implementations for http::ServerSocket
> standalone functions, and then have the default http::ServerTraits
> implementation call the standalone versions of the functions. The
> http::ServerTraits should then take a http::Socket as an argument for each
> of the functions. This complete de-coupling would allow someone to
> independently change the behavior of any related server functions, even
> while using the same http::Socket object.
>

If I understood you correctly, you're proposing that all functions should
be free functions, not member-functions, then everybody can customize any
type to model a ServerSocket. And then you criticize the strong
relationship between Socket and ServerSocket.

Asio doesn't follow this model (async_read_some is a member-function, not a
free function). It does appear to be an interesting idea, but I'm not sure
I'm prepared to solve this detail of customization level. The order of
includes could completely change the behaviour and this is the least of the
problems. Can you elaborate more, then we can discuss?

And the reason behind Socket and ServerSocket is the future addition of a
ClientSocket, which will be a refinement of Socket.

--
Vinícius dos Santos Oliveira
https://about.me/vinipsmaker

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
1234