Discussion:
[boost] [review][beast] Review of Beast starts today : July 1 - July 10
Michael Caisse via Boost
2017-07-01 07:39:45 UTC
Permalink
The formal review of Vinnie Falco’s Beast library will take place from
July 1 through July 10, 2017.

Please consider participating in this review. The success of Boost is
partially a result of the quality review process which is conducted by
the community. You are part of the Boost community. I will be grateful
to receive a review based on whatever level of effort or time you can
devote.

Beast is a C++ header-only library serving as a foundation for writing
interoperable networking libraries by providing low-level HTTP/1,
WebSocket, and networking protocol vocabulary types and algorithms using
the consistent asynchronous model of Boost.Asio.

Beast is designed for:

* Symmetry: Algorithms are role-agnostic; build clients, servers, or
both.
* Ease of Use: Boost.Asio users will immediately understand Beast.
* Flexibility: Users make the important decisions such as buffer or
thread management.
* Performance: Build applications handling thousands of connections
or more.
* Basis for Further Abstraction: Components are well-suited for
building upon.

A branch has been made for the review. You can find Beast for review at
these links:

* Documentation : <http://vinniefalco.github.io/stage/beast/review/>
* Source code : <https://github.com/vinniefalco/Beast/tree/review>
* The FAQ answers real questions that come up :

<http://vinniefalco.github.io/stage/beast/review/beast/design_choices/faq.html>

A lightning talk from CppCon 2016 introducing Beast can be found here:
<https://www.youtube.com/watch?v=uJZgRcvPFwI>


Please provide in your review whatever information you think is valuable
to understand your final choice of ACCEPT or REJECT including Beast as a
Boost library. Please be explicit about your decision.

Some other questions you might want to consider answering:

- What is your evaluation of the design?
- What is your evaluation of the implementation?
- What is your evaluation of the documentation?
- What is your evaluation of the potential usefulness of the library?
- Did you try to use the library? With which compiler(s)? Did you
have any problems?
- How much effort did you put into your evaluation? A glance? A quick
reading? In-depth study?
- Are you knowledgeable about the problem domain?

More information about the Boost Formal Review Process can be found
here: <http://www.boost.org/community/reviews.html>

Thank you for your effort in the Boost community.

Happy coding -
michael

--
Michael Caisse
Ciere Consulting
ciere.com

_______________________________________________
Unsubscribe & other changes: http://list
Niall Douglas via Boost
2017-07-01 14:46:02 UTC
Permalink
> Please consider participating in this review. The success of Boost is
> partially a result of the quality review process which is conducted by
> the community. You are part of the Boost community. I will be grateful
> to receive a review based on whatever level of effort or time you can
> devote.

As this is such a big library, some early first impressions/notes.
Positives:

+ This library is far better quality than when I last looked at it which
was actually probably a long time ago. Congrats to Vinnie on that. It
ain't easy getting it to this far.

+ I pretty much agree with everything in his design rationales in the
section at
https://vinniefalco.github.io/stage/beast/review/beast/design_choices.html
where he compares to other HTTP C++ implementations.

+ I found the docs clear, about the right balance between detail and
simplicity, and I understood everything I felt I needed to know quickly.
Well done on that as well Vinnie, it's a hard balance to achieve.

+ I very much agree with the choices in boundary separating what to
provide and what not to provide, with only a few niggles, but nothing
major and nothing showstopper. I feel a lot of the criticism seen to
date about that choice is meritless once you actually understand real
world implementations of HTTP.


Negatives:

- For this collection of HTTP-related library routines which is what
this library actually is, I do not see why any awareness of ASIO is
needed at all. I don't get why it's needed, and moreover, I think the
library would be far better off removing all awareness of a networking
implementation entirely.

If Beast implemented full HTTP like Boost.Http tried to do, I could see
the value add in tying in tightly to some networking implementation or
other. But it does not implement HTTP, it implements a set of routines
that one would use to implement HTTP instead. Most of the routines Beast
supplies that I examined would be more reusable, easier to use, and less
templatey if they made zero assumptions about what networking
implementation is in use.


- Everything appears to be a template. This sort of API design is forced
on Beast by the tight dependency on ASIO whose public API is almost
entirely templated. I could see that this design choice of ASIO's made
sense in C++ 98 days with the knowledge of best practices then extant in
2005, but in a C++ 11 library designed after 2015 it is the wrong design
choice. We simply know better nowadays, plus WG21 has much superior
vocabulary types for doing buffer sequences than ASIO's which are
needlessly complex, over engineered, and over wraught. A new library
should not repeat the design mistakes of ASIO unless it has to, and I
see no compelling reason given the limited implementation of HTTP
offered by Beast to have any dependency on ASIO at all.

Beast does a great job of drawing the correct boundary around what of
HTTP to implement and what not to implement. I very much agree with that
boundary. But the choice to model ASIO was made superfluous by that
boundary choice. The ASIO design model should be replaced with a more
generic reusable design instead, one which works with any arbitrary
socket API - or indeed, non-socket API.

An an example of a non-socket API, imagine using the UDT stream layer
instead of TCP (http://udt.sourceforge.net/) and implementing HTTP on
top of that. UDT provides its own library implementation and API which
somewhat looks like a socket API, but varies in significant ways.

With Beast's current design, trying to use Beast with the UDT API is
going to be unnatural and awkward. If Beast were instead a reusable,
generic, HTTP related utility library, it would be a much better library
for it.


- The choice to use STL allocators I feel is a code smell. I think a
correct HTTP primitive library would never allocate memory, and thus
never need to call a STL allocator. I think all the places where Beast
does allocate memory were driven by the hard dependency on ASIO, and if
you removed ASIO entirely, you would remove any need for memory allocation.


- I think basic_parser should be designed to use forward iterators
instead of requiring random access, or accept partial input in chunks
whilst keeping state. This would allow it to work seamlessly with
discontiguous underlying storage.


- I appreciate that this my final negative will not be widely agreed
with on boost-dev, but personally speaking I think Beast should work
well with C++ exceptions disabled, and therefore make no use of
exception throws at all. There are lots of end users who would like to
speak HTTP without enabling C++ exceptions for cultural or safety
validation reasons: embedded devices, IoT, games, medical devices etc.
Beast is not useful to any of those use cases if it insists on ASIO and
throws exceptions, neither of which are necessary nor make for a better
HTTP utilities library.



So to sum up, I like the library, most of the design choices are
sensible, I think it would save me a lot of hassle when implementing
HTTP as-is. But I feel it is arse-about-face in design: a simpler, less
cluttered, no-ASIO, no-malloc, less-templates design would be much
better again.

I'm not going to issue a final review now. I specifically want to see
what some others say first, most specifically Bjorn Reese who knows this
area very, very deeply and indeed mentored Boost.Http over multiple
summers. He likely will have a different weighting of priorities to me,
and he's far more a domain expert in this than I am, so his review will
influence mine.

I would also be interested in seeing the proposer's opinion on
everything I've just said before I issue a formal review. I do want to
close with saying that it's a great library Vinnie, you did a great job.
But I think a different design focus would be a lot better again for its
given scope and remit.

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
Vinnie Falco via Boost
2017-07-01 16:02:59 UTC
Permalink
On Sat, Jul 1, 2017 at 7:46 AM, Niall Douglas via Boost
<***@lists.boost.org> wrote:
> + This library is far better quality than when I last looked at it which
> was actually probably a long time ago. Congrats to Vinnie on that. It
> ain't easy getting it to this far.

Thanks! A lot of work went into it in the last couple of months, and
the users chipped in with bug reports and helpful suggestions.

> - For this collection of HTTP-related library routines...

It seems that WebSocket is the red-headed stepchild that no one talks
about. Of course, I understand that the HTTP portions of Beast are far
more interesting (controversial?) so I guess we'll be talking about
that for most of the review.

> ...which is what this library actually is

I think the author of Beast might have some idea of "what this library
actually is", especially when it is stated in the documentation:
<http://vinniefalco.github.io/beast/beast/using_http.html>

> I do not see why any awareness of ASIO is needed at all.

Also stated in the documentation:
"...using the consistent asynchronous model of Boost.Asio."
<http://vinniefalco.github.io/beast/beast/introduction.html>

> I don't get why it's needed,

Boost.Asio is effectively what will become the standard way to do C++
networking (i.e. N4588). Beast recognizes that C++ will soon have a
common, cross-platform networking interface and builds on that
interface exclusively.

> and moreover, I think the library would be far better
> off removing all awareness of a networking implementation entirely.

Beast stream algorithms are not dependent on any networking
implementation. It only depends on a networking interface.
Specifically, Beast stream algorithms require the following concepts:

SyncReadStream
SyncWriteStream
AsyncReadStream
AsyncWriteStream
ConstBufferSequence
MutableBufferSequence
DynamicBuffer

It is entirely possible to create your own objects which meet these
requirements that do not use Asio sockets. For example, a libuv socket
can be wrapped in a manner that makes it usable with Beast. None of
the concepts above require a particular networking implementation,
subject to the following caveat:

The Boost.Asio requirements for the stream and buffer concepts listed
above DO require Asio. Specifically they require the following
concrete types (plus a few more related to asynchronous initiating
function customization points which I won't bore readers with):

boost::asio::io_service
boost::asio::const_buffer
boost::asio::mutable_buffer

At the moment, Beast is tied to Boost.Asio because of the
aforementioned types. However, N4588 introduces significant
improvements to the networking interface. The concrete io_service
class is replaced with a new concept called an Executor. The buffer
requirements are relaxed, instead of requiring a concrete type such as
const_buffer it is possible to use any type that offers a data() and
size() member. Thus, N4588 removes the last vestiges of implementation
dependence on code written for it.

My plan for Beast is to port it to N4588 as soon as it becomes
available in an official capacity in the standard libraries used by
clang, gcc, or msvc. So to answer your question, Boost.Asio is needed
today because N4588 is not part of the standard. When it becomes
standardized, Beast will be dependent on the C++ Standard Library
networking interface (interface, not implementation as you stated).

> Most of the routines Beast supplies that I examined would be more reusable,
> easier to use, and less templatey if they made zero assumptions about what
> networking implementation is in use.

beast::http::serializer and beast::http::basic_parser make no
assumptions about what networking implementation is in use. Perhaps
you can demonstrate a sample program using those Beast classes which
makes zero assumptions about which networking implementation is in
use, that works with at least two different implementations; say,
Boost.Asio and libUV?

Personally, I don't see a point to investing resources into supporting
networking implementations which will not become part of the C++
Standard Library.

> WG21 has much superior vocabulary types for doing buffer sequences than ASIO's
> which are needlessly complex, over engineered, and over wraught.

I intend to make sure Beast is compatible with the standard so
whatever buffer sequences make it in to the standard, is what Beast
will use. If you believe that the buffer sequence concepts in N4588
are deficient, I strongly advise you to open a LEWG issue before the
committee makes a terrible mistake! There's still time.

> A new library should not repeat the design mistakes of ASIO unless it has to,
> and I see no compelling reason given the limited implementation of HTTP
> offered by Beast to have any dependency on ASIO at all.

It is only Beast's stream algorithms which use Boost.Asio's stream concepts.

> The ASIO design model should be replaced with a more
> generic reusable design instead, one which works with any arbitrary
> socket API - or indeed, non-socket API.

You can always write your own stream algorithm using
beast::http::basic_parser and beast::http::serializer which do not
work with streams at all and do not require Boost.Asio, subject to the
following caveat:

These Beast classes use Boost.Asio's buffer concepts which are tied to
a couple of concrete types. This dependence will disappear in the port
to N4588, which does not mandate concrete types.

> With Beast's current design, trying to use Beast with the UDT API is
> going to be unnatural and awkward. If Beast were instead a reusable,
> generic, HTTP related utility library, it would be a much better library
> for it.

I'm not sure I agree, you can always implement your own stream
algorithm which uses UDT API and use beast::http::basic_parser and
beast::http::serializer with it. The documentation even provides
examples of how you can use the serializer and parser with
std::ostream and std::istream, bypassing Asio streams entirely:

http://vinniefalco.github.io/beast/beast/using_http/buffer_oriented_serializing.html#beast.using_http.buffer_oriented_serializing.write_to_std_ostream
http://vinniefalco.github.io/beast/beast/using_http/buffer_oriented_parsing.html#beast.using_http.buffer_oriented_parsing.read_from_std_istream

> The choice to use STL allocators I feel is a code smell.

Beast intends to use standard concepts. Allocator and
AllocatorAwareContainer are concepts defined by the C++ standard, and
the universally understood models for allocator customization points.
If you feel that "STL allocators" are a "code smell" then I urge you
to submit a paper to WG21 with actionable advice (ideally, proposed
wording) on how to banish the stench.

> I think a correct HTTP primitive library would never allocate memory,
> and thus never need to call a STL allocator.

beast::http::serializer never allocates memory.
beast::http::basic_parser never allocates memory when its input is a
single buffer. An optimized allocator is provided in the
http-server-fast example which makes basic_fields never allocate from
the heap:
https://github.com/vinniefalco/Beast/blob/78a065ba39836d91d7e70d93de7f9140f518083b/example/http-server-fast/fields_alloc.hpp#L86

Beast also comes with an extensive set of tools to empower users to
avoid memory allocations:

http://vinniefalco.github.io/beast/beast/ref/beast__http__basic_dynamic_body.html
http://vinniefalco.github.io/beast/beast/ref/beast__http__string_view_body.html
http://vinniefalco.github.io/beast/beast/ref/beast__static_buffer.html
http://vinniefalco.github.io/beast/beast/ref/beast__static_buffer_n.html
http://vinniefalco.github.io/beast/beast/ref/beast__static_string.html

Here is a declaration for a message which performs no dynamic
allocations, which allows a a header of up to 4096 bytes and a body of
up to 16384 bytes:

#include <beast/core.hpp>
#include <beast/http.hpp>
#include <example/http-server-fast/fields_alloc.hpp>

fields_alloc fa{4096};

beast::http::response<
beast::http::basic_dynamic_body<beast::static_buffer_n<16384>,
beast::http::basic_fields<fields_alloc>> res{
std::piecewise_construct, std::make_tuple(), std::make_tuple(fa)};

> - I appreciate that this my final negative will not be widely agreed
> with on boost-dev, but personally speaking I think Beast should work
> well with C++ exceptions disabled, and therefore make no use of
> exception throws at all.

The DynamicBuffer concept prescribes throwing std::length_error if the
buffer maximum size is exceeded:
<http://vinniefalco.github.io/beast/beast/concept/DynamicBuffer.html>

This is a N4588 concept and will eventually become part of the C++
standard. If you feel that N4588 is deficient because it has a concept
which mandates exceptions, I urge you to open a LEWG issue before the
committee makes a terrible mistake! There's still time.

> There are lots of end users who would like to
> speak HTTP without enabling C++ exceptions for cultural or safety
> validation reasons: embedded devices, IoT, games, medical devices etc.
> Beast is not useful to any of those use cases if it insists on ASIO and
> throws exceptions, neither of which are necessary nor make for a better
> HTTP utilities library.

beast::http::serializer and beast::http::basic_parser do not throw exceptions.

Thanks

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Niall Douglas via Boost
2017-07-01 18:08:11 UTC
Permalink
On 01/07/2017 17:02, Vinnie Falco via Boost wrote:
>> - For this collection of HTTP-related library routines...
>
> It seems that WebSocket is the red-headed stepchild that no one talks
> about. Of course, I understand that the HTTP portions of Beast are far
> more interesting (controversial?) so I guess we'll be talking about
> that for most of the review.

Beast WebSocket belongs in another, separate library. That library could
bring in ASIO as a dependency.

>> I do not see why any awareness of ASIO is needed at all.
>
> Also stated in the documentation:
> "...using the consistent asynchronous model of Boost.Asio."
> <http://vinniefalco.github.io/beast/beast/introduction.html>
>
>> I don't get why it's needed,
>
> Boost.Asio is effectively what will become the standard way to do C++
> networking (i.e. N4588). Beast recognizes that C++ will soon have a
> common, cross-platform networking interface and builds on that
> interface exclusively.

I snipped all the hand waving as irrelevant to the question I posed,
which it is.

You didn't answer the question. These are the stated things Beast provides:

1. Message containers

2. Stream reading

3. Stream writing

4. Serialisation

5. Parsing

... which seem a reasonable and useful subset of HTTP to implement.

You have told me no reason why any of these needs a hard dependency on
any networking implementation, or awareness of any specific networking
design pattern.

I personally can see no good reason why they should be: **HTTP has
nothing to do with networking**. It therefore seems to me that reading
ought to be calculated views of underlying byte data, and writing ought
to be compositing a sequence of byte spans to gather into a send. Like
the Ranges TS does.

Please convince me that I am wrong.

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
Vinnie Falco via Boost
2017-07-01 19:01:16 UTC
Permalink
On Sat, Jul 1, 2017 at 11:08 AM, Niall Douglas via Boost
<***@lists.boost.org> wrote:
> Beast WebSocket belongs in another, separate library. That library could
> bring in ASIO as a dependency.

Beast is provided as a coherent package of components that work well
together, with integrated tests and documentation. It is presented as
a single library.

> 1. Message containers
> 2. Stream reading
> 3. Stream writing
> 4. Serialisation
> 5. Parsing
> You have told me no reason why any of these needs a hard dependency on
> any networking implementation, or awareness of any specific networking
> design pattern.

(repeating myself)

Beast's message containers, serialization, and parsing do not depend
on any specific networking interface.

I do not know of a way to write an algorithm which works with an
unspecified stream concept, so I had to choose which concepts I wanted
to work with. I chose these Boost.Asio concepts because they are the
closest thing to becoming a standard:

SyncReadStream
SyncWriteStream
AsyncReadStream
AsyncWriteStream
DynamicBuffer

Perhaps you can demonstrate how a network algorithm may be written
which works with an unspecified stream concept? How about a simple,
synchronous function that writes a string, I'll start you off with a
function signature:

template<class Stream>
void write(Stream& stream, std::string_view string);

Please implement this function for us and demonstrate how it works
with Boost.Asio, Networking-TS, POSIX sockets, WinSock, UDT, or libUV
without modification.

> **HTTP has nothing to do with networking**.

Beast offers algorithms to serialize and parse HTTP messages on
Boost.Asio streams. If you think that is not part of "HTTP" that's
fine, the label is unimportant. What is important is that Beast offers
this functionality.

> It therefore seems to me that reading ought to be calculated views of
> underlying byte data, and writing ought to be compositing a sequence of
> byte spans to gather into a send.

Correct. You have described the operations performed by
beast::http::serializer and beast::http::basic_parser, which do not
require any networking interface.

> WG21 has much superior vocabulary types for doing buffer sequences
> than ASIO's which are needlessly complex, over engineered, and over wraught

Please specify the WG21 vocabulary types you are referring to.

Thanks

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Niall Douglas via Boost
2017-07-01 20:38:15 UTC
Permalink
On 01/07/2017 20:01, Vinnie Falco via Boost wrote:
> On Sat, Jul 1, 2017 at 11:08 AM, Niall Douglas via Boost
> <***@lists.boost.org> wrote:
>> Beast WebSocket belongs in another, separate library. That library could
>> bring in ASIO as a dependency.
>
> Beast is provided as a coherent package of components that work well
> together, with integrated tests and documentation. It is presented as
> a single library.

Except, it is not coherent. You're bundling socket abstraction in with
HTTP parsing and inflicting huge, header-only dependencies on all end
users irrespective of what parts they actually use or need.

At the very minimum, I think Beast needs to become two, separate
libraries: (i) the HTTP utility library (ii) WebSocket.

>> 1. Message containers
>> 2. Stream reading
>> 3. Stream writing
>> 4. Serialisation
>> 5. Parsing
>> You have told me no reason why any of these needs a hard dependency on
>> any networking implementation, or awareness of any specific networking
>> design pattern.
>
> (repeating myself)
>
> Beast's message containers, serialization, and parsing do not depend
> on any specific networking interface.

So why are you forcing end users to drag in ASIO?

The reason why according to you is for the buffer infrastructure. But as
I've already told you, that's a relic from a decade ago. New code
neither ought to nor needs to use that. We have far better available today.

And if the really reusable parts of Beast, the ones not dependent on
ASIO except for some buffer adapters, can be broken off and be made free
of ASIO, that's a big value add to end users who don't need WebSocket
and just want a HTTP utilities library.

> I do not know of a way to write an algorithm which works with an
> unspecified stream concept, so I had to choose which concepts I wanted
> to work with. I chose these Boost.Asio concepts because they are the
> closest thing to becoming a standard:
>
> SyncReadStream
> SyncWriteStream
> AsyncReadStream
> AsyncWriteStream
> DynamicBuffer
>
> Perhaps you can demonstrate how a network algorithm may be written
> which works with an unspecified stream concept? How about a simple,
> synchronous function that writes a string, I'll start you off with a
> function signature:
>
> template<class Stream>
> void write(Stream& stream, std::string_view string);

You're thinking in terms of i/o. Stop doing that. Very little in your
library has, or ought to have, anything to do with i/o. It's the major
flaw in your library's design as I see it.

Think in terms of blobs of bytes. Blobs of bytes come in. Blobs of bytes
go out. No i/o. No reading, no writing. Just blobs of bytes.

>> **HTTP has nothing to do with networking**.
>
> Beast offers algorithms to serialize and parse HTTP messages on
> Boost.Asio streams. If you think that is not part of "HTTP" that's
> fine, the label is unimportant. What is important is that Beast offers
> this functionality.

i/o, ASIO, networking and all this stuff has confounded the clarity and
intent of your design which is very solid.

Stop thinking in terms of i/o. HTTP is just structured data. So treat it
as such. Parse as structured data, generate as structured data.

The natural split point for Beast into multiple, focused libraries is
between the code which only concerns itself with structured data, and
everything else.

>> WG21 has much superior vocabulary types for doing buffer sequences
>> than ASIO's which are needlessly complex, over engineered, and over wraught
>
> Please specify the WG21 vocabulary types you are referring to.

The Ranges TS is an obvious place to start from as a source of Concepts
and vocabulary types to draw from. You don't actually need a Ranges TS
as all your Views, InputRanges, OutputRanges etc. will be of char or
const char anyway. These are very easy to knock together, indeed my GSoC
student this year Tom knocked together a constexpr implementation of
those in less than a week. When the Ranges TS is available in your
compiler, of course patch in that if the end user configures a macro for
it, but you can write just enough of a Ranges TS implementation to suit
all your needs very quickly.

If that's too experimental for you (though the Ranges TS is a TS just
like the Networking TS), then do as Boost.Spirit does and work with
iterator pairs. So for example, if I want to know what the
Content-Length header is, upon query I get back an iterator pair
pointing to the front of the storage and one after the end of the
storage. Similarly, if I set the Content-Length header, I supply an
iterator pair to the new contents.

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
Vinnie Falco via Boost
2017-07-01 21:15:38 UTC
Permalink
On Sat, Jul 1, 2017 at 1:38 PM, Niall Douglas via Boost
<***@lists.boost.org> wrote:
> At the very minimum, I think Beast needs to become two, separate
> libraries: (i) the HTTP utility library (ii) WebSocket.

Beast is proposed as-is.

> The natural split point for Beast into multiple, focused libraries is
> between the code which only concerns itself with structured data, and
> everything else.

Beast is proposed as-is.

> So why are you forcing end users to drag in ASIO?

Beast will be part of Boost, whose distributions include Asio. To use
serializer or basic_parser, the header <boost/asio/buffer.hpp> must
be included, which is a pretty self-contained file. This is a
temporary situation until Boost.Asio is updated to N4588.

> The reason why according to you is for the buffer infrastructure. But as
> I've already told you, that's a relic from a decade ago. New code
> neither ought to nor needs to use that. We have far better available today.

That "relic from a decade ago" is in the latest Boost 1.65.0. As I
have targeted Boost.Asio specifically, you will naturally understand
that Beast uses that buffer infrastructure for better or for worse.
When a version of Boost.Asio appears which is "far better", then Beast
will be ported to it.

The stand-alone Asio is already a bit ahead of Boost.Asio in its
buffer technologies; since you feel that Boost.Asio is so far behind
perhaps you can take on the task of porting stand-alone Asio's changes
to Boost?

> And if the really reusable parts of Beast, the ones not dependent on
> ASIO except for some buffer adapters, can be broken off and be made free
> of ASIO, that's a big value add to end users who don't need WebSocket
> and just want a HTTP utilities library.

Eliminating the dependence on Asio's buffers is something that is on
my horizon, but it is not a feature for the library being proposed.
There hasn't been much demand for using the parser or serializer
without <boost/asio/buffer.hpp>. But if that were to change, I would
likely reprioritize the feature. A port to N4588 will automatically
solve the problem.

>>> WG21 has much superior vocabulary types for doing buffer sequences
>>> than ASIO's which are needlessly complex, over engineered, and over wraught
>>
>> Please specify the WG21 vocabulary types you are referring to.
>The Ranges TS is an obvious place to start from as a source of Concepts

Until you file the LEWG issue where you describe N4588 buffer sequence
concepts as "needlessly complex, over engineered, and over
wraught[sic]" and provide an alternative in the form of proposed
language, there is nothing actionable in your statement.

> Very little in your library has, or ought to have, anything to do with i/o.
> ..
> Stop thinking in terms of i/o. HTTP is just structured data. So treat it
> as such. Parse as structured data, generate as structured data.

Is my understanding correct that you say Beast should not provide
functions to send and receiving HTTP messages using Asio streams?

Thanks

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Niall Douglas via Boost
2017-07-02 15:55:59 UTC
Permalink
>> So why are you forcing end users to drag in ASIO?
>
> Beast will be part of Boost, whose distributions include Asio. To use
> serializer or basic_parser, the header <boost/asio/buffer.hpp> must
> be included, which is a pretty self-contained file. This is a
> temporary situation until Boost.Asio is updated to N4588.

If the dependency is so limited, then remove it entirely and split Beast
into non-ASIO and ASIO parts.

Most of Beast is a structured data utility library focused on HTTP. If
you made the parser capable of discontiguous input (iterator input) and
it could handle more gracefully partial/incomplete input than it
currently does, I would think it pretty much ideal and a superb
foundation for everything HTTP related for all possible end users.

>> Very little in your library has, or ought to have, anything to do with i/o.
>> ..
>> Stop thinking in terms of i/o. HTTP is just structured data. So treat it
>> as such. Parse as structured data, generate as structured data.
>
> Is my understanding correct that you say Beast should not provide
> functions to send and receiving HTTP messages using Asio streams?

Absolutely correct. A highly reusable and flexible Beast would have a
design where knowledge of ASIO is unimportant. A second, separate
library can then combine the generic, reusable Beast stuff with ASIO to
implement lots of useful things like WebSockets etc. Separate the
concerns, decouple implementation.

I would imagine you have three design choices there:

1. Stop short of i/o, and instead provide source iterator pairs from
which ASIO (or anything else) can construct a gather buffer sequence via
boost::make_iterator_range() which is how you tell ASIO to borrow
instead of copy a gather buffer sequence. It is on the end user to pump
any state machine or sequence or reactor.

2. Provide a "tick function" for any reactor in use by the user to pump
HTTP. Study the Qt networking framework as a comparator to ASIO (it's
pretty similar).

3. Provide a generic method of wrapping some arbitrary third party
networking implementation, so Beast provides async_read(), async_write()
etc as now, the HTTP layer does its thing, and the underlying i/o
implementation gets called eventually. The classic - if ugly -
implementation of this is OpenSSL via its BIO mechanism. You can do far
better in C++ 11.


Me, personally speaking, I would favour option 1. It's the least work,
most flexible for end users with whatever weird i/o transport they might
be using, and seems most in keeping with the overall library design
philosophy of minimal but powerful abstraction.

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
Vinnie Falco via Boost
2017-07-02 15:56:41 UTC
Permalink
On Sun, Jul 2, 2017 at 8:55 AM, Niall Douglas via Boost
<***@lists.boost.org> wrote:
> ...

> Very little in your library has, or ought to have, anything to do with i/o.

Is my understanding correct that you say Beast should not provide
functions to send and receive HTTP messages using Asio streams?

Thanks

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Felipe Magno de Almeida via Boost
2017-07-03 01:38:43 UTC
Permalink
On Sun, Jul 2, 2017 at 12:56 PM, Vinnie Falco via Boost
<***@lists.boost.org> wrote:
> On Sun, Jul 2, 2017 at 8:55 AM, Niall Douglas via Boost
> <***@lists.boost.org> wrote:
>> ...
>
>> Very little in your library has, or ought to have, anything to do with i/o.
>
> Is my understanding correct that you say Beast should not provide
> functions to send and receive HTTP messages using Asio streams?

I would very much appreciate if Asio dependency could be contained
in some headers which I could avoid #include'ing. I actually patched
a specific version of Beast so that was possible. So I could use it
in microcontrollers where sockets don't even exist.

> Thanks

Regards,
--
Felipe Magno de Almeida

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Vinnie Falco via Boost
2017-07-02 16:00:19 UTC
Permalink
On Sun, Jul 2, 2017 at 8:55 AM, Niall Douglas via Boost
<***@lists.boost.org> wrote:
>> Is my understanding correct that you say Beast should not provide
>> functions to send and receiving HTTP messages using Asio streams?
>
> Absolutely correct.

This opinion is so far out of touch with the feedback I have gotten
from users about what they want, that it raises the question whether
your screeds have any value.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Niall Douglas via Boost
2017-07-02 16:07:30 UTC
Permalink
>>> Is my understanding correct that you say Beast should not provide
>>> functions to send and receiving HTTP messages using Asio streams?
>>
>> Absolutely correct.
>
> This opinion is so far out of touch with the feedback I have gotten
> from users about what they want, that it raises the question whether
> your screeds have any value.

Ok, Vinnie. Enough of the aggression. You're here for feedback on your
library's design and implementation. If you simply want to be told your
library is great and welcome to Boost, you're in the wrong place.

If you don't want my feedback because of some grudge against me, say so
and you will see nothing more from me until my final review. Stop with
the nasty asides.

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
Vinnie Falco via Boost
2017-07-02 23:08:25 UTC
Permalink
On Sun, Jul 2, 2017 at 9:07 AM, Niall Douglas via Boost
<***@lists.boost.org> wrote:
> Ok, Vinnie. Enough of the aggression.
> ...
> If you don't want my feedback because of some grudge against me, say so

There's no grudge but this is what I'm hearing:

"Beast should not perform socket I/O."

I don't know if its your style of speaking, or word choice but this
feedback is patently absurd on the face of it. So you think users
shouldn't be able to implement a web server or HTTP client using
Beast? How does one even begin to respond to that? I'm going to push
back considerably on statements of the form "your library shouldn't do
what it does" such as yours.

I want to give you the benefit of the doubt, maybe you're trying to
say something else such as, the library should be presented as two
separate libraries? But that's not how it sounds.

Thanks

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Niall Douglas via Boost
2017-07-03 00:00:21 UTC
Permalink
On 02/07/2017 18:25, Vinnie Falco wrote:
> On Sun, Jul 2, 2017 at 9:07 AM, Niall Douglas via Boost
> <***@lists.boost.org> wrote:
>> Ok, Vinnie. Enough of the aggression.
>> ...
>> If you don't want my feedback because of some grudge against me, say so
>
> There's no grudge but this is what I'm hearing:
>
> "Beast should not perform socket I/O."
>
> I don't know if its your style of speaking, or word choice but this
> feedback is patently absurd on the face of it.

I don't see the absurdity at all.

Let me give you some background.

I've implemented basic HTTP perhaps six times now in my career to date.
Four times in C/C++, twice in Python. I don't have the depth of
knowledge of full fat HTTP like say Bjorn does, seeing as he contributed
extensively to curl, but I've implemented this stuff lots of times now.

And how you've designed Beast is close to my earlier HTTP
implementations, and far from my later HTTP implementations. This is why
I keep asking about your fundamental design assumptions. I think you
evolved your design starting from the assumption of socket i/o, when you
should have started from blobs of structured data in, blobs of
structured data out. I know for a fact that's suboptimal because I've
been there before too.

> So you think users
> shouldn't be able to implement a web server or HTTP client using
> Beast? How does one even begin to respond to that?

Your userbase to date suffers from selection bias. They want something
which works with ASIO, so they use Beast because it ticks that box. I
can assure you that for every user you have gained to date, you lost at
least two, maybe three users who looked at that ASIO dependency and
thought "god no, not worth the hassle" because they also know how
painful it is to get a custom socket implementation to work with ASIO.

I also know that you'll never, ever encompass all the weird data
transports your full potential user base will have. All sorts of weird
custom, often proprietary things. So free your library of the ASIO
dependency. You don't need it, indeed it's damaging you. Design your
library to work with any unknown transport instead. For example,
switched banks of reference counted shared memory with the current one
selected by a std::atomic<char *>. Blobs of structured data in, blobs of
structured data out - that's your only requirement.

As I've mentioned on list now several times, ASIO readily will consume
any custom gather buffer sequence type: simply specialise the
appropriate ASIO free functions and traits and you can feed it
beast::const_raw_buffer which is a const char * and a size_t (or still
better, a span<const char> or a string_view). So your library's users
can #include <boost/beast/asio_buffer_adapters.hpp> or something and
thereafter directly feed Beast scatter-gather buffer sequences to ASIO.
Or they can for loop them into an array of struct iovec and call the
equivalent of writev() on whatever weird custom transport they are using.

Point is, you need to be _facilitating_ i/o, not _implementing_ i/o.

> I'm going to push
> back considerably on statements of the form "your library shouldn't do
> what it does" such as yours.

I think your library should focus on making HTTP easy for everybody with
a need for HTTP. Do one thing really well, it's where Boost.Http fell down.

> I want to give you the benefit of the doubt, maybe you're trying to
> say something else such as, the library should be presented as two
> separate libraries? But that's not how it sounds.

BTW, just so you know, I'm currently on a conditional accept with a low
confidence interval. It's why I'm interrogating your fundamental design
choices: as much as the _correct_ HTTP library design allows things like
switched banks of shared memory as the transport, such flexibility is
not _necessary_ for a HTTP implementation. And where I am stuck is on
what weighting to give these imperatives, you must remember it's not
just you and me going back and forth. Lots of other people are likely as
stuck on this as me and are reading everything on boost-dev. So your
justifications of design choice to me, and persuasion that your choices
are not showstoppers, are what gets a library accepted.

After all I'm definitely not the only person who has implemented HTTP
half a dozen times, and is likely looking at your library and wondering
about the design assumptions too. It's just they haven't said it yet
because our dialogue was answering their questions. So even if you and
me stop, you may well see the exact same themes - though perhaps more
tactfully put than from I - being raised anyway.

I particularly strongly weigh Bjorn's opinion, we used to work at the
same employer for a bit where we developed a replacement for TCP
extending ASIO, and where HTTP was going to be deployed over that.
Interestingly Beast would likely have worked just fine on that as it was
ASIO based, but we very nearly didn't choose ASIO as the base, in which
case Beast would have been useless to us. So if he greenlights you, I'm
happy, if not, then you should take the feedback he gives you very
seriously. He really knows his stuff.

Niall


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Vinnie Falco via Boost
2017-07-03 00:03:42 UTC
Permalink
On Sun, Jul 2, 2017 at 5:00 PM, Niall Douglas via Boost
<***@lists.boost.org> wrote:
> So free your library of the ASIO dependency.

1. Beast stream operations are built for Boost.Asio.
2. Beast's dynamic buffers and buffer adapters are built for
<boost/asio/buffer.hpp>

This is not going to change. When I port Beast to Net-TS, I will make
similar statements:

1. Beast stream operations are built for Net-TS
2. Beast's dynamic buffers and buffer adapters are built for
<experimental/network/buffers> (or whatever the header is called)

I have no desire to enlarge the scope of Beast beyond that which is
suitable for standardization. Like it or not, Boost.Asio is the
closest we have to something standard, and soon it will be Net-TS
which is practically identical to Asio except for cosmetic difference
in buffer types.

If you feel that Beast should work with non-Asio I/O models then the
venue for that fight is in the LEWG. Beast will track the standard or
as close to it as possible. So if you convince the working group there
is room in the standard for networking I/O models different from
Net-TS, Beast will adopt them as a matter of policy.

If you want to fork Beast and show me how its done you have my
blessing. If your fork became popular I would definitely reconsider.

> ...your justifications of design choice to me, and persuasion that your
> choices are not showstoppers, are what gets a library accepted.

I can't imagine any argument more persuasive than "it tracks standards."

> I think your library should...Do one thing really well

On this we can agree. That "one thing" is "HTTP operations on
Boost.Asio stream concepts."

...and WebSockets too! Why do they neglect the websockets? Its like
the kid who always gets picked last for the team at P.E.

Thanks

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Niall Douglas via Boost
2017-07-03 00:17:55 UTC
Permalink
(note to this reply: on his request, Vinnie and I have replayed an
offlist conversation we had back onto boost-dev in order to get
feedback. Hence the tone adopted is a bit weird and I'm talking about
boost-dev as a third party, which makes no sense if I were writing this
to here. Be aware I have edited this message somewhat over the original
as this goes on the public record, it is not identical to the original)


> I have no desire to enlarge the scope of Beast beyond that which is
> suitable for standardization.

I would be absolutely amazed if HTTP hard coded for the Networking TS
could be standardised.

There are tons of big C++ users out there who would rightly veto such a
thing. Qt provides an extensive networking implementation which does not
work well with ASIO. So does Apple, Google and Microsoft, all of whom
have substantial proprietary networking implementations, and whose WG21
reps would veto such a proposal in my opinion.

If you are aiming for Beast to be standardised, I think you are
absolutely dead in the water with the current design. I had no idea
until now that you intended that, and I really think you need to tell
the Boost peer review that, because I don't think anyone else realises
that either.

If they do realise that, they'll very considerably raise the bar to pass
review like how they treated Outcome and AFIO, both of which were
advertised to be intended for standards. So I won't mention it, it's up
to you. But if you want Beast into the C++ standard, you really ought to
say that's your plan now as an announcement. You'll get a much more
useful to you review, even if that means a rejection. I found both the
Outcome and AFIO reviews enormously useful.

> If you feel that Beast should work with non-Asio I/O models then the
> venue for that fight is in the LEWG. Beast will track the standard or
> as close to it as possible. So if you convince the working group there
> is room in the standard for networking I/O models different from
> Net-TS, Beast will adopt them as a matter of policy.

That's not how standards work.

WG21 had to pick *some* networking implementation. It was getting
embarrassing for C++ to not have one. But by choosing ASIO, in no
possible way will that choice be allowed by major C++ users with
representation on WG21 to damage the existing ecosystem of diverse and
very popular other networking implementations, most of whom are vastly
more popular in user count than ASIO is.

From what I've been told, there is a fair whack of people on WG21 who
see ASIO as the interim Networking v1, with a strong expectation that a
v2 will replace it once they've done iostreams v2 and built out a proper
general purpose i/o layer for C++. So hard coding HTTP to Networking v1
would be foolish, it upsets the proprietary vendors many of whom are
hoping to get their proprietary system into the standard in years to
come, and it's a waste of work when Networking v2 may happen and you'd
like your HTTP implementation to work on both. What you'll then see is
repeated deferment of decision, it'll keep getting kicked down the road
and it'll never happen.

But listen, don't take my word for it. I've only been tangentially
involved in standards for a decade or so, and less with WG21 than other
WGs. Ask someone like Bjarne, he's got a vision of where i/o in C++
needs to go next. And I understand that he is not exactly keen on ASIO's
API design.

Niall


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Vinnie Falco via Boost
2017-07-03 00:34:37 UTC
Permalink
On Sun, Jul 2, 2017 at 5:17 PM, Niall Douglas via Boost
<***@lists.boost.org> wrote:
> (note to this reply: on his request, Vinnie and I have replayed an
> offlist conversation we had back onto boost-dev in order to get
> feedback.

Thank you. The conversation was of sufficient importance to merit an
on-list discussion.

> I really think you need to tell the Boost peer review that, because
> I don't think anyone else realises that either.

Did anyone not realize that Beast's roadmap includes a standardization
proposal? If so then the documentation and my mailing list comments
have done a poor job...as Prego say's "It's in there!"

> ...if you want Beast into the C++ standard, you really ought to
> say that's your plan now as an announcement. You'll get a much more
> useful to you review

See item 5:
<http://vinniefalco.github.io/stage/beast/review/beast/design_choices/faq.html>

> WG21...see ASIO as the interim Networking v1, with a strong expectation that a
> v2 will replace it once they've done iostreams v2

I don't have the same experience and involvement with the C++
luminaries that you have, so I lack an inside track on what will
become part of C++2023 or C++2026. Meanwhile, Beast users want HTTP
solutions today so in the absence of reliable tea leaves I can only
design for what is standard (or close to it).

Thanks

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Niall Douglas via Boost
2017-07-03 01:14:46 UTC
Permalink
>> ...if you want Beast into the C++ standard, you really ought to
>> say that's your plan now as an announcement. You'll get a much more
>> useful to you review
>
> See item 5:
> <http://vinniefalco.github.io/stage/beast/review/beast/design_choices/faq.html>

I freely admit I read that entire document and missed the sentence "it
is the author's goal to propose this library for standardization". I
thought that FAQ item was about your roadmap for supporting standalone
ASIO. Sorry.

>> WG21...see ASIO as the interim Networking v1, with a strong expectation that a
>> v2 will replace it once they've done iostreams v2
>
> I don't have the same experience and involvement with the C++
> luminaries that you have, so I lack an inside track on what will
> become part of C++2023 or C++2026. Meanwhile, Beast users want HTTP
> solutions today so in the absence of reliable tea leaves I can only
> design for what is standard (or close to it).

Do remember I suggested you design for a different part of the standard,
not away from the standard.

I've been preparing the ground and reading the tea leaves for an
eventual AFIO standardisation for some years now by talking to anyone on
WG21 with an opinion regarding i/o. Obviously there is a strong
selection bias in there in my sampling, I may be far off the mark for
the consensus opinion which I presently believe to be mostly
indifference as an iostreams v2 is not seen as high priority right now.
But if say something like Rust or Swift rolled out a provably superior
solution which got headlines, that could change very quickly.

I also would say as a former SC22 mirror convenor that an accurate way
of looking at ISO is that it's the place multinationals send their
people to slow down or prevent standardisation of things which could
harm their interests. A lot of people express frustration with WG21, but
it makes sense when seen as a place mainly to stop and delay change.
That's how ISO configures things, multinationals overwhelmingly dominate
the national votes through being multi national.

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
Vinnie Falco via Boost
2017-07-03 01:27:05 UTC
Permalink
On Sun, Jul 2, 2017 at 6:14 PM, Niall Douglas via Boost
<***@lists.boost.org> wrote:
> Do remember I suggested you design for a different part of the standard,
> not away from the standard.

Is there another networking proposal hiding somewhere that I am not privy to?

Thanks

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Edward Diener via Boost
2017-07-03 02:44:56 UTC
Permalink
On 7/2/2017 8:17 PM, Niall Douglas via Boost wrote:
> (note to this reply: on his request, Vinnie and I have replayed an
> offlist conversation we had back onto boost-dev in order to get
> feedback. Hence the tone adopted is a bit weird and I'm talking about
> boost-dev as a third party, which makes no sense if I were writing this
> to here. Be aware I have edited this message somewhat over the original
> as this goes on the public record, it is not identical to the original)
>
>
>> I have no desire to enlarge the scope of Beast beyond that which is
>> suitable for standardization.
>
> I would be absolutely amazed if HTTP hard coded for the Networking TS
> could be standardised.
>
> There are tons of big C++ users out there who would rightly veto such a
> thing. Qt provides an extensive networking implementation which does not
> work well with ASIO. So does Apple, Google and Microsoft, all of whom
> have substantial proprietary networking implementations, and whose WG21
> reps would veto such a proposal in my opinion.
>
> If you are aiming for Beast to be standardised, I think you are
> absolutely dead in the water with the current design. I had no idea
> until now that you intended that, and I really think you need to tell
> the Boost peer review that, because I don't think anyone else realises
> that either.

No, this sort of thing is irrelevant. Whether any Boost library would
like to be part of the C++ standard or not should not come into
consideration when reviewers look at a library to be added to Boost. In
reality nearly all Boost authors of libraries probably would like their
libraries to be added as a C++ standard library, but this has nothing to
do with the quality of the library itself.

Furthermore, as I follow discussions about Beast, I am pretty sure that
the library author is not saying that, which is meaningless, but instead
is saying that he designed his library to be compatible to what the
Networking group of the C++ standard is considering adding to the C++
standard. You are of course free to disagree with that decision, since
there is nothing inherently sacrosanct on whatever is, or may be, in the
C++ standard. But that has nothing to do with the saying a library would
like to be part of the C++ standard, which no technical value whatsoever.

>
> If they do realise that, they'll very considerably raise the bar to pass
> review like how they treated Outcome and AFIO, both of which were
> advertised to be intended for standards. So I won't mention it, it's up
> to you. But if you want Beast into the C++ standard, you really ought to
> say that's your plan now as an announcement. You'll get a much more
> useful to you review, even if that means a rejection. I found both the
> Outcome and AFIO reviews enormously useful.
>
>> If you feel that Beast should work with non-Asio I/O models then the
>> venue for that fight is in the LEWG. Beast will track the standard or
>> as close to it as possible. So if you convince the working group there
>> is room in the standard for networking I/O models different from
>> Net-TS, Beast will adopt them as a matter of policy.
>
> That's not how standards work.
>
> WG21 had to pick *some* networking implementation. It was getting
> embarrassing for C++ to not have one. But by choosing ASIO, in no
> possible way will that choice be allowed by major C++ users with
> representation on WG21 to damage the existing ecosystem of diverse and
> very popular other networking implementations, most of whom are vastly
> more popular in user count than ASIO is.
>
> From what I've been told, there is a fair whack of people on WG21 who
> see ASIO as the interim Networking v1, with a strong expectation that a
> v2 will replace it once they've done iostreams v2 and built out a proper
> general purpose i/o layer for C++. So hard coding HTTP to Networking v1
> would be foolish, it upsets the proprietary vendors many of whom are
> hoping to get their proprietary system into the standard in years to
> come, and it's a waste of work when Networking v2 may happen and you'd
> like your HTTP implementation to work on both. What you'll then see is
> repeated deferment of decision, it'll keep getting kicked down the road
> and it'll never happen.
>
> But listen, don't take my word for it. I've only been tangentially
> involved in standards for a decade or so, and less with WG21 than other
> WGs. Ask someone like Bjarne, he's got a vision of where i/o in C++
> needs to go next. And I understand that he is not exactly keen on ASIO's
> API design.
>
> Niall


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Emil Dotchevski via Boost
2017-07-03 03:02:58 UTC
Permalink
On Sun, Jul 2, 2017 at 7:44 PM, Edward Diener via Boost <
***@lists.boost.org> wrote:

> On 7/2/2017 8:17 PM, Niall Douglas via Boost wrote:
>
>> (note to this reply: on his request, Vinnie and I have replayed an
>> offlist conversation we had back onto boost-dev in order to get
>> feedback. Hence the tone adopted is a bit weird and I'm talking about
>> boost-dev as a third party, which makes no sense if I were writing this
>> to here. Be aware I have edited this message somewhat over the original
>> as this goes on the public record, it is not identical to the original)
>>
>>
>> I have no desire to enlarge the scope of Beast beyond that which is
>>> suitable for standardization.
>>>
>>
>> I would be absolutely amazed if HTTP hard coded for the Networking TS
>> could be standardised.
>>
>> There are tons of big C++ users out there who would rightly veto such a
>> thing. Qt provides an extensive networking implementation which does not
>> work well with ASIO. So does Apple, Google and Microsoft, all of whom
>> have substantial proprietary networking implementations, and whose WG21
>> reps would veto such a proposal in my opinion.
>>
>> If you are aiming for Beast to be standardised, I think you are
>> absolutely dead in the water with the current design. I had no idea
>> until now that you intended that, and I really think you need to tell
>> the Boost peer review that, because I don't think anyone else realises
>> that either.
>>
>
> No, this sort of thing is irrelevant. Whether any Boost library would like
> to be part of the C++ standard or not should not come into consideration
> when reviewers look at a library to be added to Boost. In reality nearly
> all Boost authors of libraries probably would like their libraries to be
> added as a C++ standard library, but this has nothing to do with the
> quality of the library itself.
>

+1

In C and C++ it is possible to design libraries that are both portable and
low overhead compared to pretty much any other language. While Python may
need all kinds of libraries to do basic things, in C and C++ many times
there is no need for a library to be standardized, there would be little
gain. Consider for example something like libpng or Boost -- they're
available, often preinstalled on pretty much any operating system under the
sun already.

Further, I think it is a bad idea to standardize a library before it has
been deployed for many years outside of the standard, no matter how good it
may be. If it is that good, people will reach and use it anyway, and if not
the standard is better for not including it.

It is especially evil to attempt to standardize a library as means to force
adoption -- again, if the library is better than any other alternative,
people will use it. If not, it has no place in the standard.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Phil Endecott via Boost
2017-07-03 16:37:09 UTC
Permalink
Niall Douglas wrote:
> Qt provides an extensive networking implementation which does not
> work well with ASIO. So does Apple, Google and Microsoft, all of whom
> have substantial proprietary networking implementations

I think this is an important point - on what platforms is Beast
actually useful in practice?

I have some iOS apps, and it's not clear to me to what extent ASIO
works with that platform's event loop, or other important OS features
(for example, if you just use the POSIX sockets API on iOS then you
might find that the radio gets turned off to save power, because the
OS doesn't know that you need it to be kept on). So currently I do
HTTP on iOS using a simple C++ wrapper around Apple's objC NSURLConnection.

We could speculate about what will happen if ASIO gets standardised.
Will Apple (and the others) provide an implementation that works properly
on their platform? Maybe, but I would not bet on it.

How much of Beast is useful without ASIO?


Regards, Phil.





_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Vinnie Falco via Boost
2017-07-03 16:40:19 UTC
Permalink
On Mon, Jul 3, 2017 at 9:37 AM, Phil Endecott via Boost
<***@lists.boost.org> wrote:
> I think this is an important point - on what platforms is Beast
> actually useful in practice?

Wherever Boost.Asio is useful.

> How much of Beast is useful without ASIO?

Not very much, although you can use beast::http::serializer and
beast::http::basic_parser using only <boost/asio/buffer.hpp> which is
pretty small, self contained, and knows nothing about sockets.

There is a point of contention here, one reviewer thinks that
buffer.hpp should not be a dependency. However that discussion is
stalled due to non-technical reasons.

Realistically speaking if you are trying to use Beast without Asio
(modulo buffer.hpp as stated above) you aren't going to get very far
at all.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Emil Dotchevski via Boost
2017-07-03 16:54:00 UTC
Permalink
On Mon, Jul 3, 2017 at 9:40 AM, Vinnie Falco via Boost <
***@lists.boost.org> wrote:

> > How much of Beast is useful without ASIO?
>
> Not very much, although you can use beast::http::serializer and
> beast::http::basic_parser using only <boost/asio/buffer.hpp> which is
> pretty small, self contained, and knows nothing about sockets.
>
> There is a point of contention here, one reviewer thinks that
> buffer.hpp should not be a dependency. However that discussion is
> stalled due to non-technical reasons.
>
> Realistically speaking if you are trying to use Beast without Asio
> (modulo buffer.hpp as stated above) you aren't going to get very far
> at all.
>

The question is must Beast be coupled to Asio, not can it be used without
something like Asio. And I don't mean just in the case of someone using
serializer/basic_parser and nothing else.

In my opinion, arguing that coupling is necessary should begin with
defining an interface which Beast can use, which can be trivially
implemented in terms of Asio. In designing this interface the only concern
should be avoiding the coupling; specifically, performance considerations
should be completely ignored. Only then we can have a reasonable discussion
is it worth it. Regardless, in my experience this kind of exercise always
improves the design of a library.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Zach Laine via Boost
2017-07-03 17:25:13 UTC
Permalink
On Mon, Jul 3, 2017 at 11:54 AM, Emil Dotchevski via Boost <
***@lists.boost.org> wrote:

> On Mon, Jul 3, 2017 at 9:40 AM, Vinnie Falco via Boost <
> ***@lists.boost.org> wrote:
>
> > > How much of Beast is useful without ASIO?
> >
> > Realistically speaking if you are trying to use Beast without Asio
> > (modulo buffer.hpp as stated above) you aren't going to get very far
> > at all.
>
> In my opinion, arguing that coupling is necessary should begin with
> defining an interface which Beast can use, which can be trivially
> implemented in terms of Asio. In designing this interface the only concern
> should be avoiding the coupling; specifically, performance considerations
> should be completely ignored. Only then we can have a reasonable discussion
> is it worth it.


This is a very good way to think about this.


> Regardless, in my experience this kind of exercise always
> improves the design of a library.


This is definitely true.

I think those two points are (mostly?) what Niall was trying to convey.

Zach

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Nelson, Erik - 2 via Boost
2017-07-03 17:42:41 UTC
Permalink
> -----Original Message-----
> From: Boost [mailto:boost-***@lists.boost.org] On Behalf Of Zach Laine
> via Boost
> Sent: Monday, July 03, 2017 1:25 PM
>
> On Mon, Jul 3, 2017 at 11:54 AM, Emil Dotchevski via Boost <
> ***@lists.boost.org> wrote:
>
> > On Mon, Jul 3, 2017 at 9:40 AM, Vinnie Falco via Boost <
> > ***@lists.boost.org> wrote:
> >
> > > > How much of Beast is useful without ASIO?
> > >
> > > Realistically speaking if you are trying to use Beast without Asio
> > > (modulo buffer.hpp as stated above) you aren't going to get very far
> > > at all.
> >
> > In my opinion, arguing that coupling is necessary should begin with
> > defining an interface which Beast can use, which can be trivially
> > implemented in terms of Asio. In designing this interface the only concern
> > should be avoiding the coupling; specifically, performance considerations
> > should be completely ignored. Only then we can have a reasonable
> discussion
> > is it worth it.
>
>
> This is a very good way to think about this.
>

+1


> > Regardless, in my experience this kind of exercise always
> > improves the design of a library.
>
>
> This is definitely true.
>

+1
Emil Dotchevski via Boost
2017-07-03 00:17:56 UTC
Permalink
On Sun, Jul 2, 2017 at 5:00 PM, Niall Douglas via Boost <
***@lists.boost.org> wrote:

> On 02/07/2017 18:25, Vinnie Falco wrote:
> > On Sun, Jul 2, 2017 at 9:07 AM, Niall Douglas via Boost
> > <***@lists.boost.org> wrote:
> >> Ok, Vinnie. Enough of the aggression.
> >> ...
> >> If you don't want my feedback because of some grudge against me, say so
> >
> > There's no grudge but this is what I'm hearing:
> >
> > "Beast should not perform socket I/O."
> >
> > I don't know if its your style of speaking, or word choice but this
> > feedback is patently absurd on the face of it.
>
> I don't see the absurdity at all.
>
> Let me give you some background.
>
> I've implemented basic HTTP perhaps six times now in my career to date.
> Four times in C/C++, twice in Python. I don't have the depth of
> knowledge of full fat HTTP like say Bjorn does, seeing as he contributed
> extensively to curl, but I've implemented this stuff lots of times now.
>

I think Vinnie is right that it is "your style". "I've done HTTP six times
in my career" is not an argument, the subtext is "you're inexperienced and,
this library is a nice try, but if you want to play with the big boys you
have to do it differently."

Further, I know many smart people who have done something six times in
their career, and never got close to a good design.

I'm not an expert so I can't comment on the Beast (other than to say that
IMO it is a poor choice of name), but it would be helpful (including to
people like me) to add structure to your criticisms; e.g. this is what the
library does now, but this is a bad idea for this and that reason, and it
would be better to do it this way, rather than "dude, this is so 10 years
out of date!" :)

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Niall Douglas via Boost
2017-07-03 00:55:54 UTC
Permalink
> I'm not an expert so I can't comment on the Beast (other than to say that
> IMO it is a poor choice of name), but it would be helpful (including to
> people like me) to add structure to your criticisms; e.g. this is what the
> library does now, but this is a bad idea for this and that reason, and it
> would be better to do it this way, rather than "dude, this is so 10 years
> out of date!" :)

I did add considerable structure to what I think he should do instead.
To sum it up:

1. Consume blobs of structured data. Produce blobs of structured data.

2. Facilitate i/o rather than implement i/o.

3. Be inspired by the Ranges TS rich suite of concepts, but failing that
iterator pairs will do.

... and I went into a fair bit of implementation detail of how you'd
arrange things differently, so I mentioned by name what customisation
points in ASIO you'd use to tell ASIO about your types representing a
block of structured data and so on.

What I didn't do was write essays of detail. It would take too long,
it's not my place, and I'm pretty sure Vinnie knows ASIO internals as
well as I do, so I am assuming he understands everything I'm writing.

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
Peter Dimov via Boost
2017-07-03 01:05:28 UTC
Permalink
Niall Douglas wrote:

> What I didn't do was write essays of detail. It would take too long, it's
> not my place, and I'm pretty sure Vinnie knows ASIO internals as well as I
> do, so I am assuming he understands everything I'm writing.

I understand everything you're writing, but the complete picture still
eludes me. Specific examples:

http::request_parser<http::string_body> parser;

parser.header_limit( 8192 );
parser.body_limit( 8192 );

http::read( socket_, buffer, parser, ec );

/*...*/

do_request( parser.get(), ec );

How would this look like in the blobs in, blobs out model?

Same question for the reverse one:

std::vector<unsigned char> data;

/* ... fill data... */

beast::string_view sv( (char*)data.data(), data.size() );
http::response<http::string_view_body> res( sv );

res.result( http::status::ok );

res.set( http::field::server, BEAST_VERSION_STRING );
res.set( http::field::content_type, "image/bmp" );
res.set( http::field::content_length, data.size() );

http::write( socket_, res, ec );

How does the ASIO-decoupled code look?


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Niall Douglas via Boost
2017-07-03 01:43:11 UTC
Permalink
>> What I didn't do was write essays of detail. It would take too long,
>> it's not my place, and I'm pretty sure Vinnie knows ASIO internals as
>> well as I do, so I am assuming he understands everything I'm writing.
>
> I understand everything you're writing, but the complete picture still
> eludes me. Specific examples:
>
> http::request_parser<http::string_body> parser;
>
> parser.header_limit( 8192 );
> parser.body_limit( 8192 );
>
> http::read( socket_, buffer, parser, ec );
>
> /*...*/
>
> do_request( parser.get(), ec );
>
> How would this look like in the blobs in, blobs out model?

Slightly more bare metal because you talk to ASIO by hand, but nothing
too awful I think:

```
char buffer[16384]; // no need to explicitly limit header + body

// Create a HTTP request parsing view of char[16384]
http::request_parser<http::string_body> parser(buffer);

// Read enough data to parse the request
char *p = buffer;
do
{
p += asio::read(socket, asio::buffer(p, buffer + sizeof(buffer) - p));
} while(!parser.has_body(p - buffer));
do_request(parser.get(), ec);

// Use any spill at the end
memcpy(buffer, buffer + parser.size_bytes(), p - (buffer +
parser.size_bytes()));
p = buffer;
...
```

> Same question for the reverse one:
>
> std::vector<unsigned char> data;
>
> /* ... fill data... */
>
> beast::string_view sv( (char*)data.data(), data.size() );
> http::response<http::string_view_body> res( sv );
>
> res.result( http::status::ok );
>
> res.set( http::field::server, BEAST_VERSION_STRING );
> res.set( http::field::content_type, "image/bmp" );
> res.set( http::field::content_length, data.size() );
>
> http::write( socket_, res, ec );
>
> How does the ASIO-decoupled code look?

```
std::vector<unsigned char> data;

// Create a response borrowing 'data' as a single contiguous
// gather buffer, known from it matching ContiguousContainer concept
http::response res(data);

// These prepend additional gather buffers for the headers
res.result( http::status::ok );
res.set( http::field::server, BEAST_VERSION_STRING );
res.set( http::field::content_type, "image/bmp" );
res.set( http::field::content_length, data.size() );

// Tell ASIO to send the response by gathering all the buffers
asio::write(socket, boost::make_iterator_range(res.begin(), res.end()));
```

The latter used a bit of C++ 17 template inferencing and has some
unwritten type trait specialisation and free function overloads to tell
ASIO about a custom `beast::buffer` type pointed to by the iterators,
but otherwise it should be fairly obvious.

Do ask any questions if it isn't actually obvious :). It's nearly 3am
here so my obviousness radar is probably not working well.

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
Vinnie Falco via Boost
2017-07-03 01:55:23 UTC
Permalink
On Sun, Jul 2, 2017 at 6:43 PM, Niall Douglas via Boost
<***@lists.boost.org> wrote:
> char buffer[16384]; // no need to explicitly limit header + body
>
> // Create a HTTP request parsing view of char[16384]
> http::request_parser<http::string_body> parser(buffer);
>
> // Read enough data to parse the request
> char *p = buffer;
> do
> {
> p += asio::read(socket, asio::buffer(p, buffer + sizeof(buffer) - p));
> } while(!parser.has_body(p - buffer));
> do_request(parser.get(), ec);
>
> // Use any spill at the end
> memcpy(buffer, buffer + parser.size_bytes(), p - (buffer +
> parser.size_bytes()));
> p = buffer;

You're asking for beast::http::parser to not use <boost/asio/buffer.hpp>

> // These prepend additional gather buffers for the headers
> res.result( http::status::ok );
> res.set( http::field::server, BEAST_VERSION_STRING );
> res.set( http::field::content_type, "image/bmp" );
> res.set( http::field::content_length, data.size() );
>
> // Tell ASIO to send the response by gathering all the buffers
> asio::write(socket, boost::make_iterator_range(res.begin(), res.end()));

This is in no way equivalent to what Beast does now, I suggest you
study BodyReader and http::serializer.

Still, it sounds like again you are simply asking that
http::serializer not use <boost/asio/buffer.hpp>

I will solicit help (not from you) to see if this problem can be
solved in a fashion that remains Asio friendly and also preserves
Beast's features (your examples don't, but you are unfamiliar with the
library so that is understandable).

Thanks

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Vinnie Falco via Boost
2017-07-03 02:07:08 UTC
Permalink
On Sun, Jul 2, 2017 at 6:43 PM, Niall Douglas via Boost
<***@lists.boost.org> wrote:
> It's nearly 3am here...

I looked once again at http::basic_parser and http::serializer. It
might be possible to refactor http::basic_parser into a separate class
and with some mess in http::parser support two versions. One which
allows ConstBufferSequence and the other which allows just pair<void*,
std::size>.

http::serializer is another story entirely. It contains and is tied to
adapters which contain Asio buffer types. And it uses Asio buffer
algorithms.

You've taken the position that this is an easy matter, that you have
extensive knowledge and experience in the domain, so I would kindly
ask that you provide a working prototype of http::serializer which
does not use Asio buffer types and yet remains compatible with the
rest of Beast.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Niall Douglas via Boost
2017-07-03 12:33:57 UTC
Permalink
On 03/07/2017 03:07, Vinnie Falco via Boost wrote:
> On Sun, Jul 2, 2017 at 6:43 PM, Niall Douglas via Boost
> <***@lists.boost.org> wrote:
>> It's nearly 3am here...
>
> I looked once again at http::basic_parser and http::serializer. It
> might be possible to refactor http::basic_parser into a separate class
> and with some mess in http::parser support two versions. One which
> allows ConstBufferSequence and the other which allows just pair<void*,
> std::size>.

I've always felt concern about pair<void*, size_t>. AFIO v2 uses it, but
I am uncomfortable with it. In your case you can't use it because if you
declare it as a buffer type to ASIO and another piece of code does the
same, you get an ODR violation. So probably a struct beast::buffer {
void *data; size_t length; }; or something is safer.

> http::serializer is another story entirely. It contains and is tied to
> adapters which contain Asio buffer types. And it uses Asio buffer
> algorithms.
>
> You've taken the position that this is an easy matter, that you have
> extensive knowledge and experience in the domain, so I would kindly
> ask that you provide a working prototype of http::serializer which
> does not use Asio buffer types and yet remains compatible with the
> rest of Beast.

Your http::serializer claims it is for:

1. Send the header first, and the body later.
2. Set chunk extensions or trailers using a chunk decorator.
3. Send a message incrementally: bounded work in each I/O cycle.
4. Use a series of caller-provided buffers to represent the body.

The way I've always implemented HTTP serialisers in the recent past is
as a collection of open ranges of contiguous byte buffers for ASIO to
gather send from.

Generally I've kept a headers() collection and a body() collection.
Iterators for the whole serialiser iterate all the headers first, then
the body. But if the end user wanted to send just the headers first,
they simply by hand feed the headers range to ASIO first, then the body
range. So that's Item 1 taken care of.

Item 2, as I mentioned before I've never needed more than basic HTTP in
my career to date, so never needed to implement chunked encoding. But I
can tell you what my first instinct is: body() is a FIFO drain, so as
the user appends new body buffers, they get drained and sent. So, the
end iterator returned by body() is able to stop pointing after the last
item in body() if new items are added to body().

Item 3 is very straightforward because the end user is the person
creating the iterator range to hand to ASIO. So they simply choose a
range which is a subset of the total instead of the total.

Item 4 is even easier, because this entire design already is a series of
caller provided buffers from start to finish. Beast might provide
convenience functions for generating those buffers, but essentially the
caller supplies the buffers.

Finally, trailers I have never had need to implement either, but I'd
suggest a trailers() collection for those as well. Exactly the same
semantics as earlier.


I appreciate Vinnie that you envisioned Beast as doing all this stuff
for the end user and hiding all this complexity by wrapping up ASIO with
an i/o API which hides how ASIO is being used underneath.

I think that makes sense in a high level HTTP library. For a low level
HTTP library I think it a design mistake, much simpler is much better.
Less is more. I've used the above serialiser design on a number of
occasions now, it's very efficient, composable, and flexible. It *does*
push understanding of HTTP onto the end user, but then if the end user
doesn't understand HTTP, they wouldn't be able to use a low level
library anyway.

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
Vinnie Falco via Boost
2017-07-03 12:57:07 UTC
Permalink
On Mon, Jul 3, 2017 at 5:33 AM, Niall Douglas via Boost
<***@lists.boost.org> wrote:
> I've always felt concern about pair<void*, size_t>.

My use of pair was expository only.

> Generally I've kept a headers() collection and a body() collection.

Words are cheap. Please provide a serializer which compiles and runs,
with tests, and operates on Beast messages and does not use
<boost/asio/buffer.hpp>. I've provided something that works, time for
you to do the same.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Niall Douglas via Boost
2017-07-03 15:16:07 UTC
Permalink
>> Generally I've kept a headers() collection and a body() collection.
>
> Words are cheap. Please provide a serializer which compiles and runs,
> with tests, and operates on Beast messages and does not use
> <boost/asio/buffer.hpp>. I've provided something that works, time for
> you to do the same.

That's not how Boost reviews work Vinnie.

We each provide observations, comments, questions and proposals of
alternatives. We discuss. We may each provide formal reviews.

The review manager considers all of the discussion, and provides a
judgement. So I don't need to convince *you* of anything. I need to
convince everybody else on boost-dev, and thence Michael. It's a bonus
to convince you too, but it's not a requirement.

I think there is a great library lurking in Beast, and you've done a
great job bringing us a HTTP library for C++ that is far better than any
I've seen yet. But I think some more chiselling is yet needed to hew it out.

In particular, I think it's not consistently low level enough yet, and I
think a View + Ranges API design for the bottom layer makes a lot more
sense, with additional higher level ASIO-dependent layers laid on top.
If they others disagree with this opinion, they'll (usually silently)
ignore it. If they don't understand it, they'll ask for clarification.
If they agree, they'll formally vote the same way or similarly. That's
how reviews work.

(It's also very possible that another reviewer blows a big hole in my
alternative design as having a showstopper design fault, and I thus
change *my* opinion. You might consider taking this dialectical approach
of proving why my ideas can't work rather than challenging people to
write alternative implementations. If I had the spare time to do that,
I'd have written a HTTP library a long time ago and brought it here for
review. So would many others on here who have far more experience in
HTTP than either you or I, and if properly resourced, could have made
the definitive implementation bar none)

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
Vinnie Falco via Boost
2017-07-03 15:27:14 UTC
Permalink
On Mon, Jul 3, 2017 at 8:16 AM, Niall Douglas via Boost
<***@lists.boost.org> wrote:
>> Words are cheap.
> ...
> That's not how Boost reviews work Vinnie.

Is your issue with beast::http::basic_parser and beast::serializer
that it depends on <boost/asio/buffer.hpp>? Would your needs be
satisfied if it did not have this dependence?

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Vinícius dos Santos Oliveira via Boost
2017-07-08 21:22:24 UTC
Permalink
2017-07-03 14:57 GMT+02:00 Vinnie Falco via Boost <***@lists.boost.org>:

> > Generally I've kept a headers() collection and a body() collection.
>
> Words are cheap. Please provide a serializer which compiles and runs,
> with tests, and operates on Beast messages and does not use
> <boost/asio/buffer.hpp>. I've provided something that works, time for
> you to do the same.


This is one of the most ridiculous things one can say during a Boost review.

It's like freeing yourself from any critique. Your vote only counts if
you've implemented a library that can do this, this and that. So I'll
implement one feature that nobody uses today and then nobody can criticize
my library. What's the purpose of the review then? If the review pleases
you, it's one more approval. If the review displeases you, you just need to
run to “you aren't Beast author nor any library close to that so your
opinion don't count”.

I'd **never** reply such nonsense in reviews done to libraries of mine.

--
Vinícius dos Santos Oliveira
https://vinipsmaker.github.io/

_______________________________________________
Unsubscribe & other changes: http
Vinnie Falco via Boost
2017-07-03 13:21:53 UTC
Permalink
On Mon, Jul 3, 2017 at 5:33 AM, Niall Douglas via Boost
<***@lists.boost.org> wrote:
> ...ASIO...

You've said a lot and I'd like some clarity, is your issue with
beast::http::basic_parser and beast::serializer that it depends on
<boost/asio/buffer.hpp>? Would your needs be satisfied if it did not
have this dependence?

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Peter Koch Larsen via Boost
2017-07-03 13:44:03 UTC
Permalink
Just one perhaps uninformed remark. I have glanced over this thread as
I like to keep in touch with what happens in boost, but have no need
for a websocket interface.
To me, Vinnies library seems to be a well-designed and capable
library. But it seems to me that the library tries to accomplish two
things that coud be separated: parsing of http-headers and
transmission of http-headers. My guess is that - as Niall pointed out
- there is a large group of developers that could benefit from the
parsing part alone, and probably also a group that would perform their
own parsing and just use the transfer part. If this is so, Beast
should be split into two libraries.
Perhaps a fast way to accomplish that would be to have a small helper
library that could convert to/from asio-buffers to e.g.
std::vector<(w)char> and std::(w)string? Even if this caused
suboptimal performance, my guess is that this approach would have
appeal and not frighten to many users. We have had libraries in boost
with suboptimal performance before (lexical_cast is one such example)
where later revisions reduced or eliminated the performance loss.

/Peter

On Mon, Jul 3, 2017 at 3:21 PM, Vinnie Falco via Boost
<***@lists.boost.org> wrote:
> On Mon, Jul 3, 2017 at 5:33 AM, Niall Douglas via Boost
> <***@lists.boost.org> wrote:
>> ...ASIO...
>
> You've said a lot and I'd like some clarity, is your issue with
> beast::http::basic_parser and beast::serializer that it depends on
> <boost/asio/buffer.hpp>? Would your needs be satisfied if it did not
> have this dependence?
>
> _______________________________________________
> Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Vinnie Falco via Boost
2017-07-03 13:48:46 UTC
Permalink
On Mon, Jul 3, 2017 at 6:44 AM, Peter Koch Larsen via Boost
<***@lists.boost.org> wrote:
> My guess is that - as Niall pointed out
> - there is a large group of developers that could benefit from the
> parsing part alone,

They can use the parsing part alone today as well as the serializing
part, with a dependence on <boost/asio/buffer.hpp>. Examples of this
usage are in the documentation.

I believe the dependency on <boost/asio/buffer.hpp> is the main point
of contention (Niall, confirm?). I would like to have this discussion
if possible since a resolution would satisfy everyone.

>and probably also a group that would perform their own parsing and just use the transfer part.

This I have not heard of before, and is not a supported use-case. I
don't think its particularly interesting to users.

> Beast should be split into two libraries.

Beast is proposed as-is.

Thanks

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Niall Douglas via Boost
2017-07-03 15:24:29 UTC
Permalink
On 03/07/2017 14:21, Vinnie Falco via Boost wrote:
> On Mon, Jul 3, 2017 at 5:33 AM, Niall Douglas via Boost
> <***@lists.boost.org> wrote:
>> ...ASIO...
>
> You've said a lot and I'd like some clarity, is your issue with
> beast::http::basic_parser and beast::serializer that it depends on
> <boost/asio/buffer.hpp>? Would your needs be satisfied if it did not
> have this dependence?

If you also stopped implementing i/o, and instead facilitated i/o, I
believe I could live with your other design choices. You've made a good
library Vinnie.

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
Vinnie Falco via Boost
2017-07-03 15:25:55 UTC
Permalink
On Mon, Jul 3, 2017 at 8:24 AM, Niall Douglas via Boost
<***@lists.boost.org> wrote:
> If you also stopped implementing i/o, and instead facilitated i/o, I
> believe I could live with your other design choices. You've made a good
> library Vinnie.

Is your issue with beast::http::basic_parser and beast::serializer
that it depends on <boost/asio/buffer.hpp>? Would your needs be
satisfied if it did not have this dependence?

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Vinnie Falco via Boost
2017-07-03 15:30:02 UTC
Permalink
On Mon, Jul 3, 2017 at 8:24 AM, Niall Douglas via Boost
<***@lists.boost.org> wrote:
> If you also stopped implementing i/o, and instead facilitated i/o, I
> believe I could live with your other design choices.

Are you suggesting that Beast should not support reading and writing
HTTP messages on Asio streams?

--------------------------------------------------
On Sun, Jul 2, 2017 at 8:55 AM, Niall Douglas wrote:
>> ..you say Beast should not provide functions to
>> send and receive HTTP messages using Asio streams?
>
> Absolutely correct.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Paul A. Bristow via Boost
2017-07-03 13:19:15 UTC
Permalink
> -----Original Message-----
> From: Boost [mailto:boost-***@lists.boost.org] On Behalf Of Niall Douglas via Boost
> Sent: 03 July 2017 13:34
> To: ***@lists.boost.org
> Cc: Niall Douglas
> Subject: Re: [boost] [review][beast] Review of Beast starts today : July 1 - July 10
>
> On 03/07/2017 03:07, Vinnie Falco via Boost wrote:
> > On Sun, Jul 2, 2017 at 6:43 PM, Niall Douglas via Boost
> > <***@lists.boost.org> wrote:

<snip>

> I think that makes sense in a high level HTTP library. For a low level
> HTTP library I think it a design mistake, much simpler is much better.
> Less is more. I've used the above serialiser design on a number of
> occasions now, it's very efficient, composable, and flexible. It *does*
> push understanding of HTTP onto the end user, but then if the end user
> doesn't understand HTTP, they wouldn't be able to use a low level
> library anyway.

If BEAST is accepted, is there any reason why such an even--lower-level library could not (or should not) be written (and also
accepted)?

Paul

---
Paul A. Bristow
Prizet Farmhouse
Kendal UK LA8 8AB
+44 (0) 1539 561830





_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Niall Douglas via Boost
2017-07-03 15:19:58 UTC
Permalink
>> I think that makes sense in a high level HTTP library. For a low level
>> HTTP library I think it a design mistake, much simpler is much better.
>> Less is more. I've used the above serialiser design on a number of
>> occasions now, it's very efficient, composable, and flexible. It *does*
>> push understanding of HTTP onto the end user, but then if the end user
>> doesn't understand HTTP, they wouldn't be able to use a low level
>> library anyway.
>
> If BEAST is accepted, is there any reason why such an even--lower-level library could not (or should not) be written (and also
> accepted)?

That's a great question. It could be that I am over egging the
importance of there being a lower layer independent of networking
implementation. But then my past HTTP implementations have tended to go
over some really weird non-socket transport layers, so I'm going on what
I know.

Hence I delay my review until others have given theirs. I know that's a
cop out, but it's also recognition that my experience of "normal" HTTP
is limited, and I could be making unreasonable demands.

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
Vinnie Falco via Boost
2017-07-03 15:27:53 UTC
Permalink
On Mon, Jul 3, 2017 at 8:19 AM, Niall Douglas via Boost
<***@lists.boost.org> wrote:
> That's a great question. It could be that I am over egging the
> importance of there being a lower layer independent of networking
> implementation.

Is your issue with beast::http::basic_parser and beast::serializer
that it depends on <boost/asio/buffer.hpp>? Would your needs be
satisfied if it did not have this dependence?

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Niall Douglas via Boost
2017-07-03 16:15:39 UTC
Permalink
On 03/07/2017 16:27, Vinnie Falco via Boost wrote:
> On Mon, Jul 3, 2017 at 8:19 AM, Niall Douglas via Boost
> <***@lists.boost.org> wrote:
>> That's a great question. It could be that I am over egging the
>> importance of there being a lower layer independent of networking
>> implementation.
>
> Is your issue with beast::http::basic_parser and beast::serializer
> that it depends on <boost/asio/buffer.hpp>? Would your needs be
> satisfied if it did not have this dependence?

I did warn you to stop with the aggression Vinnie.

So ok, I am out of this review now. No more from me until the end when I
write my formal review.

Good luck with the review Vinnie.

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
Vinnie Falco via Boost
2017-07-03 16:17:33 UTC
Permalink
On Mon, Jul 3, 2017 at 9:15 AM, Niall Douglas via Boost
<***@lists.boost.org> wrote:
> I did warn you to stop with the aggression Vinnie.

Is your issue with beast::http::basic_parser and beast::serializer
that it depends on <boost/asio/buffer.hpp>? Would your needs be
satisfied if it did not have this dependence?

--------------------------------------------------
On Sun, Jul 2, 2017 at 8:55 AM, Niall Douglas wrote:
>> ..you say Beast should not provide functions to
>> send and receive HTTP messages using Asio streams?
>
> Absolutely correct.

Follow me on Github: https://github.com/vinniefalco

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Emil Dotchevski via Boost
2017-07-03 16:34:55 UTC
Permalink
On Mon, Jul 3, 2017 at 6:19 AM, Paul A. Bristow via Boost <
***@lists.boost.org> wrote:

> > I think that makes sense in a high level HTTP library. For a low level
> > HTTP library I think it a design mistake, much simpler is much better.
> > Less is more. I've used the above serialiser design on a number of
> > occasions now, it's very efficient, composable, and flexible. It *does*
> > push understanding of HTTP onto the end user, but then if the end user
> > doesn't understand HTTP, they wouldn't be able to use a low level
> > library anyway.
>
> If BEAST is accepted, is there any reason why such an even--lower-level
> library could not (or should not) be written (and also
> accepted)?
>

I don't see why not. For example I think it was a mistake to reject Synapse
on the basis of "we have a signal library in Boost already", because there
is literally nothing in common between it and Boost Signal, except that the
latter can be used in a subset of the cases where Synapse can be used.

If we reject a library it should be on its own merits, not on the merits of
another library, except if the differences are trivial.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Vinícius dos Santos Oliveira via Boost
2017-07-08 21:04:17 UTC
Permalink
2017-07-03 2:17 GMT+02:00 Emil Dotchevski via Boost <***@lists.boost.org>:

> I think Vinnie is right that it is "your style". "I've done HTTP six times
> in my career" is not an argument, the subtext is "you're inexperienced and,
> this library is a nice try, but if you want to play with the big boys you
> have to do it differently."


He added context on top of that.

I'd keep ASIO dependency, but even so Niall's feedback is useful in driving
attention to a point of view that can be easily neglected.

--
Vinícius dos Santos Oliveira
https://vinipsmaker.github.io/

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman
Emil Dotchevski via Boost
2017-07-01 22:15:41 UTC
Permalink
On Sat, Jul 1, 2017 at 1:38 PM, Niall Douglas via Boost <
***@lists.boost.org> wrote:

> The reason why according to you is for the buffer infrastructure. But as
> I've already told you, that's a relic from a decade ago. New code
> neither ought to nor needs to use that. We have far better available today.
>

"A relic from a decade ago" is not an argument.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Niall Douglas via Boost
2017-07-02 15:07:19 UTC
Permalink
>> The reason why according to you is for the buffer infrastructure. But as
>> I've already told you, that's a relic from a decade ago. New code
>> neither ought to nor needs to use that. We have far better available today.
>>
>
> "A relic from a decade ago" is not an argument.

It would not be if there hadn't been very significant progress in how to
do generic and powerful buffers and views since.

However, we have the Ranges TS now, we have Contiguous Container concept
support, we have spans and views, all on the standards track. ASIO gets
to keep its legacy design because it's so important to standardise
existing practice, not invent standards by committee. But any new code
must be held to a higher standard.

And most especially in this particular case, dragging in the entire of
ASIO for a few buffer types is just crazy daft. Instead use some Ranges
TS inspired types - or a C++ 98 era iterator pair - from which ASIO can
auto construct buffer sequences on its own. Do not drag in ASIO for such
a trivial, and unnecessary, use case, so gratuitously damaging the end
user experience as a result.

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
Vinnie Falco via Boost
2017-07-02 15:08:20 UTC
Permalink
On Sun, Jul 2, 2017 at 8:07 AM, Niall Douglas via Boost
<***@lists.boost.org> wrote:
>...

> Very little in your library has, or ought to have, anything to do with i/o.

Is my understanding correct that you say Beast should not provide
functions to send and receive HTTP messages using Asio streams?

Thanks

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Vinícius dos Santos Oliveira via Boost
2017-07-08 17:39:20 UTC
Permalink
2017-07-01 22:38 GMT+02:00 Niall Douglas via Boost <***@lists.boost.org>:

> So why are you forcing end users to drag in ASIO?
>
> The reason why according to you is for the buffer infrastructure. But as
> I've already told you, that's a relic from a decade ago. New code
> neither ought to nor needs to use that. We have far better available today.
>

That's so neurotic (as defined by Jung). In one review, you'll complain
that C++03 is all we need. In another review, you'll complain that
C++22-tier abstractions are not being used. Both complaints are irrelevant.
You can use any stable C++ to submit a Boost library.

--
Vinícius dos Santos Oliveira
https://vinipsmaker.github.io/

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman
Vinícius dos Santos Oliveira via Boost
2017-07-08 17:50:48 UTC
Permalink
2017-07-01 22:38 GMT+02:00 Niall Douglas via Boost <***@lists.boost.org>:

> If that's too experimental for you (though the Ranges TS is a TS just
> like the Networking TS), then do as Boost.Spirit does and work with
> iterator pairs. So for example, if I want to know what the
> Content-Length header is, upon query I get back an iterator pair
> pointing to the front of the storage and one after the end of the
> storage. Similarly, if I set the Content-Length header, I supply an
> iterator pair to the new contents.
>

That would be pretty bad as HTTP also involves connection management (e.g.
"connection: close").

It'd work for an HTTP parser. But Beast seems pretty low-level already.

--
Vinícius dos Santos Oliveira
https://vinipsmaker.github.io/

_______________________________________________
Unsubscribe & other changes: http://lists.boo
Peter Dimov via Boost
2017-07-01 18:08:02 UTC
Permalink
Niall Douglas wrote:

> - I appreciate that this my final negative will not be widely agreed with
> on boost-dev, but personally speaking I think Beast should work well with
> C++ exceptions disabled, ...

BOOST_THROW_EXCEPTION abides by best Rust/SG14 practices of aborting on
exception throw when exceptions are disabled. :-)


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Emil Dotchevski via Boost
2017-07-01 18:32:52 UTC
Permalink
On Sat, Jul 1, 2017 at 7:46 AM, Niall Douglas via Boost <
***@lists.boost.org> wrote:

> There are lots of end users who would like to
> speak HTTP without enabling C++ exceptions for cultural or safety
> validation reasons: embedded devices, IoT, games, medical devices etc.
>

You forgot to mention "unpredictability" of exceptions. :)

Also, there is no error handling in games.


> Beast is not useful to any of those use cases if it insists on ASIO and
> throws exceptions, neither of which are necessary nor make for a better
> HTTP utilities library.
>

They're certainly not necessary for users who don't need robust error
handling.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Niall Douglas via Boost
2017-07-01 19:52:12 UTC
Permalink
> There are lots of end users who would like to
> speak HTTP without enabling C++ exceptions for cultural or safety
> validation reasons: embedded devices, IoT, games, medical devices etc.
>
> You forgot to mention "unpredictability" of exceptions. :)
>
> Also, there is no error handling in games.

Anyone not error handling in HTTP needs their head examined.

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
Vinnie Falco via Boost
2017-07-01 19:54:25 UTC
Permalink
On Sat, Jul 1, 2017 at 12:52 PM, Niall Douglas via Boost
<***@lists.boost.org> wrote:
> Anyone not error handling in HTTP needs their head examined.

I'll kindly ask that you do not make inflammatory statements in
threads which mention Beast in the subject line. Some folks have
relatives or friends who may be mentally ill, and your ad-hominem
implies that those individuals are somehow less worthy of respect.
Such language is non-inclusive and diminishes the stature of the list
(in my opinion).

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Emil Dotchevski via Boost
2017-07-01 22:30:08 UTC
Permalink
On Sat, Jul 1, 2017 at 12:52 PM, Niall Douglas via Boost <
***@lists.boost.org> wrote:

> > There are lots of end users who would like to
> > speak HTTP without enabling C++ exceptions for cultural or safety
> > validation reasons: embedded devices, IoT, games, medical devices
> etc.
> >
> > You forgot to mention "unpredictability" of exceptions. :)
> >
> > Also, there is no error handling in games.
>
> Anyone not error handling in HTTP needs their head examined.
>

You mean error checking, I mean handling and recovery. Games tend to just
retry and eventually reboot on errors, rather than tear-down data
structures and recover gracefully. It's one reason why game programmers
don't see value in exception handling.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Vinícius dos Santos Oliveira via Boost
2017-07-08 14:19:24 UTC
Permalink
I only found the time to join the discussion now 😢

I'd appreciate if 4 more days were given to review it, but I'll do my best
to review it until July 10.

This is the first message I'll answer from the whole discussion. Time to
have fun discussing HTTP 😀

2017-07-01 16:46 GMT+02:00 Niall Douglas via Boost <***@lists.boost.org>:

> - I think basic_parser should be designed to use forward iterators
> instead of requiring random access, or accept partial input in chunks
> whilst keeping state. This would allow it to work seamlessly with
> discontiguous underlying storage.
>

After the review on my library, I noted there was a big interest in
low-level control for handling HTTP messages. I developed a parser library
which has the good properties of the NodeJS parser (no syscalls, no
allocation/internal-buffers, interruptable at any time...) and avoids its
bad design decisions[1].

In less than 170 lines you can handle the receiving part of your library:
https://github.com/vinipsmaker/tufao/blob/43cd33f3388fd7061dedc75d683810a258f42eb3/src/httpserverrequest.cpp#L125

And I don't assume you're going to read RFC7230. Decoded values are decoded
(different than NodeJS "easy-to-use-wrong" parser).

Anyway, it's fairly easy to adapt this parser model to C++ iterator model
(I had this concern in mind when designing the API — as a matter of fact,
one of the test files adapts the parser model so I can define a lot of
tests just by comparing two vectors[2]).

And I might answer this same message again after finishing the review. Lots
of hours ahead of me to spend with HTTP.

[1]
https://vinipsmaker.wordpress.com/2016/08/05/boost-http-has-a-new-parser/
[2]
https://github.com/BoostGSoC14/boost.http/blob/b00380a1f8c2588baa4cd5732460624b80f23a1f/test/request11.cpp#L544

--
Vinícius dos Santos Oliveira
https://vinipsmaker.github.io/

_______________________________________________
Unsubscribe & other changes: ht
Vinnie Falco via Boost
2017-07-08 14:51:36 UTC
Permalink
On Sat, Jul 8, 2017 at 7:19 AM, Vinícius dos Santos Oliveira via Boost
<***@lists.boost.org> wrote:
> I only found the time to join the discussion now 😢

Glad to see you here Vinícius! I see you've been busy working on your
repositories (I check on them from time to time) - that's great!

> Anyway, it's fairly easy to adapt this parser model to C++ iterator model

Using templated iterators (I think that's the direction you're
implying) offers flexibility. But there are advantages when using a
raw pointer to a contiguous piece of memory holding the entire HTTP
header or other structured element (chunk-header, final-chunk). Such
as using SSE4.2 intrinsics to accelerate the parsing:

<https://github.com/vinniefalco/Beast/blob/6f88f0182d461e9cabe55032f966c9ca4f77bf46/include/beast/http/detail/basic_parser.hpp#L223>

> After the review on my library, I noted there was a big interest in low-level control for handling HTTP messages.

Can you please list the low-level control features or maybe link to
the corresponding boost-dev posts which describe the desired low-level
control? I thought I addressed them but I'd like to be sure

Thanks

_______________________________________________
Unsubscribe & other changes: http://lists.boost.or
Zach Laine via Boost
2017-07-01 23:42:49 UTC
Permalink
On Sat, Jul 1, 2017 at 2:39 AM, Michael Caisse via Boost <
***@lists.boost.org> wrote:

> The formal review of Vinnie Falco’s Beast library will take place from
> July 1 through July 10, 2017.
>
> Please consider participating in this review. The success of Boost is
> partially a result of the quality review process which is conducted by
> the community. You are part of the Boost community. I will be grateful
> to receive a review based on whatever level of effort or time you can
> devote.
>

Aright, you got me.

[snip]

One thing up top: igzf what you call your library. You're the author, you
get to name it. We have Spirit (w/Qi and Karma), Hana, Phoenix, and Proto
already. No one can claim that convention around here is that a library's
name matches its uses.

Some other questions you might want to consider answering:
>
> - What is your evaluation of the design?
>

* Overall, I quite like it! I have some complaints, but they are about
small design choices. The largest design choices, of adhering closely to
the now-begin-standardized ASIO/Net-TS, and to stay in a layer just above
ASIO (and not much higher), are the right ones. Also, making the library
explicitly standards-tracked is a good choice. We need more of that in
Boost these days.

* Allocators are gross. I know this is a controversial position, but I
think they're far more trouble than they're worth. Since you already have
buffers that seem to me to cover all the major allocation strategies
already, can't you get away with adding a buffer adapter that takes any
random access container as its underlying storage, and be done with it?
This is an effort to reduce your maintenance workload, and the semantic
overhead of users. Introducing allocators and then trying to use them
uniformly has lead to some awful stuff in the standard, like allocators in
the std::pair and std::tuple ctors, even though they do not allocate. Yuck.

* Is there a reason why buffer_cat_view and buffer_prefix_view are distinct
types? It isn't obvious just from their descriptions. Moreover, fewer
user-facing types is better. I would greatly prefer to write code like
this:

buffers_view<BufT0, BufT1, ...> buffers = buffer_cat(buf0, buf1, ...);
buffers_view<BufT0, BufT1, ...> buffers_slice = buffer_slice(0, prefix_end);
buffers = buffers_slice(offset, size);

To accomplish this, I think you just need to make a single type that owns a
sequence of buffers, and maintains a pair of iterators or indices into the
owned sequence. If there is a deeper reason that I have not noticed for
why the current design should remain as-is, please add this to a Rationale
section.

* Similarly, as a user I'd prefer not to have string_body as a distinct
type (unless it's a convenience typedef for dynamic_body<std::string>). If
std::string is not a model of DynamicBuffer, I think DynamicBuffer could
stand a slight reformulation.

* I get why message<> has a Body template parameter, but why is Fields
configurable? There is no message<> instantiation under examples/ or
include/ that uses anything other than basic_fields<> that I could find.
Do people ever use something else? Again, a brief rationale would be
useful. I saw the "HTTP Message Container" page, btw, but while it
suggested someone might want a different Fields template param, it didn't
indicate why.

- What is your evaluation of the implementation?
>

I did not look much at the implementation, except as quick checks against
the documentation.


> - What is your evaluation of the documentation?
>

Very good. It has lot and lots of concrete examples of how to use
different parts of the library in many different ways. That's really
important for a large library.

* The Quick Start contains its subsections in full, and then there are TOC
links to each subsection separately. Probably one or the other would do.

* Each of the example links takes me to a GitHub page for the file linked.
That should change to inline source like the Quick Start code. The same
thing is true of all the reference documentation.

* HTTP Crawl seems to be doing synchronous reads. Is there an example
elsewhere that does a bunch of reads like this asynchronously?

* example/http-server-fast/http_server_fast.cpp explicitly checks for '/'
path separators, and uses strings instead of filesystem::paths. It would
be cool if the former were changed for Windows folks, and the latter
changed, just 'cuz.

* At least http_server_small.cpp (and maybe others? I didn't go back and
check) has only Christopher M. Kohlhoff as an author. Vinnie, you probably
need a copyright audit of all your files.

* I found the mix of function and type entries in "Table 6. Buffer
Algorithms" to be initially quite confusing. Maybe separate them?

* The buffer_prefix_view documentation in "Table 6. Buffer Algorithms"
says "This is the type of buffer returned by buffer_prefix.", but 2/3
overloads with that name return something else.

* HTTP Message Container says this:

"
Now we can declare a function which takes any message as a parameter:

template<bool isRequest, class Fields, class Body>
void f(message<isRequest, Fields, Body>& msg);
This function can manipulate the fields common to requests and responses.
If it needs to access the other fields, it can do so using tag dispatch
with an object of type std::integral_constant<bool, isRequest>.
"

This strikes me as odd advice. With pre-C++17 I would write:

template<class Fields, class Body>
void f(message<true, Fields, Body>& msg)
{ /*...*/ }

template<class Fields, class Body>
void f(message<false, Fields, Body>& msg)
{ /*...*/ }

Why would I have a third overload and use tag dispatching? Most users are
going to be really confused by what I quoted above, I think.

Of course, with C++17 and later I'd write:

template<bool isRequest, class Fields, class Body>
void f(message<isRequest, Fields, Body>& msg)
{
if constexpr (isRequest) {
// ...
} else {
// ...
}
}

.. and that lines up nicely with the given rationale (just not the tag
dispatching bit).

* Please add an explicit Rationale section for smaller design choices. The
FAQ and design justification sections that exist are fine, but they mostly
cover the biggest-picture items. Each of those small design choices are
going to go out the window if you can't remember a strong argument for one
of them 5 years from now, when LEWG are asking you why a certain interface
is the way it is.

- What is your evaluation of the potential usefulness of the library?
>

High. Super duper high. The fact that I have no standard way to do
anything in this library today, and this library is
standardization-oriented is a great thing.


> - Did you try to use the library? With which compiler(s)? Did you
> have any problems?
>

I did not use the library.


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

I spent about 3 hours reading documentation and examples, with a quick
glance at some of the implementation.


> - Are you knowledgeable about the problem domain?


Somewhat. I've written a couple of Node.js-based web servers, a couple of
straight-ASIO non-web servers and clients, and done a bit of web
development. I've done nothing that works at exactly the level Beast
targets.

Zach

_______________________________________________
Unsubscribe & other chang
Vinnie Falco via Boost
2017-07-02 00:22:40 UTC
Permalink
On Sat, Jul 1, 2017 at 4:42 PM, Zach Laine via Boost
<***@lists.boost.org> wrote:

Thanks for checking out the library!

> * Is there a reason why buffer_cat_view and buffer_prefix_view are distinct
> types?

Yes, buffer_cat_view and buffer_prefix_view are two completely
different things. A buffer_cat_view represents a buffer sequence of
buffer sequences, while a buffer_prefix_view is just a range adapter
over a single buffer sequence.

> I think you just need to make a single type that owns a sequence of buffers,
> and maintains a pair of iterators or indices into the owned sequence.

You've more or less described buffer_prefix_view.

A rough description of buffer_cat_view would be "a type that owns a
heterogenous list of buffer sequences, whose iterators reflect the
current position within a variant consisting of each iterator types
from the list of buffer sequences." Quite different :)

> * Similarly, as a user I'd prefer not to have string_body as a distinct
> type (unless it's a convenience typedef for dynamic_body<std::string>). If
> std::string is not a model of DynamicBuffer, I think DynamicBuffer could
> stand a slight reformulation.

std::string does not model DynamicBuffer, which itself is a Net-TS
concept and thus immutable from Beast's perspective. Therefore
beast::http::basic_dynamic_body cannot work with std::string and
beast::http::string_body becomes necessary . I will likely add
beast::http::basic_string_body to allow the allocator to be
customized.

> * I get why message<> has a Body template parameter, but why is Fields
> configurable? There is no message<> instantiation under examples/ or
> include/ that uses anything other than basic_fields<> that I could find.
> Do people ever use something else? Again, a brief rationale would be
> useful. I saw the "HTTP Message Container" page, btw, but while it
> suggested someone might want a different Fields template param, it didn't
> indicate why.

I have not done extensive work in this area of the library yet but I
assure you it is a useful feature. A custom Fields implementation
could support only a subset of all possible fields (for example it
might only be capable of representing Content-Length,
Transfer-Encodoing, Server, and User-Agent), storing the field name in
a small integer enum instead of keeping the entire text. It could
store Content-Length as an integer instead of a string. Custom Fields
could use its own strategy for storing the field/value pairs.

Most importantly, a custom Fields could be used to adapt another
library's message model into something that Beast can consume. Or to
allow a Beast message to be consumable by another library with a
proper wrapper. All of these contemplated use-cases require the user
to also provide a subclass of basic_parser, as the beast::http::parser
is only usable with beast::http::basic_fields.

These use-cases seem exotic but I feel that the niches they satisfy
will come up and have merit. Think embedded devices or Internet of
Things. Creating examples of custom Fields is on my to-do:
<https://github.com/vinniefalco/Beast/issues/504>

> * Allocators are gross. I know this is a controversial position,

I agree, that is controversial! And I'm not entirely happy with the
interface of AllocatorAwareContainer but it is what it is. Which is to
say it is standardized.

> I think they're far more trouble than they're worth. Since you already have
> buffers that seem to me to cover all the major allocation strategies
> already, can't you get away with adding a buffer adapter that takes any
> random access container as its underlying storage, and be done with it?

That already exists, you can use beast::buffers_adapter on any linear
buffer (including one provided by a container) and then use that
adapter with beast::http::basic_dynamic_body.

There are only a few AllocatorAwareContainer in Beast, those include
basic_flat_buffer, basic_multi_buffer, but most importantly
http::basic_fields. I don't think removing allocator support in
http::basic_fields is such a good idea, especially when one of the
examples already demonstrates how performance may be improved through
its use:
<https://github.com/vinniefalco/Beast/blob/78a065ba39836d91d7e70d93de7f9140f518083b/example/http-server-fast/fields_alloc.hpp#L86>

Folks who want to get the most out of Beast will be taking advantage
of Beast's allocator support so I must respectfully disagree with your
position.

> * The Quick Start contains its subsections in full, and then there are TOC
> links to each subsection separately. Probably one or the other would do.

This is intended, I want every example to have a ToC entry so that
users can go to it quickly. Maybe if they forget the section it was in
then the Toc entry will jog their memory.

> * Each of the example links takes me to a GitHub page for the file linked.
> That should change to inline source like the Quick Start code.

Hmm I don't know about that, the server-framework is enormous.
Including the example source code in bulk would make it hard to
navigate. And I plan on more examples, I don't think that scales. Of
course, if the consensus is that this change reflects an improvement
then I will make it gladly. I have opened this issue for discussion:
<https://github.com/vinniefalco/Beast/issues/565>

>The same thing is true of all the reference documentation.

I'm not sure what you mean here.

> * HTTP Crawl seems to be doing synchronous reads. Is there an example
> elsewhere that does a bunch of reads like this asynchronously?

Not yet but I can add something like that:
<https://github.com/vinniefalco/Beast/issues/566>

> * At least http_server_small.cpp (and maybe others? I didn't go back and
> check) has only Christopher M. Kohlhoff as an author. Vinnie, you probably
> need a copyright audit of all your files.

The copyright is correct, Chris is the author of http_server_small.cpp
and http_server_fast.cpp

> * I found the mix of function and type entries in "Table 6. Buffer
> Algorithms" to be initially quite confusing. Maybe separate them?

How about changing the title to "Buffer Algorithms and Types"?

> * The buffer_prefix_view documentation in "Table 6. Buffer Algorithms"
> says "This is the type of buffer returned by buffer_prefix.", but 2/3
> overloads with that name return something else.

Yeah that's a little confusing. I'll sort it:
<https://github.com/vinniefalco/Beast/issues/567>

> * HTTP Message Container says this:
> ...
> If it needs to access the other fields, it can do so using tag dispatch
> ...
> Why would I have a third overload and use tag dispatching? Most users are
> going to be really confused by what I quoted above, I think.

You're right. I think I had functions like this in mind:
<https://github.com/vinniefalco/Beast/blob/78a065ba39836d91d7e70d93de7f9140f518083b/include/beast/http/impl/basic_parser.ipp#L347>

The doc is a bit confusing, I will fix it.

> * Please add an explicit Rationale section for smaller design choices. The
> FAQ and design justification sections that exist are fine, but they mostly
> cover the biggest-picture items. Each of those small design choices are
> going to go out the window if you can't remember a strong argument for one
> of them 5 years from now, when LEWG are asking you why a certain interface
> is the way it is.

Good advice but either vague or broad in scope (every design choice?),
if there are specific design choices that should be explained then
this becomes more actionable:
<https://github.com/vinniefalco/Beast/issues/569>

Thanks

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Zach Laine via Boost
2017-07-02 01:20:08 UTC
Permalink
On Sat, Jul 1, 2017 at 7:22 PM, Vinnie Falco via Boost <
***@lists.boost.org> wrote:

> On Sat, Jul 1, 2017 at 4:42 PM, Zach Laine via Boost
> <***@lists.boost.org> wrote:
>
> Thanks for checking out the library!
>
> > * Is there a reason why buffer_cat_view and buffer_prefix_view are
> distinct
> > types?
>
> Yes, buffer_cat_view and buffer_prefix_view are two completely
> different things. A buffer_cat_view represents a buffer sequence of
> buffer sequences, while a buffer_prefix_view is just a range adapter
> over a single buffer sequence.
>
> > I think you just need to make a single type that owns a sequence of
> buffers,
> > and maintains a pair of iterators or indices into the owned sequence.
>
> You've more or less described buffer_prefix_view.
>

Right.


> A rough description of buffer_cat_view would be "a type that owns a
> heterogenous list of buffer sequences, whose iterators reflect the
> current position within a variant consisting of each iterator types
> from the list of buffer sequences." Quite different :)


They don't seem that different to me. The data look the same from the
outside, but the underlying structure is different. The underlying
structure doesn't seem like it *needs* to be different, though. One could
be flattened into the other. These are all views, right? Cheap-to-copy
views? So, in terms of the existing types, if buffer_cat() returned a
buffer_prefix_view with n=0, would that break things? I mean a semantic
breakage, not a compilation breakage.


> > * Similarly, as a user I'd prefer not to have string_body as a distinct
> > type (unless it's a convenience typedef for dynamic_body<std::string>).
> If
> > std::string is not a model of DynamicBuffer, I think DynamicBuffer could
> > stand a slight reformulation.
>
> std::string does not model DynamicBuffer, which itself is a Net-TS
> concept and thus immutable from Beast's perspective. Therefore
> beast::http::basic_dynamic_body cannot work with std::string and
> beast::http::string_body becomes necessary . I will likely add
> beast::http::basic_string_body to allow the allocator to be
> customized.


I get it. That's a shame.


> > * Allocators are gross. I know this is a controversial position,
>
> I agree, that is controversial! And I'm not entirely happy with the
> interface of AllocatorAwareContainer but it is what it is. Which is to
> say it is standardized.


It's not that uncommon an opinion these days. Be mentally prepared for a
hard left turn wrt allocators (which may never come), if this finally makes
it to LEWG.


> > * Please add an explicit Rationale section for smaller design choices.
> The
> > FAQ and design justification sections that exist are fine, but they
> mostly
> > cover the biggest-picture items. Each of those small design choices are
> > going to go out the window if you can't remember a strong argument for
> one
> > of them 5 years from now, when LEWG are asking you why a certain
> interface
> > is the way it is.
>
> Good advice but either vague or broad in scope (every design choice?),
> if there are specific design choices that should be explained then
> this becomes more actionable:
> <https://github.com/vinniefalco/Beast/issues/569>
>

Well, certainly not every choice. But definitely every nonobvious choice,
or a choice that looks from the outside like taste, should go in a
rationale section. imo.

Zach

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Vinnie Falco via Boost
2017-07-02 03:44:33 UTC
Permalink
On Sat, Jul 1, 2017 at 6:20 PM, Zach Laine via Boost
<***@lists.boost.org> wrote:
> They don't seem that different to me. The data look the same from the
> outside, but the underlying structure is different. The underlying
> structure doesn't seem like it *needs* to be different, though. One could
> be flattened into the other. These are all views, right? Cheap-to-copy
> views? So, in terms of the existing types, if buffer_cat() returned a
> buffer_prefix_view with n=0, would that break things? I mean a semantic
> breakage, not a compilation breakage.

Perhaps it is possible to combine the functionality of
buffer_prefix_view and buffer_cat_view, but it would come at the
expense of a larger object. Buffer sequences are supposed to be
copyable and normally they are not particularly large. For example
beast::multi_buffer::const_buffers_type costs just one pointer and
therefore pretty darn cheap. But some parts of Beast declare truly
gnarly buffer sequences, like this one:
<https://github.com/vinniefalco/Beast/blob/78a065ba39836d91d7e70d93de7f9140f518083b/include/beast/http/serializer.hpp#L190>

using ch2_t = consuming_buffers<buffer_cat_view<
detail::chunk_header,
boost::asio::const_buffers_1,
boost::asio::const_buffers_1,
typename reader::const_buffers_type,
boost::asio::const_buffers_1,
boost::asio::const_buffers_1,
boost::asio::const_buffers_1,
boost::asio::const_buffers_1>>;

reader::const_buffers_type can itself be a buffer_cat_view or some
other sort of adapted buffer sequence, it depends on the traits of the
Body being used.

Chris was very helpful in working with me to optimize the
http-server-fast example, he pointed out that the size of this buffer
was exactly 256 bytes which unfortunately is one byte larger than it
needs to be in order to take advantage of an Boost.Asio implementation
optimization. I got lucky and found an unused member in
consuming_buffers which I was able to remove.

The moral of the story is that sizeof(ConstBufferSequence) counts, so
these buffer range adapters need to be separate in order to be as
small as possible.

Thanks

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Mike Gresens via Boost
2017-07-02 17:33:45 UTC
Permalink
Am 02.07.2017 um 02:22 schrieb Vinnie Falco via Boost:
>
>> * Similarly, as a user I'd prefer not to have string_body as a distinct
>> type (unless it's a convenience typedef for dynamic_body<std::string>). If
>> std::string is not a model of DynamicBuffer, I think DynamicBuffer could
>> stand a slight reformulation.
> std::string does not model DynamicBuffer, which itself is a Net-TS
> concept and thus immutable from Beast's perspective. Therefore
> beast::http::basic_dynamic_body cannot work with std::string and
> beast::http::string_body becomes necessary . I will likely add
> beast::http::basic_string_body to allow the allocator to be
> customized.
>
>

As a user I'd prefer not to have string_body as a distinct type too.
Same for string_view_body. Both are tightly bounded to distinct types.

Instead of having more and more body types tightly bounded to distinct
types i would prefer less body types loosely bounded to concepts.

So with a concept of "character sequence" we could introduce a
"sequence_body" working with models like std::string, std::string_view,
boost::string_ref, std::vector<char>, etc.

And with a concept of "character stream" we could introduce a
"stream_body" working with models like std::[o|i]stringstream,
std::[o|i]fstream, boost::iostreams::stream, etc.

These two new body types could replace string_body, string_view_body,
const_body (from example/common) and mutable_body (from example/common).

So we end up with less body types and less redundance, but much more
flexibility and testability.

Sample code for using these new body types below.
What do other participants think about this concept based body design?

Thanks,
Mike...


#include <beast/core.hpp>
#include <beast/http.hpp>
#include <boost/asio.hpp>
#include <iostream>
#include <fstream>
#include <sstream>
#include "sequence_body.hpp"
#include "stream_body.hpp"

inline constexpr auto host = "httpbin.org";

void sequence_body_demo(boost::asio::ip::tcp::socket& socket,
beast::flat_buffer& buffer)
{
using string_view_body = beast::http::sequence_body<std::string_view>;
using string_body = beast::http::sequence_body<std::string>;

beast::http::request<string_view_body> request;
request.method(beast::http::verb::post);
request.target("/post");
request.set(beast::http::field::host, host);
request.set(beast::http::field::content_length, 3);
request.body = "foo";
beast::http::write(socket, request);

beast::http::response<string_body> response;
beast::http::read(socket, buffer, response);
std::cout << response.body << std::endl;
}

void stream_body_demo(boost::asio::ip::tcp::socket& socket,
beast::flat_buffer& buffer)
{
using ifstream_body = beast::http::stream_body<std::ifstream>;
using osstream_body = beast::http::stream_body<std::ostringstream>;

beast::http::request<ifstream_body> request;
request.method(beast::http::verb::post);
request.target("/post");
request.set(beast::http::field::host, host);
request.set(beast::http::field::content_length, 3); // chunked does
not work, httpbin needs content_length
request.body.open("foo.txt");
beast::http::write(socket, request);

beast::http::response<osstream_body> response;
beast::http::read(socket, buffer, response);
std::cout << response.body.str() << std::endl;
}

int main()
{
try
{
boost::asio::io_service service;
boost::asio::ip::tcp::socket socket {service};
boost::asio::ip::tcp::resolver resolver {service};
boost::asio::connect(socket, resolver.resolve({host, "http"}));
beast::flat_buffer buffer;

sequence_body_demo(socket, buffer);
stream_body_demo(socket, buffer);
}
catch (std::exception const& exception)
{
std::cerr << "error: " << exception.what() << std::endl;
}
return 0;
}

==>

{
"args": {},
"data": "foo",
"files": {},
"form": {},
"headers": {
"Connection": "close",
"Content-Length": "3",
"Host": "httpbin.org"
},
"json": null,
"origin": "46.59.255.80",
"url": "http://httpbin.org/post"
}

{
"args": {},
"data": "foo",
"files": {},
"form": {},
"headers": {
"Connection": "close",
"Content-Length": "3",
"Host": "httpbin.org"
},
"json": null,
"origin": "46.59.255.80",
"url": "http://httpbin.org/post"
}


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Vinnie Falco via Boost
2017-07-02 18:34:05 UTC
Permalink
On Sun, Jul 2, 2017 at 10:33 AM, Mike Gresens via Boost
<***@lists.boost.org> wrote:
> As a user I'd prefer not to have string_body as a distinct type too. Same
> for string_view_body. Both are tightly bounded to distinct types.

To be clear, Peter was asking why not `response<std::string>` which is
a different but legitimate question. There are design reasons why
`std::string` cannot be a choice for Body.

> So with a concept of "character sequence" we could introduce a
> "sequence_body" working with models like std::string, std::string_view,
> boost::string_ref, std::vector<char>, etc.

What do you propose replacing this declaration with?

response<string_body> req;

Thanks

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Mike Gresens via Boost
2017-07-02 18:52:54 UTC
Permalink
Am 02.07.2017 um 20:34 schrieb Vinnie Falco via Boost:

> On Sun, Jul 2, 2017 at 10:33 AM, Mike Gresens via Boost
> <***@lists.boost.org> wrote:
>> As a user I'd prefer not to have string_body as a distinct type too. Same
>> for string_view_body. Both are tightly bounded to distinct types.
> To be clear, Peter was asking why not `response<std::string>` which is
> a different but legitimate question. There are design reasons why
> `std::string` cannot be a choice for Body.

I'm not that sure. He gave the "dynamic_body<std::string>" example.
So he wanted a body using std::string, i think.

>
>> So with a concept of "character sequence" we could introduce a
>> "sequence_body" working with models like std::string, std::string_view,
>> boost::string_ref, std::vector<char>, etc.
> What do you propose replacing this declaration with?
>
> response<string_body> req;

response<string_body> req;"

nothing to change here since

using string_body = beast::http::sequence_body<std::string>; // convenience decl in beast

Thanks,
Mike...


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Vinnie Falco via Boost
2017-07-02 19:34:46 UTC
Permalink
On Sun, Jul 2, 2017 at 11:52 AM, Mike Gresens via Boost
<***@lists.boost.org> wrote:
> Am 02.07.2017 um 20:34 schrieb Vinnie Falco via Boost:
> response<string_body> req;"
>
> nothing to change here since
>
> using string_body = beast::http::sequence_body<std::string>; //
> convenience decl in beast

Phew, okay, so we are on the same page. I don't want users to have to
write `beast::http::const_sequence_body<std::string>`

What you are asking for is a feature request, which requires no change
in interface to any algorithm. That's a much easier ask. You already
proposed two containers which were merged to the master branch in the
example/common directory (thanks for your contribution!)

<https://github.com/vinniefalco/Beast/blob/78a065ba39836d91d7e70d93de7f9140f518083b/example/common/const_body.hpp>

<https://github.com/vinniefalco/Beast/blob/78a065ba39836d91d7e70d93de7f9140f518083b/example/common/mutable_body.hpp>

These are not public interfaces yet, but I could be convinced to make
them public interfaces if:

1. There is established practice that is either standardized or close to it [1]

2. The names and implementation of the concept checks either use
existing standard library facilities if they exist, or provide an
implementation that tracks the standard (or proposal)

3. The documentation (javadoc comments and exposition in the
documentation) meets the quality requirements of the library

4. There is general support (which there very likely already is)

I don't mind doing this work myself but I am not in a hurry to do so
since existing use of Beast will be unaffected and there are other
priorities. On the other hand if someone else wants to submit a pull
request, I wouldn't turn it away!

[1] This might be related
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n3884.pdf

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Zach Laine via Boost
2017-07-02 00:33:13 UTC
Permalink
On Sat, Jul 1, 2017 at 6:42 PM, Zach Laine <***@gmail.com>
wrote:

> On Sat, Jul 1, 2017 at 2:39 AM, Michael Caisse via Boost <
> ***@lists.boost.org> wrote:
>
>> The formal review of Vinnie Falco’s Beast library will take place from
>> July 1 through July 10, 2017.
>>
>> Please consider participating in this review. The success of Boost is
>> partially a result of the quality review process which is conducted by
>> the community. You are part of the Boost community. I will be grateful
>> to receive a review based on whatever level of effort or time you can
>> devote.
>>
>
[snip]

And I vote to accept Beast. Sigh. It's always something.

Zach

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boos
Nik Bougalis via Boost
2017-07-02 18:55:30 UTC
Permalink
*What is your evaluation of the design?*

Overall, elegant. The decision to stick to C++11 forced some design
decisions, but nothing I can't live with.

My personal feelings about asio's interface aside, I believe that the clean
and natural fit of Beast interfaces with the asio model means that anyone
already familiar with asio (which is the golden standard in C++) will be
able to quickly and easily leverage Beast.

One small negative—and this not really specific to Beast—is that the
proliferation of templates (which, I think, are necessary to achieve the
elegant design Vinnie has) can make stack traces very hard to read and
follow. It's probably unfair to mention this in the Beast review especially
considering how asio is really a much bigger offender here.


*What is your evaluation of the implementation?*

We have been using Beast in production on rippled, a distributed payment
system that's handling hundreds of millions in daily volume, and have been
very pleased with the implementation.

I find the implementation to be high quality and robust, with attention
paid to detail.

As a data point on the websocket library: it easily outperformed the
previous websocket library we were using, even in early, less polished
versions, and was more robust and easier to use to boot.

I am pleasantly surprised at the thorough tests that the library comes with
and the author's dedication to quality code and speedy resolution of
existing issues.


*What is your evaluation of the documentation?*

Thorough and well-written. It includes useful examples that compile quickly
and easily and demonstrate the important concepts of the library.


*What is your evaluation of the potential usefulness of the library?*

Very useful. It provides functionality that's almost indispensable.

I've written an HTTP server in the past, for a chat application I was
developing. It was both painful and incredibly time-consuming to get
everything just right.

Beast would have abstracted all the HTTP boilerplate away, allowing me to
focus on the business logic.

The ability to develop clients just as easily as servers, with the same
pieces, is also great for the same reasons.

I understand that Beast isn't supposed to be a full fledged HTTP server or
HTTP client and that's just fine. What's important is that it provides a
strong foundation on which we can build what we need.

The websocket functionality is great for much the same reason.


*Did you try to use the library? With which compiler(s)? Did you have any
problems?*

Yes, I've used Beast both at Ripple and on my own personal projects.

I've compiled projects with Beast without issue on MSVC, GCC and Clang.


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

Significant: I've been involved with Beast since the early days and the
author has bounced ideas off of me. Additionally, I've been a user of the
library for a while and have a lot of real-world practical experience with
it.


*Are you knowledgeable about the problem domain?*

I'm not an HTTP or WebSocket expert, but I've implemented a
standard-compliant HTTP server in the past and have signficant experience
with network programming.

My position is that Beast should be *ACCEPTED* into Boost.

_______________________________________________
Unsubscribe & other changes: http://lists
Vinnie Falco via Boost
2017-07-03 21:17:58 UTC
Permalink
On Mon, Jul 3, 2017 at 9:54 AM, Emil Dotchevski via Boost
<***@lists.boost.org> wrote:
> ...
> The question is must Beast be coupled to Asio, not can it be used without
> something like Asio. And I don't mean just in the case of someone using
> serializer/basic_parser and nothing else.
>
> In my opinion, arguing that coupling is necessary should begin with
> defining an interface which Beast can use, which can be trivially
> implemented in terms of Asio. In designing this interface the only concern
> should be avoiding the coupling; specifically, performance considerations
> should be completely ignored. Only then we can have a reasonable discussion
> is it worth it. Regardless, in my experience this kind of exercise always
> improves the design of a library.

The question is, can Beast be written against generic concepts which
define synchronous and asynchronous operations over buffer oriented
streams, instead of being tied to Boost.Asio?

First lets imagine what such generic concepts might look like,
independent of Beast. We will start with the synchronous case which is
the easiest.

In the code which follows:

* `Stream` is a type representing a buffer oriented I/O stream

* `Buffers` is type representing an ordered sequence of zero
or more non-owning references to contiguous octet buffers

* `Error` is type representing an error code

The following synchronous stream operations are defined:

/** Read data from a stream synchronously
`stream` The stream to read from
`buffers` The buffers to read into
`error` Set to the error if any occurred.
returns: The number of bytes transferred, or 0 on error.
*/
std::size
read(
Stream& stream,
Buffers const& buffers,
Error& error);

/** Write data to a stream synchronously
`stream The stream to write to
`buffers The buffers to write from
`error` Set to the error if any occurred.
returns: The number of bytes transferred, or 0 on error.
*/
std::size_t
write(
Stream& stream,
Buffers const& buffers,
Error& error);

This looks pretty generic and at this point users familiar with
Boost.Asio might raise their hands to tell me that the interfaces look
identical. This is just a coincidence because, lets face it -
synchronous interfaces don't come in that many varieties. I don't
think synchronous interfaces are particularly interesting and Beast
could certainly be made to use something like this but even with
simple interfaces some issues come up:

1. What is the concrete type of Error? Or is Error a template
parameter that meets certain requirements?

2. HTTP stream read algorithms need to know when the stream, is
closed, what error code represents End-of-File?

3. What are the requirements of Buffers?

Beast could be written against generic concepts, but in order to do so
the questions above need to be answered (points 1 through 3).
Otherwise there is no specification and thus, no way to write an
algorithm. We will come back to these questions, but now on to the
next topic.

We want Beast to also provide HTTP operations on asynchronous streams,
so we need to have a generic concept against which to work. There are
many models of asynchronous computation. For example there are
futures, coroutines, callbacks, and queues (I can't really think of
any others at the moment). Lets imagine what callback-based generic
asynchronous stream read and write operations might look like:

In the code which follows:

* `Stream` is a type representing a buffer oriented I/O stream

* `Buffers` is type representing an ordered sequence of zero
or more non-owning references to contiguous octet buffers

* `Error` is type representing an error code

* `Callback` is a type which is Callable with the signature
`void(Error, std::size_t)`

The following asynchronous stream operations are defined:

/** Begin an operation to read data from a stream asynchronously
`stream` The stream to read from
`buffers` The buffers to read into
`callback` A callback to invoke when the operation completes.
The callback will receive the error if any, and the
number of bytes transferred.
returns: The number of bytes transferred, or 0 on error.
*/
void
async_read(
Stream& stream,
Buffers const& buffers,
Callback const& callback);

/** Begin an operation to write data to a stream synchronously
`stream` The stream to write to
`buffers` The buffers to write from
`callback` A callback to invoke when the operation completes.
The callback will receive the error if any, and the
number of bytes transferred.
returns: The number of bytes transferred, or 0 on error.
*/
void
async_write(
Stream& stream,
Buffers const& buffers,
Callback const& callback);

Now anyone who knows Asio is going to stop me and say, hey! This looks
a lot like Asio! But that's a side issue because this generic
interface I have provided has some defects. Or rather, you can say
that it is under-specified. We have the same questions about the
requirements for the types Error and Buffers, and the value
representing End-of-File. But now there are even bigger questions with
significant ramifications:

1. Upon which thread is the callback invoked?

2. Who invokes the callback and how is it invoked?

3. How is access to the `stream` synchronized?

4. What about futures and coroutines?

The simple generic free function signatures alone which I provided
above are not sufficient to describe a complete model for asynchronous
operations on streams. We also need provisions to answer the questions
above. Does the implementation provide the threads, or are they
provided by the user? Does synchronization use mutexes? Lock-free
programming? Queues? Back to this in a moment...

Some folks who either dislike callbacks or just happen to love futures
may raise an objection that the callback-based generic model I have
described does not capture their use case. Actually, Christopher
Kohlhoff (author of Boost.Asio) demonstrates how the callback-based
model is a proper superset of all other asynchronous computation
models. And he shows that callback-based models provide the greatest
opportunity for performance and efficiency by doing away with
unnecessary synchronization points.

This is explained in his paper N3896 "Library Foundations for
Asynchronous Operations" and shows how initiating functions like
`async_read` and `async_write` above may be trivially modified to
support not just callbacks but also futures, coroutines, and
user-defined types. Beast fully supports this system and even comes
with tutorials and helper classes which let you implement the model
yourself in your own initiating functions.

"Library Foundations for Asynchronous Operations" (N3896)
<http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n3896.pdf>

"Writing Composed Operations" (Beast)
<http://vinniefalco.github.io/stage/beast/review/beast/using_networking/writing_composed_operations.html>

Thankfully this solves one piece of our puzzle. N3896 allows us to say
with certainty that our hypothetical generic network model should use
callbacks as the foundational notification mechanism for the
completion of asynchronous operations.

Perhaps we can look towards the author of N3896 for more solutions? Drumroll...

Working Draft, C++ extensions for Networking (N4588)
a.k.a. "Networking-TS"
<http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/n4588.pdf>

What is this you ask? Well... this is a working draft which describes
a proposed set of C++ extensions for Networking. It has TCP/IP sockets
and UDP streams, serial ports I think (?) but lets not worry about
that. Most importantly, the Networking-TS defines a generic model for
synchronous and asynchronous stream-oriented I/O operations! This
means that:

Synchronous and asynchronous algorithms written against
Networking-TS concepts will work with any stream types
that meet the requirements **

Yes, Networking-TS comes with generic stream concepts. Lets restate
the original question:

"Can Beast be written against generic concepts which define
synchronous and asynchronous operations over buffer
oriented streams?"

The answer is:

Yes. Beast is already written against generic concepts which
define synchronous and asynchronous operations over buffer
oriented streams. These are provided by Boost.Asio.

Specifically, these concepts:

SyncReadStream
<https://timsong-cpp.github.io/cppwp/networking-ts/buffer.stream.reqmts.syncreadstream>

SyncWriteStream
<https://timsong-cpp.github.io/cppwp/networking-ts/buffer.stream.reqmts.syncwritestream>

AsyncReadStream
<https://timsong-cpp.github.io/cppwp/networking-ts/buffer.async.read>

AsyncWriteStream
<https://timsong-cpp.github.io/cppwp/networking-ts/buffer.async.write>

Its not the responsibility or goal of Beast to invent a new generic
model of synchronous or asynchronous networking. That's already been
taken care of. Beast follows it. It is the job of authors to make sure
their network implementations meet the requirements of the various
Networking-TS stream concepts. Once they do that, then any algorithms
written against the concepts can now work with these implementations.

Again folks might raise their hand and say "but Beast depends on
Boost.Asio!" and they would be right. The reality is that
Networking-TS has not entered the standard and is not uniformly
available across compiler vendors. Boost.Asio is the closest thing to
the standardized version of Networking-TS, so Beast is written against
that. Its not perfect, but it is the best we can do and the runner-up
is not even in the race (are there any other generic stream designs?).

I believe that the burden of proof rests with those who claim that
Beast should be written against a different generic stream design.
Please show me one such design that is mature, well-specified, and
in-use, other than Boost.Asio.

tl;dr; Boost.Asio's models of synchronous and asynchronous
buffer-oriented streams are already generic, and Beast is written
against Boost.Asio.

Thanks

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Klemens Morgenstern via Boost
2017-07-04 19:51:52 UTC
Permalink
> Please provide in your review whatever information you think is valuable
> to understand your final choice of ACCEPT or REJECT including Beast as a
> Boost library. Please be explicit about your decision.

Definite ACCEPT.

>
> Some other questions you might want to consider answering:
>
> - What is your evaluation of the design?
I think the design is very solid. If you are familiar with boost.asio,
beast seems quite natural. This also means that other methods to handle
asynchronous results (e.g. coroutines) can be used if they work with
asio, which is a big plus.
It is a bit confusing at some point that some functions are free
standing while others are members, but afaik there is some design reason
for that.
In addition I feel there's something strange when you have
`boost::asio::async_read` and `beast::http::async_read`. But I'm also
not clear how this can be used.

> - What is your evaluation of the implementation?
As far as I've seen it, it is very solid.

> - What is your evaluation of the documentation?
It is excellent and I had no problems understanding it, even with my
limited knowledge of http and websocket.
> - What is your evaluation of the potential usefulness of the library?
Very, very high, this library is needed. It is the next logical step
from boost.asio.
> - Did you try to use the library? With which compiler(s)? Did you
> have any problems?
I am using it in a real project since two weeks, http as well as
websocket. No problems thus far.
> - How much effort did you put into your evaluation? A glance? A quick
> reading? In-depth study?
Two weeks development.
> - Are you knowledgeable about the problem domain?
Familiar with asio, not with http & websocket. But I had not problem
getting started with beast even with my limited expertise.
>
> More information about the Boost Formal Review Process can be found
> here: <http://www.boost.org/community/reviews.html>
>
> Thank you for your effort in the Boost community.
>
> Happy coding -
> michael
>



_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Glen Fernandes via Boost
2017-07-05 01:30:46 UTC
Permalink
What follows is my review of the Beast library:

A. Should the library be accepted into Boost?

Yes. I vote ACCEPT, with no conditions for acceptance.

B. What is your evaluation of the design?

Good. As someone who is only barely familiar with using ASIO, it took some
time to understand and appreciate the library. Beast is designed with the same
goals that make the much of C++ standard library and other Boost libraries
useful: It tries to provide as much control to the user of the library, and
aims to make as few assumptions as possible that would alienate potential use
cases. If there is an avenue for customization, whether it is the policy for
dynamic allocation or otherwise, the library tries to expose it to the user.

C. What is your evaluation of the implementation?

The implementation is of a high quality, implemented using modern idiomatic
C++, careful chooses dependencies on other well maintained Boost libraries
when possible to avoid duplicating already well-implemented functionality. It
has generally correct support for the C++11 allocator model. Many of the
interfaces which are templates are additionally constrained with compile time
checks and assertions, to make sure that any user supplied types satisfy those
concepts correctly.

The library code looks to be well covered by tests, and the library's test
suite appears extensive, inluding benchmarks, code coverage, as well as tests
for implementation detail components (like various traits and helper
functions).

D. What is your evaluation of the documentation?

Excellent. Even though I believe the interfaces are well designed, the library
is sufficiently complex that without its current level of documentation, it
would have taken me several additional hours (especially as non-ASIO user) to
understand how to use it.

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

It looks very useful, albeit _to me_ (given my unfamiliarity with HTTP,
and with ASIO), not very simple to use without the aid of the examples.
Intuitively, I can see developers proficient with the domain being able
to use it to write powerful high level libraries or applications.

F. Did you try to use the library? With which compiler(s)?
Did you have any problems?

Yes. I tried the tests, examples, benchmarks with gcc 6.3 (with libstdc++),
with clang 3.9.1 (with libstdc++ and libc++). Additionally I wrote a HTTP
client program by copying an example and modifying it.

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

Not more than a fwe hours over the course of this review, which includes:
- Getting a little familiar with ASIO.
- Reading through the Beast implementation and examples code.
- Building and running the tests and examples
(with b2, against current Boost master branch).
- Using Beast to write a simple HTTP client program.

H. Are you knowledgeable about the problem domain?

Not about HTTP, Websockets, or even the ASIO library.
(Only about TCP, sockets, and general network I/O).

Some minor suggestions, after reading through the latest code:

1. Test Jamfile should be changed so that b2 simply works,
without first having to run b2 headers.

2. Include C library facility headers after C++ standard library headers.
For example:

#include <memory>
#include <cstdint>

3. Various default-initialization new[] could be boost::make_unique_noinit:

p_ = boost::make_unique_noinit<std::uint8_t[]>(capacity_);

rd_.buf = boost::make_unique_noinit<std::uint8_t[]>(rd_.buf_size);

4. Alternatively:
Perhaps those cases of std::unique_ptr could instead support allocators.

5. There are clamp() utilities defined in two places
(ranges.hpp and clamp.hpp).

Maybe all of these can exist in one place.

6. Trivial utilities (like clamp, ascii_tolower, etc.) could be constexpr.
Since you support C++11 they can be written as single return expression.

template<class T>
constexpr inline T clamp(T x, T y)
{
return x < y ? x : y;
}

7. Rely on Boost.Config when possible for detecting MSVC,
in case compilers emulate it.

Use BOOST_MSVC_FULL_VER instead of _MSC_FULL_VER.

8. Change the style of feature detection or defect macros,
to not be defined to 1 or 0.

Instead they should just be defined or not defined,
consistent with Boost.Config.

#define BEAST_NO_BOOST_STRING_VIEW

#if !defined(BEAST_NO_BOOST_STRING_VIEW)

9. string_view 'value' parameters might be more idiomatic (and even optimal),
instead of const-reference parameters.

bool iequals(string_view lhs, string_view rhs)

10. empty_base_optimization should check is_final on more than just clang.
Other C++ implementations also support this trait.

11. Maybe basic_multi_buffer(const Allocator&) be marked explicit.

Note: None of the above may be required, actionable, or even worth the author
considering. They are just thoughts that I noted while I was reading the code.

Thank you, Vinnie, for proposing your efforts for inclusion in Boost.

Glen

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Vinnie Falco via Boost
2017-07-05 02:11:36 UTC
Permalink
On Tue, Jul 4, 2017 at 6:30 PM, Glen Fernandes via Boost
<***@lists.boost.org> wrote:
> 5. There are clamp() utilities defined in two places
> (ranges.hpp and clamp.hpp).
>
> Maybe all of these can exist in one place.

There are 3 versions of clamp(), they are all slightly different, and
there are no Boost equivalents.

clamp.hpp is carefully designed to handle the case where Beast needs
to limit the size of a buffer to either max std::size_t or some larger
number that is represented as a 64-bit unsigned regardless of whether
the 32-bit or 64-bit address model is being built.

This ensures that 32-bit builds of Beast are capable of digesting
information whose size is stored in a 64-bit unsigned. For example, a
32-bit build of Beast can still process a chunked message body that
exceeds 2^32 bits in size.

The other file helps sweep zlib's numerous integer conversion warnings
under the rug without modifying the sources too much (so that I can
compare the Beast version against the original more easily).

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Robert Ramey via Boost
2017-07-05 04:12:29 UTC
Permalink
On 7/4/17 6:30 PM, Glen Fernandes via Boost wrote:
> Some minor suggestions, after reading through the latest code:
>
> 1. Test Jamfile should be changed so that b2 simply works,
> without first having to run b2 headers.

Hmmm - how can one do this? I presume tests and library include things like

#include <boost/config.hpp>

and

#include <boost/beast/...hpp>

How is this going to function without having run b2 headers first?

Robert Ramey



_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Glen Fernandes via Boost
2017-07-05 04:21:57 UTC
Permalink
On Wed, Jul 5, 2017 at 12:12 AM, Robert Ramey via Boost
<***@lists.boost.org> wrote:
> Hmmm - how can one do this? I presume tests and library include things like
>
> #include <boost/config.hpp>
>
> and
>
> #include <boost/beast/...hpp>
>
> How is this going to function without having run b2 headers first?

It should do it (generate all headers) automatically when you run b2
in that test directory.

For example:
$ git clone --recursive https://github.com/boostorg/boost
$ cd boost
$ ./bootstrap.sh
$ cd libs/smart_ptr/test
$ ../../../b2

Works fine without me having to run b2 headers explicitly.

Glen

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Vinnie Falco via Boost
2017-07-05 04:25:21 UTC
Permalink
On Tue, Jul 4, 2017 at 9:21 PM, Glen Fernandes via Boost
<***@lists.boost.org> wrote:
> $ cd libs/smart_ptr/test

Beast is not designed to build in-tree since it is not yet part of Boost

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Peter Dimov via Boost
2017-07-05 11:29:40 UTC
Permalink
Vinnie Falco wrote:
> On Tue, Jul 4, 2017 at 9:21 PM, Glen Fernandes via Boost
> <***@lists.boost.org> wrote:
> > $ cd libs/smart_ptr/test
>
> Beast is not designed to build in-tree since it is not yet part of Boost

The reason Boost libraries (usually) work without `b2 headers` is that
Jamfile.v2 in libs/ does this:

project boost/libs
: requirements <implicit-dependency>/boost//headers
;

If you add this implicit dependency to the Beast project in its Jamroot, it
should work too.


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Edward Diener via Boost
2017-07-05 15:24:28 UTC
Permalink
On 7/5/2017 12:25 AM, Vinnie Falco via Boost wrote:
> On Tue, Jul 4, 2017 at 9:21 PM, Glen Fernandes via Boost
> <***@lists.boost.org> wrote:
>> $ cd libs/smart_ptr/test
>
> Beast is not designed to build in-tree since it is not yet part of Boost

Still the ideal of reviewing a library usually means that the library
can be cloned beneath the Boost libs sub-directory and just "work",
whether the library is part of Boost or not.


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Robert Ramey via Boost
2017-07-05 16:26:51 UTC
Permalink
On 7/5/17 8:24 AM, Edward Diener via Boost wrote:
> On 7/5/2017 12:25 AM, Vinnie Falco via Boost wrote:
>> On Tue, Jul 4, 2017 at 9:21 PM, Glen Fernandes via Boost
>> <***@lists.boost.org> wrote:
>>> $ cd libs/smart_ptr/test
>>
>> Beast is not designed to build in-tree since it is not yet part of Boost
>
> Still the ideal of reviewing a library usually means that the library
> can be cloned beneath the Boost libs sub-directory and just "work",
> whether the library is part of Boost or not.

Hmmm - I'm not sure where that idea comes from. It's certainly not in
the official requirements for a boost review.

And there is a bigger problem here. When one puts his library "out
there" the hope is that people will use it and he will get feed back
from users, bug fixes etc. But to make the library easy to use within
boost is to make it hard to use outside of boost. A library should "just
work" whether or not the user has boost installed or not. or whether he
is running as a subdirectory of boost or not. Or whether he uses b2 or not.

This is a continuing problem in making boost easier to use for users and
easier to start using. We need to make changes to address this.

Robert Ramey

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


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Vinnie Falco via Boost
2017-07-05 16:32:33 UTC
Permalink
On Wed, Jul 5, 2017 at 9:26 AM, Robert Ramey via Boost
<***@lists.boost.org> wrote:
> When one puts his library "out there"
> the hope is that people will use it and he will get feed back from users,
> bug fixes etc. But to make the library easy to use within boost is to make
> it hard to use outside of boost. A library should "just work" whether or not
> the user has boost installed or not. or whether he is running as a
> subdirectory of boost or not. Or whether he uses b2 or not.

Yes that was my thinking as well when I set it up. However, Peter's
suggestion to add:

project boost/libs
: requirements <implicit-dependency>/boost//headers

Seems to work. Or rather, it didn't break anything. So the problem you
allude to might be easily fixed by just having a better "best
practices" document. Maybe there could be a "blank" repository under
boostorg/ that implements a "Hello world!" function in a header file
and is organized exactly like a boost library with tests, b2/cmake,
Travis, Appveyor, CircleCI integration, documentation, and works in or
out of tree.

Someone could just clone this blank project and then they are good to go.

Thanks

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Peter Dimov via Boost
2017-07-05 16:59:38 UTC
Permalink
Vinnie Falco wrote:
> Yes that was my thinking as well when I set it up. However, Peter's
> suggestion to add:
>
> project boost/libs
> : requirements <implicit-dependency>/boost//headers
>
> Seems to work. Or rather, it didn't break anything. So the problem you
> allude to might be easily fixed by just having a better "best practices"
> document.

The proper way to handle this would probably be to move everything you have
in Jamroot to a Jamfile, and have a blank Jamroot. This way, when the
library is put under $(BOOST_ROOT)/libs, one just needs to delete the empty
Jamroot for it to pick up the Boost Jamroot.


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Vinnie Falco via Boost
2017-07-05 17:10:08 UTC
Permalink
On Wed, Jul 5, 2017 at 9:59 AM, Peter Dimov via Boost
<***@lists.boost.org> wrote:
> ...when the
> library is put under $(BOOST_ROOT)/libs, one just needs to delete the empty
> Jamroot for it to pick up the Boost Jamroot.

That's a hassle because if you want to work with Git now what you are
making commits which remove or re-add the Jamroot?

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Robert Ramey via Boost
2017-07-05 17:12:09 UTC
Permalink
On 7/5/17 9:32 AM, Vinnie Falco via Boost wrote:
> On Wed, Jul 5, 2017 at 9:26 AM, Robert Ramey via Boost
> So the problem you
> allude to might be easily fixed by just having a better "best
> practices" document. Maybe there could be a "blank" repository under
> boostorg/ that implements a "Hello world!" function in a header file
> and is organized exactly like a boost library with tests, b2/cmake,
> Travis, Appveyor, CircleCI integration, documentation, and works in or
> out of tree.
>
> Someone could just clone this blank project and then they are good to go.

Actually it has been my intention to do this for Boost Library
Incubator. In fact, I crated Safe Numerics for this express purpose.
But now I've got a few problems.

a) The safe numerics library received a few comments which I felt I had
to respond to. I mentioned that my main purpose was to use it as a
vehicle to demonstrate how to make a boost library. Then the respondent
got a little testy that he might have wasted his time looking at it. So
I spent a little time enhancing it and addressing issues. This sort of
escalated, safe numerics wrapped it's fingers around my throat and
wouldn't let go. Years later - here I am. Now I've "boostified" it so
I need a new example....

b) Things have evolved since I made my advice section and it needs to be
enhanced and updated. We've got CI tools we didn't have then, Our CMake
initiatives may have born some fruit so things need some work.

c) Finally, we need to make some progress on the ability to use/test one
library. This is mostly an issue of crafting "best practices"
information. But still it's a fair amount of work.

Robert Ramey

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Vinnie Falco via Boost
2017-07-05 17:18:22 UTC
Permalink
On Wed, Jul 5, 2017 at 10:12 AM, Robert Ramey via Boost
<***@lists.boost.org> wrote:
> ...

I'd like a repository like boostorg/ci which I can add to my project
using either git-subtree or git-submodule which comes with handy
scripts and files such as travis.yml, appveyor.yml, circleci.yml
includes that I can use.

Unfortunately I don't think its possible to have .yml include files so
I will settle for a command line utility which reads a config file
which I create and then auto-generates CI files for Travis, Appveyor,
CircleCI, any others.

I would like it to support code coverage, valgrind, undefined behavior
sanitizer, address sanitizer, I want any of the tests that it runs to
run under a debugger on Travis and have the stack trace utilities so
if it crashes the Travis output shows a stack.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Edward Diener via Boost
2017-07-05 22:01:34 UTC
Permalink
On 7/5/2017 12:26 PM, Robert Ramey via Boost wrote:
> On 7/5/17 8:24 AM, Edward Diener via Boost wrote:
>> On 7/5/2017 12:25 AM, Vinnie Falco via Boost wrote:
>>> On Tue, Jul 4, 2017 at 9:21 PM, Glen Fernandes via Boost
>>> <***@lists.boost.org> wrote:
>>>> $ cd libs/smart_ptr/test
>>>
>>> Beast is not designed to build in-tree since it is not yet part of Boost
>>
>> Still the ideal of reviewing a library usually means that the library
>> can be cloned beneath the Boost libs sub-directory and just "work",
>> whether the library is part of Boost or not.
>
> Hmmm - I'm not sure where that idea comes from. It's certainly not in
> the official requirements for a boost review.
>
> And there is a bigger problem here. When one puts his library "out
> there" the hope is that people will use it and he will get feed back
> from users, bug fixes etc. But to make the library easy to use within
> boost is to make it hard to use outside of boost. A library should "just
> work" whether or not the user has boost installed or not. or whether he
> is running as a subdirectory of boost or not. Or whether he uses b2 or
> not.

You are saying that a library that is supposed to be reviewed for
inclusion in Boost is supposed to somehow work outside of Boost ? I do
not think so and can't even begin to understand that sort of logic.

>
> This is a continuing problem in making boost easier to use for users and
> easier to start using. We need to make changes to address this.
>
> Robert Ramey


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Vinnie Falco via Boost
2017-07-05 22:17:26 UTC
Permalink
On Wed, Jul 5, 2017 at 3:01 PM, Edward Diener via Boost
<***@lists.boost.org> wrote:
> You are saying that a library that is supposed to be reviewed for inclusion
> in Boost is supposed to somehow work outside of Boost ? I do not think so
> and can't even begin to understand that sort of logic.

Speaking for my own library Beast, I have developed a following of
users over the lifetime of the library and these users aren't cloning
the Beast repository into boost/libs/beast. They are getting it from
vcpkg, git-subtree or git-submodule into their own repository, or
cloning it out-of-tree.

It would be a poor experience if Beast did not build out-of-tree or if
it required users to put files into their system provided Boost
installation or to create their own Boost installation.

It was stated somewhere that Boost reviewers like to see instances of
libraries that are already being used in practice. A library can work
out-of-tree or it can be unpopular - pick one.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Edward Diener via Boost
2017-07-06 05:05:19 UTC
Permalink
On 7/5/2017 6:17 PM, Vinnie Falco via Boost wrote:
> On Wed, Jul 5, 2017 at 3:01 PM, Edward Diener via Boost
> <***@lists.boost.org> wrote:
>> You are saying that a library that is supposed to be reviewed for inclusion
>> in Boost is supposed to somehow work outside of Boost ? I do not think so
>> and can't even begin to understand that sort of logic.
>
> Speaking for my own library Beast, I have developed a following of
> users over the lifetime of the library and these users aren't cloning
> the Beast repository into boost/libs/beast. They are getting it from
> vcpkg, git-subtree or git-submodule into their own repository, or
> cloning it out-of-tree.
>
> It would be a poor experience if Beast did not build out-of-tree or if
> it required users to put files into their system provided Boost
> installation or to create their own Boost installation.
>
> It was stated somewhere that Boost reviewers like to see instances of
> libraries that are already being used in practice. A library can work
> out-of-tree or it can be unpopular - pick one.

My comment immediately below is not referencing Beast per se, which I
highly regard but about which I do not have enough programming knowledge
in its particular domain to give an intelligent review.

I am not arguing that you cannot make a library being reviewed for
inclusion in Boost work outside of the Boost tree. I am arguing that you
should make a library being reviewed for inclusion in Boost work in its
normal place below the Boost libs subdirectory. I get a bit ticked when
programmers who present there library for review can't be bothered with
this. As a potential reviewer I do not want to go through contortions
simply to be able to build a non-header only library, run tests for a
library, or create local documentation for the library. No matter how
brilliant a library might be if I can't clone that library below the
libs directory and use normal Boost means to build the library or run
the tests, as well as being given instructions for building the local
docs if bjam is not being used to do so, I am not interested in that
library at all just for starters.


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Robert Ramey via Boost
2017-07-06 04:44:55 UTC
Permalink
On 7/5/17 3:01 PM, Edward Diener via Boost wrote:

> You are saying that a library that is supposed to be reviewed for
> inclusion in Boost is supposed to somehow work outside of Boost ?

yep - that's exactly what I'm saying.

> I do
> not think so and can't even begin to understand that sort of logic.

It's pretty simple. A library submitted for review is not yet a boost
library. It's submitted with the idea that should it be accepted into
boost it will be "boostified". To require this extra work on a library
which might not be accepted has always been considered an overly onerous
requirement for review.

Off topic - I believe that boost libraries should also work outside of
boost.

Robert Ramey

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Mike Gresens via Boost
2017-07-06 18:21:50 UTC
Permalink
This is the result of my review of the Beast library


1) What is your evaluation of the design?

IMO it's natural that new boost libs are based on existing boost libs.
So it's fine that Beast is based on ASIO and uses ASIO types.

I had some discussions about the provided body types.
I think they could be more generic, e.g. sequence_body<Sequence> and
stream_body<Stream>.
But the provided body types are more than enough to work with them
out-of-the-box.


2) What is your evaluation of the implementation?

I did not investigate too much.
That are "implementation details" and most of the time i had the
perspective of an user.

The important part for me is the very high test coverage and tons of
unit tests.
Impressive: http://vinniefalco.github.io/autobahn/index.html


3) What is your evaluation of the documentation?

The documentation is pretty good.
It took me only 5min to port a curl based client to a Beast based client
(it's a really small http client).

May be there could be more hints for client/server developers.
E.g. where is the best place / how is the best practice to add
cookie-handling, Authentication challenges, etc.


4) What is your evaluation of the potential usefulness of the library?

Very useful. You could use it for simple http/websocket stuff -
out-of-the-box.
And more important - Beast might be the next step (after ASIO) to a
full-featured http-client implementation (with URLs, Cookies,
Authentication, etc.) or to a websocket implementation providing
multiplexing etc. in boost.


5) Did you try to use the library? With which compiler(s)? Did you have
any problems?

Yes, ported a curl based http client to Beast.
Very impressive was the speed of Beast: Beast based client was 2x faster
than curl based client.

Works fine with gcc-6, gcc-7 and clang-4 on Linux.
Works fine with Apple-clang 7.3 on OSX.


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

Two weeks.
Working with Beast as an user.
Contributed some small thinks to Beast as a developer.


7) Are you knowledgeable about the problem domain?

Not too much.
Doing json/hessian rpc calls using http(s) in a real application.
So investigated other libraries too like curl(pp), Poco::Net, cppnetlib,
etc.


8) Should the library be accepted into Boost?

Beast should be ACCEPTED.


Thank you, Vinnie, for all your work!

Best regards,
Mike


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Artyom Beilis via Boost
2017-07-06 20:32:12 UTC
Permalink
> May be there could be more hints for client/server developers.
> E.g. where is the best place / how is the best practice to add
> cookie-handling, Authentication challenges, etc.
>
>

Funnily, for someone who actually solved numerous of the issues this
question can easily be rephrased as:

> Now I finally learned how drive a bicycle, lets ask tips for driving semi-trailer.

And this is **the major** problem of the library.

In the CppCMS frameworks the HTTP part is less than 2.5% of the core
code. The rest is the "minor" stuff like cookies, session, url parsing
and much-much more. And note - I'm talking about server part only

Now once you choose to use Beast you'll hit the wall very quickly as
you'll have to do all tricks your own and they will consume most of
your work for example URL decoding, parsing query string, parsing or
generating trivial forms or handling trivial cookies. So instead of
concentrating on your application issues you are going to deal with
1001 small issues that require both experience and knowledge to do
them right.

The biggest problem is actually that vast majority security issues do
not come from HTTP parsing at all, but rather all "minor stuff" [1]
that left out of scope of the Beast.

This question you had written had popped for me the big red flag -
without proper well organized tools that handle the "minor stuff"
Beast users are virtually doomed to writing insecure code. Unlike most
HTTP servers/clients are build with security by design - do safe stuff
by default, Beast lives this ALL to end user.

This part **extremely** concerns me - as somebody who actually
developed both web services and tools to make them secure and aware of
unforgiving nature of WWW.

Artyom Beilis

[1]: http://cppcms.com/wikipp/en/page/secure_programming

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Nik Bougalis via Boost
2017-07-07 13:51:45 UTC
Permalink
On Thu, Jul 6, 2017 at 1:32 PM, Artyom Beilis via Boost
<***@lists.boost.org> wrote:
>
> > May be there could be more hints for client/server developers.
> > E.g. where is the best place / how is the best practice to add
> > cookie-handling, Authentication challenges, etc.
> >
> >
>
> Funnily, for someone who actually solved numerous of the issues this
> question can easily be rephrased as:
>
> > Now I finally learned how drive a bicycle, lets ask tips for driving semi-trailer.
>
> And this is **the major** problem of the library.
>
> In the CppCMS frameworks the HTTP part is less than 2.5% of the core
> code. The rest is the "minor" stuff like cookies, session, url parsing
> and much-much more. And note - I'm talking about server part only


There's no denying that "raw" HTTP is _almost_ simple before all
these other "minor" things that a full-fledged server or client needs are
thrown in.

But I think it's important to look at the goal of the library, per the author:
to enable others to write rich client and server libraries on top while it
takes care of the bottom-most plumbing in the most efficient way.

I think it achieves that. Hands down.


> Now once you choose to use Beast you'll hit the wall very quickly as
> you'll have to do all tricks your own and they will consume most of
> your work for example URL decoding, parsing query string, parsing or
> generating trivial forms or handling trivial cookies. So instead of
> concentrating on your application issues you are going to deal with
> 1001 small issues that require both experience and knowledge to do
> them right.
>
> The biggest problem is actually that vast majority security issues do
> not come from HTTP parsing at all, but rather all "minor stuff" [1]
> that left out of scope of the Beast.
>
> This question you had written had popped for me the big red flag -
> without proper well organized tools that handle the "minor stuff"
> Beast users are virtually doomed to writing insecure code. Unlike most
> HTTP servers/clients are build with security by design - do safe stuff
> by default, Beast lives this ALL to end user.
>
> This part **extremely** concerns me - as somebody who actually
> developed both web services and tools to make them secure and aware of
> unforgiving nature of WWW.

I appreciate you raising this, Artyom. Beast should be held to a high
standard, especially since it's handling user-input from untrusted
channels (i.e. socket buffers). So yeah, the code should be written
carefully, without thorough error checks, especially around the
handling of memory.

Based on my informal audit of the codebase, and my interactions with
Vinnie, I know that it is written carefully and am very confident that it
does not contain any bugs that would compromise the security of an
application built using it.

With that said, I also think you're exaggerating just a little bit. I just
don't think you need to be _THAT_ worried.

Yes, adding full "client" or "server" support, with cookie jars, and
authentication methods and tracking of origins, and who knows
what else, is not for the faint of heart.

Yes, some will try to implement such things themselves and they
will mess it up. Spectacularly. But is that different than _any_
other library? Can't people spectacularly mess up multithreaded
programming? Should we have held that against Boost.Thread?

Yes, if you want a full-fledged client or server library, there are
better alternatives than Beast. As Vinnie has said, Beast will
never be curl, or CppCMS and it doesn't need to!

The idea is to offer a robust implementation of the lowest HTTP
plumbing to (a) enable robust and clean WebSocket support
and (b) allow others to more quickly and easily write such
full-fledged client or server libraries layered on top of Beast
that are efficient without needing to worry about the bottom
layer of HTTP plumbling.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Vadim Zeitlin via Boost
2017-07-06 19:25:51 UTC
Permalink
On Sat, 1 Jul 2017 00:39:45 -0700 Michael Caisse via Boost <***@lists.boost.org> wrote:

MCvB> Please provide in your review whatever information you think is valuable
MCvB> to understand your final choice of ACCEPT or REJECT including Beast as a
MCvB> Boost library. Please be explicit about your decision.

Hello,

I'm sorry for lack of details in this review, I really hoped to find more
time to look into Beast, but it seems I'm not going to be able to do it
before the end of the review period, so I wanted to post a mini-review in
the hope that it can be better than nothing.

The most important limitation of this review is that I've only looked at
and used WebSocket-related parts of it and have only looked superficially
at the HTTP part despite the fact that it's the major part of the library.

MCvB> - What is your evaluation of the design?

IIUC, Beast was designed to be familiar to programmers with knowledge of
ASIO and it definitely succeeds in it. Personally, I can't say that I like
ASIO API that much, but there is no denying that Beast was much simpler to
pick up and start using immediately because it was so close to ASIO
conceptually.

MCvB> - What is your evaluation of the implementation?

I have only glanced at it and so didn't see much, but what I saw looked
good. I definitely didn't notice anything wrong.

MCvB> - What is your evaluation of the documentation?

Documentation is very good.

MCvB> - What is your evaluation of the potential usefulness of the library?

WebSocket part is very useful, I think it's the best current
implementation of the WebSocket protocol for C++. I don't have any use for
the HTTP part of myself, but I sure hope it could be useful for the
creation of future, high(er) level HTTP libraries that I'd love to have.

MCvB> - Did you try to use the library? With which compiler(s)? Did you
MCvB> have any problems?

I did use the library for some tests, in order to decide whether I was
going to rewrite an exiting client/server system to use it instead of
WebSocket++ (which I finally didn't, but for the reasons that are out of
scope of this review). I used it with MSVS 2015 and g++ 6 and didn't have
any problems with either.

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

A quick reading for the WebSocket part, at most a glance for the HTTP one.
I did spend a few hours writing small tests (client/server, sync/async,
plain/TLS) using the library.

MCvB> - Are you knowledgeable about the problem domain?

No, not particularly, I basically just read the RFC 6455, but mostly to
just have some idea of what was going on under the hood. I didn't have to
really know much about it in order to use Beast, which is a very good
thing, of course.


To finish this mini (micro?) review, I think that Beast should be accepted
into Boost if only for the WebSocket part. It both looks very good to me on
its own merits and in comparison with WebSocket++, which is, I think, the
only viable alternative for writing cross-platform clients/servers in C++
currently.

Thanks a lot to the author for his work and dedication!
VZ
Vinnie Falco via Boost
2017-07-06 19:47:03 UTC
Permalink
On Thu, Jul 6, 2017 at 12:25 PM, Vadim Zeitlin via Boost
<***@lists.boost.org> wrote:
> WebSocket part is very useful, I think it's the best current
> implementation of the WebSocket protocol for C++.

Thanks! Finally some love for the WebSockets!

> I don't have any use for the HTTP part of myself

Ah, but don't you see? A bit of background:

A WebSocket session is established when the client sends an HTTP
Upgade request, and the server sends back an appropriate HTTP Upgrade
response ("101 Switching Protocols")

beast::websocket::stream gives you full control over the WebSocket
HTTP handshake used at the start of the session. You can adjust header
fields in either the client or server roles. On the server you can
read the HTTP request yourself and dispatch it to your web server if
its not a websocket upgrade, else hand the beast::http::request object
to the beast::websocket::stream to initiate the websocket session.

tl;dr Users get the full power of Beast's HTTP message facilities to
get control over the handshake. If you want to do Basic
Authentication, request subprotocols, pass application specific data
or credentials, verify domains, interact with proxy fields, filter
messages, etc... this is all possible using the uniform HTTP
interfaces.

WebSocket handshake customizations are documented here:
<http://vinniefalco.github.io/beast/beast/using_websocket/handshaking_clients.html>
<http://vinniefalco.github.io/beast/beast/using_websocket/handshaking_servers.html>

Thanks

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Jared Wyles via Boost
2017-07-08 07:24:54 UTC
Permalink
Hello all,

I spent today playing with beast. I had been watching Vinnie build this
library on the cpp slack with keen interest.
This review comes after spending a day building something with boost. I
rewrote something I had written on top of ASIO to offer a http server and
web sockets to use the v8 inspector protocol similar to what nodeJS offers
but for an internal project.

I tried to use the docs, and in dire circumstances use the examples. where
possible and avoid reading the actual implementation code, so this review
is mostly about API design/documentation (given its mission statement, this
seems fair :D)


- What is your evaluation of the design?
With in an hour I had managed to go from a cursory understanding of beast
through random interactions on the cpp slack, to a working threaded http
server. I call this a win.

I had no real problems here, having an understanding of boost::asio
certainly put me in the right frame of mind.

I loved the fact that it avoided the usual temptation of stringly typed
status codes, header access etc. The Field access was a joy to work with
knowing I would get a compiler error on incorrect access.

It strikes the right balance between convenience and keeping a shallow API
surface. Things that you are likely to want access to all the time, status
code, path etc are available via helper methods.

I did run into a few doc issues which I have outlined to vinnie off list.
One is being tracked here [1]


Another that i think could be useful.

The docs here [2] could indicate that it will always include the leading
"/" one of those small things that I always umm and ahh over when writing
equality checks for items that uses strings for paths.

I wanted to call out that having a websocket library included in the one
library is a huge time saver. No having to worry about version
miss-matches, dependency tracking etc. In my implementation the debug
client requests a path using http GET. The server returns some information
and then a upgrade request comes in. Being able to call accept(request)
when i know i am expecting the upgrade and have it just work with out
having to think about it was great! A huge -1 from me to splitting the
libraries.

- What is your evaluation of the implementation?

I didn't look with much detail, the few times I had to debug some things
and stepped into the code, it looked well structured, easy to follow and
figure out where I went wrong.

There seems to be a good usage of static_assert to help avoid situations
where decorator_requests are expected over a request etc.

- What is your evaluation of the documentation?

As per above, I managed to get quite far just by reading the documentation.
The places where I fell over and had to read examples was around getting
data out of beast into standard types.
I started to piece it together by reading the code(and having a mental
shift that beast buffers are heavily based on asio buffers)
I eventually just used the example here [3]
I did reach for boost::asio::buffer_cast but couldn't get that to work.
I did find the error of my ways in [4] this method would be super useful in
beast it self!
Which leads me to...
One thing I am not sure, is there room in beast for more convenience
methods here, I can see a lot of library authors having to repeat a lot of
code to turn buffers -> string/ string streams etc. This could easily be
provided as a utility out side of beast though, or via simple documentation
in a FAQ to avoid having to reach for reading the implementation code.

Another issue I hit was websockets and buffers and who owns draining etc.
There is already a github issue for this [5] as well as a conversation on
this mailing list.

- What is your evaluation of the potential usefulness of the library?
On a purely selfish level. It meets a few of my immediate needs!

On a bigger level, the web is eating the world. Currently dealing with HTTP
in C++ is harder than it should be for simple things like rest, oauth, or
getting a simple server up and running quickly are harder than they should
be. The only rest client I know of that supports oauth 2 is cpprestsdk by
microsoft [6], this makes it much harder than it should be to talk to a
growing list of services only available via rest/graphql via http.

Beast has taken away the boring parts of doing all of that, so library
authors can focus on writing solid web based libraries for the C++
ecosystem. Hopefully if this is accepted in to boost this will be the
tipping point to more C++ http based libs!

- Did you try to use the library? With which compiler(s)? Did you have
any problems?

I used the library with
clang 4.0 and Apple LLVM version 8.0.0 (clang-800.0.42.1)
Compiled with -std=c++14

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

A fairly in-depth study of the api and the documentation. Along with
building a library on top of it.

A quick reading of the actual implementation details when having to debug
problems.

- Are you knowledgeable about the problem domain?

Yes. I have worked on the web for a while, across a variety of languages
and platforms. Have written more than 1 HTTP server and web socket
server/clients on top of boost::asio (they seem to follow me through my
career). Have worked on such web technologies such as blink/CEF/webkit.

My one line summary:
Beast should be ACCEPTED into Boost.

Jared.


[1] https://github.com/vinniefalco/Beast/issues/621
[2]
http://vinniefalco.github.io/beast/beast/ref/beast__http__message/target.html
[3]
http://vinniefalco.github.io/beast/beast/using_http/buffer_oriented_serializing.html
.
[4]
https://github.com/vinniefalco/Beast/blob/develop/test/core/buffer_test.hpp#L23
[5] https://github.com/vinniefalco/Beast/issues/611
[6] https://github.com/Microsoft/cpprestsdk.

On Sat, 1 Jul 2017 at 17:39 Michael Caisse via Boost <***@lists.boost.org>
wrote:

> The formal review of Vinnie Falco’s Beast library will take place from
> July 1 through July 10, 2017.
>
> Please consider participating in this review. The success of Boost is
> partially a result of the quality review process which is conducted by
> the community. You are part of the Boost community. I will be grateful
> to receive a review based on whatever level of effort or time you can
> devote.
>
> Beast is a C++ header-only library serving as a foundation for writing
> interoperable networking libraries by providing low-level HTTP/1,
> WebSocket, and networking protocol vocabulary types and algorithms using
> the consistent asynchronous model of Boost.Asio.
>
> Beast is designed for:
>
> * Symmetry: Algorithms are role-agnostic; build clients, servers, or
> both.
> * Ease of Use: Boost.Asio users will immediately understand Beast.
> * Flexibility: Users make the important decisions such as buffer or
> thread management.
> * Performance: Build applications handling thousands of connections
> or more.
> * Basis for Further Abstraction: Components are well-suited for
> building upon.
>
> A branch has been made for the review. You can find Beast for review at
> these links:
>
> * Documentation : <http://vinniefalco.github.io/stage/beast/review/>
> * Source code : <https://github.com/vinniefalco/Beast/tree/review>
> * The FAQ answers real questions that come up :
>
> <
> http://vinniefalco.github.io/stage/beast/review/beast/design_choices/faq.html
> >
>
> A lightning talk from CppCon 2016 introducing Beast can be found here:
> <https://www.youtube.com/watch?v=uJZgRcvPFwI>
>
>
> Please provide in your review whatever information you think is valuable
> to understand your final choice of ACCEPT or REJECT including Beast as a
> Boost library. Please be explicit about your decision.
>
> Some other questions you might want to consider answering:
>
> - What is your evaluation of the design?
> - What is your evaluation of the implementation?
> - What is your evaluation of the documentation?
> - What is your evaluation of the potential usefulness of the library?
> - Did you try to use the library? With which compiler(s)? Did you
> have any problems?
> - How much effort did you put into your evaluation? A glance? A quick
> reading? In-depth study?
> - Are you knowledgeable about the problem domain?
>
> More information about the Boost Formal Review Process can be found
> here: <http://www.boost.org/community/reviews.html>
>
> Thank you for your effort in the Boost community.
>
> Happy coding -
> michael
>
> --
> Michael Caisse
> Ciere Consulting
> ciere.com
>
> _______________________________________________
> Unsubscribe & other changes:
> http://lists.boost.org/mailman/listinfo.cgi/boost

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinf
Niall Douglas via Boost
2017-07-08 15:06:14 UTC
Permalink
I have seen enough reviews from others to be able to finish my review.
This is my formal review of the submitted Beast library.

> - What is your evaluation of the design?

It depends from where you are approaching.

As a niche Boost library of the kind most recent entrants to Boost are,
it's of good quality, solves a need, docs are good, testing is good,
implementation is not bad. One could argue it stops too far short for a
niche library, and it should fill in some of the obvious gaps as most
niche libraries do. But if accepted as so far most of the reviews
recommend, I think it'll fill out with time. It's not a concern for
acceptance.


As a fundamental essentials Boost library with potential for
standardisation (which the author has stated is the case, and I agree),
I found a lot of problems.

To clarify what I mean by "fundamental essentials", the really profound
thing about HTTP is that whenever you invent some mini request-reponse
protocol with tagged parameters and structured payload for some use
case, you inevitably end up reinventing HTTP. It's quite uncanny in
fact, almost disturbing, how often that wheel has been reinvented, and
usually very poorly. I have encountered so many lousy, low quality,
insecure, broken, memory corrupting, reinventions of HTTP in my career
it is quite depressing.

Hence there is a huge need for a properly written, fuzz tested,
low-level, lightweight, generalised request-response structured and
tagged data framework for C++ (and all languages) which is very widely
useful. Such a framework would have a very good chance of getting
standardised because it would be so damn useful for such a wide variety
of use cases, including traditional HTTP-over-socket, but also config
files, metadata, debug printing and so much more. Anything not so
general is correspondingly harder to standardise because I believe my
observations on this topic to be widely held and agreed with once people
think about it.

Why Beast falls far short of a fundamental essentials Boost library:

1. There is a lack of a well defined formal interface separating the
structured data parts from the i/o parts. This is never a good sign.

2. There is a lack of a well defined formal model abstracting away the
data parsing and generation model from a duplex stream model. Gavin said
a stream i/o model is a good one to target, I disagree. It is a
reasonable first but *wrong* guess to target, but in fact once you've
done a few of these you realise it is not optimal. Let me explain.

A generalised low-level request-response vocabulary library should not
impose a stream i/o model as there are so many widely used non-duplex
non-stream i/o models out there in use. In the past for example I've had
available to me a low bandwidth low latency transport and a high
bandwidth high latency transport. It makes huge sense to send the HTTP
headers via the low bandwidth low latency transport and the HTTP bodies
via the high bandwidth high latency transport. Another very common use
case is the ultra-high-latency transport better known as the file
system: how many times have people reinvented variations of
request-reponse protocol with tagged parameters and structured payload
now? INI, JSON, XML, TOML, YAML - they're all just a set of tagged
parameters and structured payload where request = read() and response =
write() over a non-duplex ultra high latency transport.

Those sorts of use case are easy implement with a generalised
"view/range of blobs of bytes" i/o model as I had proposed which is most
obviously implemented in C++ using a mix of span<T> and the Ranges TS.
It's much harder to implement, and with no corresponding gain either in
ease of use, with hard assumption of a duplex scatter-gather stream i/o
model. By hard coding the use of ASIO's inflexible and niche buffer
infrastructure, it makes Beast unusable as fundamental essentials
library, even if WebSocket, ASIO and all that other unnecessary stuff
were removed.

3. Minimum overhead design principles are not followed (zero copy, zero
malloc, zero blocking). The proposer will no doubt now aggressively
shout at me and harass me with "prove it in code". But I have given up
on trying to persuade him of anything, he makes it too unpleasant and mean.

Therefore as a fundamental essentials Boost library, Beast as proposed
is *not* generally useful because of how unnecessarily limited its use
cases are. If it were being reviewed as a fundamental essentials
library, it would have to be a REJECT with an encouragement that he's
done a good first attempt, but needs to think again.

> - What is your evaluation of the implementation?

The implementation is generally of high quality and I came away positive
from scanning the source code. Too much memory allocation, too much
memory copying for my taste, but it is entirely possible that that is
not important relative to the cost of implementing HTTP-over-socket anyway.

I had intended to run some comparative benchmarks between Beast and some
private code I wrote for clients here which uses all minimum overhead
design principles to see how much of a difference there might be. But
when the author made it clear he would not consider a departure from
being hardcoded to ASIO, I decided a further investment of my time here
would not be worth it, and it would require a fair few hours which are
better invested into Outcome v2 which is to be announced as feature
complete on Monday.

So all I can say is that the implementation is good enough for a niche
Boost library.

> - What is your evaluation of the documentation?

Pretty good. I got up to speed quickly. If the unnecessary parts of
Beast were removed and thus the documentation trimmed in size, it would
be better again.

> - What is your evaluation of the potential usefulness of the
> library?

No doubt it solves well its niche use cases.

> - Did you try to use the library? With which compiler(s)? Did you
> have any problems?

I was going to, but decided not to bother given that the author would
not be receptive to any additional effort invested by me.

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

About six hours. I did look at the library in detail back when
Boost.Http was being developed under GSoC, but I've not been paying much
attention since.

> - Are you knowledgeable about the problem domain?

Very. I have worked several contracts where a lightweight subset of HTTP
was implemented using all sorts of unusual non-socket transports. I have
been working on the ultimate low level foundation layer for all future
C++ i/o since 2012, and I have given very deep thought on how best to
implement minimum overhead i/o on very low latency hardware devices,
some of which has been discussed over several years with broad general
agreement as to my approach with various WG21 members, some very senior.

> Thank you for your effort in the Boost community.

Before I give my formal recommendation, I wish to preempt the inevitable
angry response to this review by telling the author that he did a good
library, but he's done a lousy review. One of the very hardest, most
valuable, rarest thing you can obtain in any knowledge based community
is high quality useful feedback on what's possible and desirable by
experts in their fields. That sort of high quality advice costs an
enormous sum of money to rent in, yet you can get it for free here if
you play your cards right. Had the proposer come here looking to make
the ultimate low level HTTP library, he could have come away with a
wealth of useful ideas of what's possible.

Instead he came here with a very narrow goal of getting his existing
library, tolerating only very minor changes, into Boost. And that's
okay, it's a fine niche library, but such a wasted opportunity on what
he could have achieved instead. But his loss in the end.


With all that being said, the final question to answer is whether Beast
is a niche Boost library or a fundamental essentials Boost library?

My own personal opinion (as is obvious above) sways towards fundamental
essentials. After all, it's supposed to be a *low-level* HTTP library.
But the wider community, based on the content of reviews to date, appear
to be thinking niche. I had a feeling that that would be the case, and
that's why I wanted to wait until enough reviews came in for me to be
able to call it.

Following the majority opinion that Beast is a niche not a fundamental
essentials Boost library, I therefore vote to ACCEPT the library without
conditions. It's a good library and a valuable addition to Boost. Well
done Vinnie!

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
Barrett Adair via Boost
2017-07-08 19:48:41 UTC
Permalink
On Sat, Jul 1, 2017 at 2:39 AM, Michael Caisse via Boost
<***@lists.boost.org> wrote:
> The formal review of Vinnie Falco’s Beast library will take place from
> July 1 through July 10, 2017.

(snip)

I recommend to ACCEPT Beast into Boost.

> - What is your evaluation of the design?

I quite like it. I appreciate the limited, low-level scope. The
"design choices" page is convincing to me, although I am relatively
unfamiliar with the domain.

> - What is your evaluation of the implementation?

It's large, so I randomly poked around the code for about an hour. I
have no issues to raise, although I found it interesting that Beast
implements SHA1 on its own, and also contains a complete header-only
port of zlib.

> - What is your evaluation of the documentation?

I'm highly impressed. The documenation is excellent.

> - What is your evaluation of the potential usefulness of the library?

Very useful. I'm excited about Beast, and I think its acceptance would
add significant value to Boost.

> - Did you try to use the library? With which compiler(s)? Did you
> have any problems?

I did not try to use the library.

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

One hour reading docs and one hour reading code.

> - Are you knowledgeable about the problem domain?

No. I rarely write code that deals with HTTP directly, and I've never used ASIO.

Thanks,
Barrett

_______________________________________________
Unsubscribe & other changes: http://l
Daniela Engert via Boost
2017-07-09 16:38:09 UTC
Permalink
Hi Vinnie,
hi folks,

this is my first review, so please bear with me ...

Am 01.07.2017 um 09:39 schrieb Michael Caisse via Boost:> The formal
review of Vinnie Falco’s Beast library will take place from> July 1
through July 10, 2017.
I recommend to ACCEPT Beast into Boost
> - What is your evaluation of the design?

I like it because
* it resembles ASIO which I am quite familiar with
* it is focused on what it tries to accomplish
* it clearly states it's dependency on Boost.ASIO with the strong
emphasis on migrating to Networking TS when this becomes a thing

That said, I agree on the opinion which other reviewers already stated
on that it lacks some convenience helper functions when it comes to
draining or construction of requests. I'd love to see those added to the
library.

> - What is your evaluation of the implementation?

I didn't dig too deep into it but took a first cursory look at all files
before doing anything useful with Beast. Lateron when playing with the
library I looked into the bowels of some classes or functions to
understand what I might have done wrong and fix my mistakes.

I ran the full test-suite in my usual configuration quadrupel
(debug/release + 32/64 bit) at warning level 4 in non-permissive mode
using msvc 14.1 from VisualStudio 2017.2, all four tests passed with the
same results (unlike a couple of other Boost libraries). I noticed the
things, though:

* Beast's zlib emits a few harmless but stupid warnings C4127
"conditional expression is constant" (Microsoft, get rid of them!) and
quite some narrowing warnings C4244. These need some cleansing before we
can use Beast in our team because of our /W4 /WX policy.

* Beast's test-suite uses the deprecated Boost.Coroutine library which
leads to deprecation warnings

* there is an error in the Jamfile of the test-suite which creates a
truncated compiler options that msvc complains about (this is already
rectified in a later commit, though)

> - What is your evaluation of the documentation?

I like it a lot. It seems to be complete and sufficiently detailed so
that I didn't have to reach out too often to aunt Goo to find an answer
to my questions. I am missing some examples on how to use some of these
constructs in real life. For example I haven't figured out yet how to
use buffer_cat to build a request body from a mixture of different
buffer types.

> - What is your evaluation of the potential usefulness of the library?

It's very useful. From my perspective it make my life easier as it
provides just those tools that I require to deal with simple HTTP messages.

Besides the usefulness of it's own I'd love to see some higher level
library which implements the finer parts needed by HTTP clients.

> - Did you try to use the library? With which compiler(s)? Did you> have any problems?

I've set up a small toy project using msvc 14.1, Boost 1.64.0, and our
team's build system. It interrogates a SOAP server provided by the
developers of my tv-set for the weekly betatest firmware with it's
non-predictable download URL. I didn't implement the actual download so
far because Beast's file_body is not part of the review branch.

I didn't encounter any major problems.

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

In total I spent about 15 hours in the past two days studying the
documentation, running the test-suite, compiling the examples, and
implementing my toy-project.

> - Are you knowledgeable about the problem domain?
Barely - just enough understanding about the basics of HTTP messages and
using them to talk to (embedded) HTTP servers. I am just a humble user
of these things.

Ciao
Dani


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/lis
Loading...