Hi all - We are half-way through the review and have some great involvement and exchanges and a few reviews already. I hope if you haven't written a review yet that you still will. Please provide whatever effort you can afford toward the review process. Boost depends on community involvement. You might have noticed that I have solicited input from you regardless of your experience with the domain. I personally think it is a mistake to have input only from domain experts. True innovation often comes from "outsiders". If you have no experience with Asio or with HTTP, how approachable is this library? Were you able to understand the documents? Could you use the examples? Were you able to build something of your own? Domain experts -- you know the dark corners of the domain. The hard parts that have caused you hours of lost sleep and the need to hack around an existing library or to write your own. How does Beast address your concerns and the problems you have experienced in the past? Is it flexible enough to be put into your tool chest? So far we have reviews from: * Marcel Ebmer * Peter Dimov * Artyom Beilis * Zach Lain * Nik Bougalis * Klemens Morgenstern * Glen Fernandes If I missed a review from you, please let me know! Happy Coding! michael -- Michael Caisse Ciere Consulting ciere.com
On 6/07/2017 17:19, Michael Caisse wrote:
We are half-way through the review and have some great involvement and exchanges and a few reviews already.
I hope if you haven't written a review yet that you still will. Please provide whatever effort you can afford toward the review process. Boost depends on community involvement.
You might have noticed that I have solicited input from you regardless of your experience with the domain. I personally think it is a mistake to have input only from domain experts. True innovation often comes from "outsiders". If you have no experience with Asio or with HTTP, how approachable is this library? Were you able to understand the documents? Could you use the examples? Were you able to build something of your own?
I'm probably not going to post an actual formal review (since I haven't looked closely enough at it) but I did have some thoughts: 1. The name is more than a bit silly; while silly names are not in themselves a problem (see Hana, Spirit, etc, and the FAQ that brings up the same argument), this one sounds like it was chosen simply because it was phonetically similar to "Boost", which seems potentially confusing at the least. 2. The design rationale makes a big deal of limiting the scope of the library and being low-level. That's fine (and a good thing, in my opinion), but it makes me question what the WebSocket part is doing there. Perhaps I just have the wrong impression of something from the docs, but it seems like the HTTP part is focused on simply parsing and generating the HTTP message format (and thus conceptually could do so with any stream abstraction, including a non-socket-based one), but then the WebSocket part goes beyond that to include actual socket management and connection lifetimes. This impression was reinforced from some earlier discussion about the different interfaces between the HTTP part (which is stateless static methods) and the WebSocket part (which is stateful instance methods). If this impression is correct, it seems like it would be better to split Beast into two separate libraries; one that is "more" low-level and has just the HTTP (and perhaps WebSocket) stateless parsing and generation components only, and then a second library sitting on top of this that provides the WebSocket connection management. Perhaps similar to how the author brought up possible future HTTP server/client libraries as extensions on top of Beast. 3. The documentation seems extensive, well-written, and easy to follow. (I count myself as reasonably familiar with both Asio and HTTP, although not an expert.) 4. In the FAQ "There's no HTTP/2 support yet!" the phrase "Users cannot work with HTTP/1 now" does not make sense and I suspect there's a typo somewhere.
On Wed, Jul 5, 2017 at 11:51 PM, Gavin Lambert via Boost <boost@lists.boost.org> wrote:
2. The design rationale makes a big deal of limiting the scope of the library and being low-level. That's fine (and a good thing, in my opinion), but it makes me question what the WebSocket part is doing there.
`beast::websocket::stream` is a low level protocol layering on top of Boost.Asio, just like HTTP is. You have to establish the connection yourself, maintain timeouts, manage a send queue for writing, and that sort of thing.
it seems like the HTTP part is focused on simply parsing and generating the HTTP message format (and thus conceptually could do so with any stream abstraction, including a non-socket-based one)
Against which yet-unspecified stream abstraction do you feel Beast's algorithms should be written? The rationale for the current choice of using Boost.Asio's stream abstractions is explained here here: <http://boost.2283326.n4.nabble.com/review-beast-Review-of-Beast-starts-today-July-1-July-10-tc4696294.html#a4696441>
but then the WebSocket part goes beyond that to include actual socket management and connection lifetimes.
Which socket and connection lifetime management features are you referring to?
4. In the FAQ "There's no HTTP/2 support yet!" the phrase "Users cannot work with HTTP/1 now" does not make sense and I suspect there's a typo somewhere.
You're right about that, someone else pointed this out in their review (its been fixed in a new branch). I meant to say that we shouldn't wait for HTTP/2 to give users HTTP/1. Thanks
On 6/07/2017 19:24, Vinnie Falco wrote:
On Wed, Jul 5, 2017 at 11:51 PM, Gavin Lambert wrote:
it seems like the HTTP part is focused on simply parsing and generating the HTTP message format (and thus conceptually could do so with any stream abstraction, including a non-socket-based one)
Against which yet-unspecified stream abstraction do you feel Beast's algorithms should be written? The rationale for the current choice of using Boost.Asio's stream abstractions is explained here here: <http://boost.2283326.n4.nabble.com/review-beast-Review-of-Beast-starts-today-July-1-July-10-tc4696294.html#a4696441>
I said that being based on a *stream* abstraction is a good thing. As that is lower-level than a *socket* abstraction, and could be used on other streams that aren't sockets (eg. a non-socket-based networking library on some other platform, or for simulated conversations in unit tests). I somewhat agree with Niall's point that fundamentally this doesn't even need to be based on streams and it could be based on byte blob containers instead, but since in practical use it will most likely be used with streams anyway and using blob containers as the basic interface could lead to unnecessary copies, I think using streams was the right call. And if you're looking for a stream abstraction, it's hard to beat Asio's, so that was also a good call.
but then the WebSocket part goes beyond that to include actual socket management and connection lifetimes.
Which socket and connection lifetime management features are you referring to?
The HTTP code acts on streams. The WebSocket code acts on sockets, not streams. (Evidence: nowhere in the HTTP docs could I find a method that used anything other than an abstract stream as a parameter. Whereas in the WebSocket docs the very first thing talks about handing over ownership of a socket to the WebSocket stream class, and this is the basis for everything else.) This is clearly a higher level than the HTTP code. Maybe there's some technical benefit to having this in one library rather than being split into two, but then that should probably be explained in the design choices. What I was suggesting is that the WebSocket parsing and formatting code be kept in Beast (eg. the code that generates the byte blob that represents a frame; as this would be stateless stream-based similar to the HTTP code) but the part that deals with sockets should be split to a separate library Beast.WebSocket (or whatever). Otherwise the design seems a bit inconsistent -- it's low-level except for websockets.
On Thu, Jul 6, 2017 at 4:23 PM, Gavin Lambert via Boost <boost@lists.boost.org> wrote:
I said that being based on a *stream* abstraction is a good thing. As that is lower-level than a *socket* abstraction, and could be used on other streams that aren't sockets
Thanks for clarifying, I think I misread. Beast HTTP and WebSockets are both based on Asio *stream* abstractions, not socket abstractions. Beast does not use socket abstractions anywhere, subject to the caveat that stream must indicate the end of the connection/data by delivering `boost::asio::error::eof` from a call to read_some or async_read_some.
(eg. a non-socket-based networking library on some other platform, or for simulated conversations in unit tests).
Yes! This is exactly how the Beast tests work: <https://github.com/vinniefalco/Beast/blob/78a065ba39836d91d7e70d93de7f9140f518083b/extras/beast/test/pipe_stream.hpp#L30> I will add two examples to the documentation of SyncReadStream and SyncWriteStream adapters which allow std::istream and std::ostream derived classes respectively to operate with unmodified Beast HTTP stream algorithms. I allude to them (bottom of page) because I have not written them yet, but they will be written: <http://vinniefalco.github.io/stage/beast/review/beast/using_http/buffer_oriented_serializing.html#beast.using_http.buffer_oriented_serializing.write_to_std_ostream>
...fundamentally this doesn't even need to be based on streams and it could be based on byte blob containers
This is true for synchronous algorithms, but those are trivial. Blob containers by themselves prescribe no specifications for asynchronous operation. That's where the real bread and butter is: it is Beast's asynchronous algorithms which adhere to the Extensible Asynchronous Model and Executors (asio::io_service is an early version of this concept) that provide the lion's share of value.
The HTTP code acts on streams. The WebSocket code acts on sockets, not streams. (Evidence: nowhere in the HTTP docs could I find a method that used anything other than an abstract stream as a parameter. Whereas in the WebSocket docs the very first thing talks about handing over ownership of a socket to the WebSocket stream class, and this is the basis for everything else.) This is clearly a higher level than the HTTP code.
Ah! I see the confusion now. Both HTTP and WebSocket in Beast operate on stream algorithms. However, a WebSocket session has state whose lifetime must be managed. HTTP/1 does not. Beast's interface for HTTP operations uses simple free functions, while the WebSocket interface uses a different approach. It offers a wrapper which wraps your SyncStream or AsyncStream. Depending on how you declare the wrapper, it is either a non-owning reference, or an owning wrapper. Examples: // non-owning boost::asio::ip::tcp::socket sock{ios}; beast::websocket::stream<boost::asio::ip::tcp::socket&> ws{sock}; // owning boost::asio::ip::tcp::socket sock{ios}; beast::websocket::stream<boost::asio::ip::tcp::socket> ws{std::move(sock)}; That the examples show ip::tcp::socket is only because those are the streams which come with Asio (plus ssl::stream). Beast WebSocket wrappers work with any synchronous or asynchronous stream. With beast::websocket::stream, the implementation does not know that you are working with a socket. It doesn't know anything about IP addresses, endpoint_type, making connections, accepting connections, or any of that. Its the callers responsibility to provide an already-connected stream object that meets the requirements, socket or otherwise. Although HTTP/1 is stateless, HTTP/2 is not. When Beast gains an HTTP/2 implementation (a planned feature) it will use the same wrapper style as websocket: namespace beast { namespace http { /** An HTTP/2 stream. @tparam Stream A type meeting the requirements of AsyncReadStream and AsyncWriteStream */ template<class Stream> class stream { BOOST_STATIC_ASSERT( is_async_read_stream<Stream>::value && is_async_write_stream<Stream>::value); Stream stream_; public: template<class... Args> stream(Args&&... args) : stream_(std::forward<Args>(args)...) { } ... }; } // http } // beast Due to the requirements of HTTP/2 (over-engineered and overly complex in my opinion) it is not possible to provide a synchronous implementation. And unfortunately the implementation must depart from Beast's philosophy of leaving some things up to the user, a full HTTP/2 implementation must take over the "read pump" in order to enforce stream prioritization.
What I was suggesting is that the WebSocket parsing and formatting code be kept in Beast (eg. the code that generates the byte blob that represents a frame; as this would be stateless stream-based similar to the HTTP code) but the part that deals with sockets should be split to a separate library Beast.WebSocket (or whatever).
That makes the library objectively worse; users overwhelmingly want to operate WebSockets with sockets and ssl streams. And beast::websocket::stream already works with stream concepts (not sockets). Thanks
On 7/07/2017 11:49, Vinnie Falco wrote:
On Thu, Jul 6, 2017 at 4:23 PM, Gavin Lambert wrote:
The HTTP code acts on streams. The WebSocket code acts on sockets, not streams. (Evidence: nowhere in the HTTP docs could I find a method that used anything other than an abstract stream as a parameter. Whereas in the WebSocket docs the very first thing talks about handing over ownership of a socket to the WebSocket stream class, and this is the basis for everything else.) This is clearly a higher level than the HTTP code.
Ah! I see the confusion now. Both HTTP and WebSocket in Beast operate on stream algorithms. However, a WebSocket session has state whose lifetime must be managed. HTTP/1 does not. Beast's interface for HTTP operations uses simple free functions, while the WebSocket interface uses a different approach. It offers a wrapper which wraps your SyncStream or AsyncStream. Depending on how you declare the wrapper, it is either a non-owning reference, or an owning wrapper.
Ah, thank you, that resolves my concern.
Although HTTP/1 is stateless, HTTP/2 is not. When Beast gains an HTTP/2 implementation (a planned feature) it will use the same wrapper style as websocket:
If you know that this will be required in future, then why not use that same style for HTTP/1 today? It might not be necessary but it ensures a consistent interface (and it leaves you open to possibly storing some state if it turns out to be necessary for some other future HTTP/1 extension).
On 8/07/2017 01:20, Vinnie Falco wrote:
On Thu, Jul 6, 2017 at 11:37 PM, Gavin Lambert wrote:
If you know that this will be required in future, then why not use that same style for HTTP/1 today? ... It might not be necessary...
Looks like you answered your own question :)
Perhaps, although as I said in the unquoted parts of my message, you might be painting yourself into a corner if future changes do require some state.
On Sun, Jul 9, 2017 at 5:55 PM, Gavin Lambert via Boost <boost@lists.boost.org> wrote:
...you might be painting yourself into a corner if future changes do require some state.
HTTP/2 will be a separate implementation that uses the stream wrapper as I described. The existing HTTP/1 free functions will not be touched. HTTP/2 will re-use the same message container.
On 07/07/2017 01:49 AM, Vinnie Falco via Boost wrote:
Ah! I see the confusion now. Both HTTP and WebSocket in Beast operate on stream algorithms. However, a WebSocket session has state whose lifetime must be managed. HTTP/1 does not. Beast's interface for HTTP operations uses simple free functions, while the WebSocket interface
Are HTTP operations really stateless? How does FieldsReader's chunked and keep_alive fit into this picture?
Due to the requirements of HTTP/2 (over-engineered and overly complex in my opinion) it is not possible to provide a synchronous implementation. And unfortunately the implementation must depart from Beast's philosophy of leaving some things up to the user, a full HTTP/2 implementation must take over the "read pump" in order to enforce stream prioritization.
I assume that you are thinking about HTTP/2 multiplexing here. The situation is somewhat similar to unconnected UDP sockets, where you can receive datagrams from multiple endpoints on the same socket. It is possible to emulate connected UDP sockets (you can also make UDP socket connected directly, but it has some unwanted consequences) with pseudo- sockets that appear to be connected to a single endpoint even if it communications over an unconnected UDP socket. Here is an example of that: https://github.com/breese/trial.datagram
On Sun, Jul 9, 2017 at 7:40 AM, Bjorn Reese via Boost <boost@lists.boost.org> wrote:
On 07/07/2017 01:49 AM, Vinnie Falco via Boost wrote: Are HTTP operations really stateless? How does FieldsReader's chunked and keep_alive fit into this picture?
From Beast's perspective, there is no state carried from one message to the next. Within a single message there is state, this is captured by the basic_parser and serializer objects for reading and writing respectively.
Callers will usually have state. For example the caller has to inform the parser not to expect a response body when the sent request specified method==HEAD. To answer your question, yes, Beast does not maintain state from one message to the next on HTTP connections. Therefore, free functions to send and receive are the logical choice of interface.
I assume that you are thinking about HTTP/2 multiplexing here.
Yes. I haven't studied the HTTP/2 specification in depth but my first interpretation is that a correct implementation needs to have calls to async_read issued on behalf of the caller in order to implement stream prioritization and resource limits.
My Review Vote--------------Unconditional ACCEPT What is your evaluation of the design?--------------------------------------Beast's design is meant to fit in well with asio, and it achieves that. Ibelieve this is a good decision, as people can use their existing knowledge ofasio to learn beast. I also appreciate that beast's API can easily be used to build both client andserver code. What is your evaluation of the implementation? -----------------------------------------------The implementation is expertly done. There are traits to check concepts, and thecode was clearly built by someone with a deep understanding of C++. What is your evaluation of the documentation?---------------------------------------------Concepts are clearly documented, there are good examples, and there is areference for the API. Also the documentation is well formatted and easy to readand navigate. What is your evaluation of the potential usefulness of the library? -------------------------------------------------------------------Extremely useful. While Beast is a low-level library, and will not receive asmuch use as a high level library built on top of it, it will still see wide use.In addition, it can form a base that high level libraries can use. Did you try to use the library? With which compiler(s)? Did you have any problems? ----------------------------------------------------------------------------------Yes, I have used beast with both clang 4, gcc 6, and gcc 7. In addition to thework I've done with beast at ripple, for this review I built a toy web serverproject that serves up a web page and opens a websocket that communicates withthe server. How much effort did you put into your evaluation?-------------------------------------------------In-depth study. I was involved in several code reviews of beast. Are you knowledgeable about the problem domain?-----------------------------------------------I am knowledgeable about Unix network programming. I am less knowledgeable abouthttp and web development. Other: ------I'd like to note four additional things: 1) Beast has extensive tests and the principal developer is serious about test coverage. 2) Beast has an extremely fast development cadence and the principal developer is willing to tackle large tasks to reach goals he sets for beast. As an example, once it was decided to make Beast header-only, zlib was ported to C++ and made header only. 3) The principal developer is responsive to bug reports and other feedback. 4) The runtime performance of beast websockets was significantly better than the websocket library we were using at Ripple. All these things should also count in beast's favor. Disclaimer: -----------I worked with Vinnie at Ripple.
- If you have no experience with Asio or with HTTP, how approachable is this library? Were you able to understand the documents? Could you use the examples? Were you able to build something of your own? Very approachable. I was able to git-submodule the github project, add a couple lines to my CMakeLists.txt, and compose bits from the examples into just what I needed. The one major gotcha for a new user is asio's lack of configurable timeouts on sync methods. This seems to be a common problem for new asio users and moving from sync examples to async real-world code can be painful. I'd like to see an upfront note about this or more async examples as I expect this library to be popular with users completely new to asio. - What is your evaluation of the design? I haven't studied it enough to say - What is your evaluation of the implementation? It's template-heavy which is outside my normal comfort zone. Many error messages are more confusing than they'd be with procedural or oop code. Compilers have been making good progress with this and beast includes many static asserts inside its templates to help. - What is your evaluation of the documentation? It's all there and searchable with good examples. I had trouble finding the websocket binary flag without a google site search but once I found it I saw it's in the obvious place - What is your evaluation of the potential usefulness of the library? Very high. Its main competitors are complex to set up or bloated/highly coupled to a large set of libraries - Did you try to use the library? With which compiler(s)? Did you have any problems? Yes, I'm using it in production and haven't had any issues. Beast has contributed no extra compiler warnings or ubsan/asan problems to my own code aside from one false positive which Vinnie quickly fixed. I wouldn't expect any problems as the tests cover virtually everything. using clang 3.8.0 and gcc 5.4.0 and boost 1.58.0, which the author has kept compatible through some effort as the minimum version - How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? A few days using beast to add embedded http servers to existing applications, port existing libcurl client code, and write a new websocket client for an existing server - Are you knowledgeable about the problem domain? Not really, I was looking for a library to take care of this - should beast be accepted into boost? Yes, accept. I avoided using boost for many years because of integration headaches and compilation time. I've come around and now use it for basic cross-platform support for things that in other languages would be part of the standard library (like boost::filesystem and, before c++11, boost::thread). Not having a standard way to do http is an embarrassment and I'd like to see this accepted asap to hasten the inevitable libraries that will go on top and have even more value for users looking for high abstraction/simple use cases. Michael Larson On Wed, Jul 5, 2017 at 10:19 PM, Michael Caisse via Boost < boost@lists.boost.org> wrote:
Hi all -
We are half-way through the review and have some great involvement and exchanges and a few reviews already.
I hope if you haven't written a review yet that you still will. Please provide whatever effort you can afford toward the review process. Boost depends on community involvement.
You might have noticed that I have solicited input from you regardless of your experience with the domain. I personally think it is a mistake to have input only from domain experts. True innovation often comes from "outsiders". If you have no experience with Asio or with HTTP, how approachable is this library? Were you able to understand the documents? Could you use the examples? Were you able to build something of your own?
Domain experts -- you know the dark corners of the domain. The hard parts that have caused you hours of lost sleep and the need to hack around an existing library or to write your own. How does Beast address your concerns and the problems you have experienced in the past? Is it flexible enough to be put into your tool chest?
So far we have reviews from:
* Marcel Ebmer * Peter Dimov * Artyom Beilis * Zach Lain * Nik Bougalis * Klemens Morgenstern * Glen Fernandes
If I missed a review from you, please let me know!
Happy Coding! michael -- Michael Caisse Ciere Consulting ciere.com
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/ mailman/listinfo.cgi/boost
On Thu, Jul 6, 2017 at 12:10 PM, Michael Larson via Boost <boost@lists.boost.org> wrote:
I avoided using boost for many years because of integration headaches and compilation time. I've come around and now use it for basic cross-platform support for things that in other languages would be part of the standard library
This makes me very happy! (the part where you use Boost now, not the part about the headaches and compilation time)
participants (6)
-
Bjorn Reese -
Gavin Lambert -
Michael Caisse -
Michael Larson -
Scott Determan -
Vinnie Falco