zaphoyd / websocketpp Goto Github PK

There is a couple of places where the async_write method is called using a local string variable as the buffer parameter. The places are

void client::connection<connection_type>::write_request() {
.....
boost::asio::async_write(
m_connection.get_socket(),
boost::asio::buffer(raw),
.....

and

void server::connection<connection_type>::write_response() {
.....
boost::asio::async_write(
m_connection.get_socket(),
boost::asio::buffer(raw),

I fixed the issue by using a reference counted buffer from an asio example. The class code is

    class shared_const_buffer {
    public:
        explicit shared_const_buffer(const std::string &data) : m_data(new std::vector<char>(data.begin(), data.end())),
            m_buffer(boost::asio::buffer(*m_data)) {}
    public:
        typedef boost::asio::const_buffer value_type;
        typedef const boost::asio::const_buffer *const_iterator;
        const boost::asio::const_buffer *begin() const { return &m_buffer; }
        const boost::asio::const_buffer *end() const { return &m_buffer + 1; }
    private:
        boost::shared_ptr< std::vector<char> > m_data;
        boost::asio::const_buffer m_buffer;
    };

and replacing "boost::asio::buffer(" by "shared_const_buffer"

Plus a mistyping:

diff --git a/src/roles/server.hpp b/src/roles/server.hpp

•        throw http::exception("Recieved invalid HTTP Request",http::status_code::BAD_REQUEST);
  

•        throw http::exception("Received invalid HTTP Request",http::status_code::BAD_REQUEST);
  

Options:

• Throw exception
• Log error and set value to maximum allowed
• Log error and leave value at whatever it was before

See section 7.2.3

Right now this is all up to the end application. Should any of this be supported by the library?

utf8 validation is not being done.

diff --git a/src/rng/boost_rng.cpp b/src/rng/boost_rng.cpp

• boost::random::uniform_int_distribution<>(INT32_MIN,INT32_MAX)); {}
• boost::random::uniform_int_distribution<>(INT32_MIN,INT32_MAX)) {}

diff --git a/src/processors/hybi_header.cpp b/src/processors/hybi_header.cpp

-void hybi_header::set_opcode(frame::opcode::value op) {
+void hybi_header::set_opcode(websocketpp::frame::opcode::value op) {

diff --git a/src/processors/hybi_header.hpp b/src/processors/hybi_header.hpp

• void set_opcode(frame::opcode::value op);
• void set_opcode(websocketpp::frame::opcode::value op);

diff --git a/src/messages/data.cpp b/src/messages/data.cpp

•    m_payload.reserve(std::max(new_size,static_cast<uint64_t>(2*m_payload.capacity())));
  

•    m_payload.reserve(std::max<uint64_t>(new_size, static_cast<uint64_t>(2*m_payload.capacity())));
  

-void data::reset(frame::opcode::value opcode) {
+void data::reset(websocketpp::frame::opcode::value opcode) {

diff --git a/src/messages/data.hpp b/src/messages/data.hpp

• void reset(frame::opcode::value opcode);
• void reset(websocketpp::frame::opcode::value opcode);

I was using Boost 1.47 compiled from source.
Now library build without issue on both, shared and static. But is a problem compiling examples, same problem on four examples.

root@xubuntu:/DEVELOP/websocketpp/examples/chat_client# make
g++ -c -O2 -o chat_client.o chat_client.cpp
g++ -c -O2 -o chat_client_handler.o chat_client_handler.cpp
g++ -O2 chat_client.o chat_client_handler.o -o chat_client -lboost_system -lboost_thread -lboost_date_time -lboost_regex -lboost_random ../../libwebsocketpp.a
../../libwebsocketpp.a(websocket_session.o): In function websocketpp::session::session(boost::asio::io_service&, boost::shared_ptrwebsocketpp::connection_handler, unsigned long long)':
websocket_session.cpp:(.text+0x2345): undefined reference toboost::random::random_device::random_device()'
websocket_session.cpp:(.text+0x23a0): undefined reference to boost::random::random_device::random_device()'
websocket_session.cpp:(.text+0x24c9): undefined reference toboost::random::random_device::~random_device()'
websocket_session.cpp:(.text+0x24e5): undefined reference to boost::random::random_device::~random_device()'
websocket_session.cpp:(.text+0x25c7): undefined reference toboost::random::random_device::~random_device()'

.
.
.

Thanks!

All issues bellow are in the endpoint.hpp file.

1. The endpoint::close_all method iterates the m_connections set and calls the close method of every element. the result is a crash, because the close method calls the endpoint::remove_connection method which removes the connection from the m_connections member. Quick but not "nice" fix is

•    for (it = m_connections.begin(); it != m_connections.end(); it++) {
  

•        (*it)->close(code,reason);
  

•    }
  

•    while(!m_connections.empty()) { // using for is not correct because the close method calls the remove_connection, which removes connection from m_connections
  

•        (*m_connections.begin())->close(code,reason);
  

•    }
  

1. Ambiguous access of 'm_io_service'. Can be either the endpoint_base or the role or the socket classes. Fix is

   explicit endpoint(handler_ptr handler)

2. : role_type(m_io_service),
   

3.   socket_type(m_io_service),
   

4. : role_type(endpoint_base::m_io_service),
   

5.   socket_type(endpoint_base::m_io_service),
   m_state(IDLE),
   

6. The same as previous issue but in different place

        << "Endpoint is stopping immediately" << log::endl;
   

•        m_io_service.stop();
  

•        endpoint_base::m_io_service.stop();
       m_state = STOPPED;
  

1. Under Windows the following lines either can't be compiled or do not define correct friend class declarations and as result the "can't access to protected method" error

   friend class role< endpoint<role,socket> >;
   friend class socket< endpoint<role,socket> >;
   friend class connection<type,role< type >::template connection,socket< type >::template connection>;

Quick but not 100% correct fix is to use #if defined(WIN32) ... #else ... #endif with the following lines which are commented in the header file

 friend role_type; 
 friend socket_type;
 friend connection_type;

1. Simple mistyping.

•    << "Endpoint recieved signal to close all connections cleanly with code " 
  

•    << "Endpoint received signal to close all connections cleanly with code " 
  

Per section 4.4 the server should return a list of version headers in the response if the handshake fails due to the client's advertised version being unsupported.

During the switch from read until CRLF to frame lengths frames or partial frames can discarded if they were sent in the same TCP packet as the handshake.

Add an API to the connection handler to allow local applications to send messages in fragments as data becomes available rather than waiting until the entire message has been buffered.

Handshake acceptance code is currently hard coded to accept only HTTP/1.1

max messages sizes (see section 10.4)
time waiting for a handshake after connecting
number of connections per endpoint (see section 4.1 part 3)

At minimum, catch SIGINT and close all connections/write logs cleanly. Consider how this might work on non-unix systems?

note this depends on the field type. Sec-WebSocket-Version requests must be singled valued, error responses may be multi-valued (Sec 11.3). subprotocol may be multivalued, also extensions

HTTP basic auth on 401, redirects on 3xx, etc

see section 4.1, 10.5

is required by the spec

see section 10.6

Add an API to the connection handler to allow local applications to receive message data as it arrives rather than waiting until the entire fragment has been buffered.

In the issue #50 the 1st problem was linked with the endpoint::close_all method. And the fix is not correct due to the fact that the remove_connection method is not called on every close method and result is a never-ending loop. Here is a new fix:

-        while(!m_connections.empty()) {



       (*m_connections.begin())->close(code,reason);



   }



          for (std::set<connection_ptr>::iterator it_con = m_connections.begin(); it_con != m_connections.end();) {



              const connection_ptr con = *it_con++;



              con->close(code,reason);



          }

The logic is that initially the existent iterator is incremented and after may be called the m_connections.erase() method. Before the issue #50  the for circle incremented the iterator after the m_connections.erase()  call.