Skip to content

Issue #844 Add host information to log messages #45

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 13 commits into from
Closed

Issue #844 Add host information to log messages #45

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
f9317cc
Add host information to log messages
jmjatlanta May 11, 2018
f26dbd4
swapped host/payload positions in log message
jmjatlanta May 21, 2018
03dbdab
Add host information to log messages
jmjatlanta May 11, 2018
8591cfa
swapped host/payload positions in log message
jmjatlanta May 21, 2018
39ce52b
Attempt to capture forwarded host
jmjatlanta Dec 31, 2018
71bf39e
fixed conflicts
jmjatlanta Dec 31, 2018
cbad73b
Add host information to log messages
jmjatlanta May 11, 2018
d690e70
swapped host/payload positions in log message
jmjatlanta May 21, 2018
e0fae07
Attempt to capture forwarded host
jmjatlanta Dec 31, 2018
7c9a453
Merge branch 'jmj_issue_844' of ssh://github.com/bitshares/bitshares-…
jmjatlanta Dec 31, 2018
c3941d8
Attempting to fix bad commit
jmjatlanta Dec 31, 2018
afb50f6
Add custom host header
jmjatlanta Apr 24, 2019
f8c8e81
Merge branch 'master' into jmj_issue_844
jmjatlanta Apr 24, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions include/fc/network/http/websocket.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ namespace fc { namespace http {
fc::any& get_session_data() { return _session_data; }

virtual std::string get_request_header(const std::string& key) = 0;
virtual std::string get_host() = 0;

fc::signal<void()> closed;
private:
Expand Down
61 changes: 25 additions & 36 deletions src/network/http/websocket.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,6 @@ namespace fc { namespace http {
typedef base::con_msg_manager_type con_msg_manager_type;
typedef base::endpoint_msg_manager_type endpoint_msg_manager_type;

/// Custom Logging policies
/*typedef websocketpp::log::syslog<concurrency_type,
websocketpp::log::elevel> elog_type;
typedef websocketpp::log::syslog<concurrency_type,
websocketpp::log::alevel> alog_type;
*/
//typedef base::alog_type alog_type;
//typedef base::elog_type elog_type;
typedef websocketpp::log::stub elog_type;
typedef websocketpp::log::stub alog_type;

Expand Down Expand Up @@ -76,14 +68,6 @@ namespace fc { namespace http {
typedef base::con_msg_manager_type con_msg_manager_type;
typedef base::endpoint_msg_manager_type endpoint_msg_manager_type;

/// Custom Logging policies
/*typedef websocketpp::log::syslog<concurrency_type,
websocketpp::log::elevel> elog_type;
typedef websocketpp::log::syslog<concurrency_type,
websocketpp::log::alevel> alog_type;
*/
//typedef base::alog_type alog_type;
//typedef base::elog_type elog_type;
typedef websocketpp::log::stub elog_type;
typedef websocketpp::log::stub alog_type;

Expand Down Expand Up @@ -116,8 +100,6 @@ namespace fc { namespace http {
typedef base::con_msg_manager_type con_msg_manager_type;
typedef base::endpoint_msg_manager_type endpoint_msg_manager_type;

//typedef base::alog_type alog_type;
//typedef base::elog_type elog_type;
typedef websocketpp::log::stub elog_type;
typedef websocketpp::log::stub alog_type;

Expand Down Expand Up @@ -158,8 +140,7 @@ namespace fc { namespace http {

virtual void send_message( const std::string& message )override
{
idump((message));
//std::cerr<<"send: "<<message<<"\n";
idump( (message) );
auto ec = _ws_connection->send( message );
FC_ASSERT( !ec, "websocket send failed: ${msg}", ("msg",ec.message() ) );
}
Expand All @@ -173,6 +154,14 @@ namespace fc { namespace http {
return _ws_connection->get_request_header(key);
}

virtual std::string get_host()override
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please take a look at what I am doing here. I am attempting to get a "custom" header to help with the log message. This is untested code, but I would like some input to (in)validate my approach.

{
auto header = get_request_header("BS-Forwarded");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Header name should be configurable like @abitmore said, and absence of configured value means ignore all headers and use remote host of connection.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented a configurable header. Will need to adjust in bitshares-core to make use of it.

if (header.length() > 2)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why 2?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because I love magic numbers. Perhaps you don't like 2, so I replaced it with 3. (actually replaced with !string::empty() )

return header;
return _ws_connection->get_host();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think get_host is wrong, according to the docs it returns the host component of the requested URI, not the remote host.

Copy link
Author

@jmjatlanta jmjatlanta Apr 24, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will investigate. Please do not bother reviewing until I edit this comment.

}

T _ws_connection;
};

Expand All @@ -198,12 +187,11 @@ namespace fc { namespace http {
_server_thread.async( [&](){
auto current_con = _connections.find(hdl);
assert( current_con != _connections.end() );
wdump(("server")(msg->get_payload()));
//std::cerr<<"recv: "<<msg->get_payload()<<"\n";
auto payload = msg->get_payload();
std::shared_ptr<websocket_connection> con = current_con->second;
std::shared_ptr<websocket_connection> ws_con = current_con->second;
wdump( ("server") (ws_con->get_host() ) (payload) );
++_pending_messages;
auto f = fc::async([this,con,payload](){ if( _pending_messages ) --_pending_messages; con->on_message( payload ); });
auto f = fc::async([this,ws_con,payload](){ if( _pending_messages ) --_pending_messages; ws_con->on_message( payload ); });
if( _pending_messages > 100 )
f.wait();
}).wait();
Expand All @@ -222,11 +210,11 @@ namespace fc { namespace http {
auto con = _server.get_con_from_hdl(hdl);
con->defer_http_response();
std::string request_body = con->get_request_body();
wdump(("server")(request_body));
wdump( ("server") (current_con->get_host()) (request_body) );

fc::async([current_con, request_body, con] {
std::string response = current_con->on_http(request_body);
idump((response));
idump( ("server") (current_con->get_host()) (response) );
con->set_body( response );
con->set_status( websocketpp::http::status_code::ok );
con->send_http_response();
Expand Down Expand Up @@ -334,8 +322,9 @@ namespace fc { namespace http {
auto current_con = _connections.find(hdl);
assert( current_con != _connections.end() );
auto received = msg->get_payload();
std::shared_ptr<websocket_connection> con = current_con->second;
fc::async([con,received](){ con->on_message( received ); });
std::shared_ptr<websocket_connection> ws_con = current_con->second;
wdump( ("server") (ws_con->get_host()) (received) )
fc::async([ws_con,received](){ ws_con->on_message( received ); });
}).wait();
});

Expand All @@ -347,14 +336,14 @@ namespace fc { namespace http {
_on_connection( current_con );

auto con = _server.get_con_from_hdl(hdl);
wdump(("server")(con->get_request_body()));
wdump( ("server") (con->get_host()) (con->get_request_body()) );
auto response = current_con->on_http( con->get_request_body() );
idump((response));
idump( ("server") (current_con->get_host()) (response) );
con->set_body( response );
con->set_status( websocketpp::http::status_code::ok );
} catch ( const fc::exception& e )
{
edump((e.to_detail_string()));
edump( (current_con->get_host()) (e.to_detail_string()) );
}
current_con->closed();

Expand Down Expand Up @@ -428,7 +417,7 @@ namespace fc { namespace http {
_client.clear_access_channels( websocketpp::log::alevel::all );
_client.set_message_handler( [&]( connection_hdl hdl, message_ptr msg ){
_client_thread.async( [&](){
wdump((msg->get_payload()));
wdump( (_connection->get_host()) (msg->get_payload()) );
//std::cerr<<"recv: "<<msg->get_payload()<<"\n";
auto received = msg->get_payload();
fc::async( [=](){
Expand Down Expand Up @@ -488,7 +477,7 @@ namespace fc { namespace http {
_client.clear_access_channels( websocketpp::log::alevel::all );
_client.set_message_handler( [&]( connection_hdl hdl, message_ptr msg ){
_client_thread.async( [&](){
wdump((msg->get_payload()));
wdump( (_connection->get_host()) (msg->get_payload()) );
_connection->on_message( msg->get_payload() );
}).wait();
});
Expand Down Expand Up @@ -671,7 +660,7 @@ namespace fc { namespace http {

auto con = my->_client.get_connection( uri, ec );

if( ec ) FC_ASSERT( !ec, "error: ${e}", ("e",ec.message()) );
if( ec ) FC_ASSERT( !ec, "uri: ${uri} error: ${e}", ("uri", uri) ("e",ec.message()) );

my->_client.connect(con);
my->_connected->wait();
Expand All @@ -698,7 +687,7 @@ namespace fc { namespace http {

auto con = smy->_client.get_connection( uri, ec );
if( ec )
FC_ASSERT( !ec, "error: ${e}", ("e",ec.message()) );
FC_ASSERT( !ec, "uri: ${uri} error: ${e}", ("uri", uri) ("e",ec.message()) );
smy->_client.connect(con);
smy->_connected->wait();
return smy->_connection;
Expand All @@ -721,7 +710,7 @@ namespace fc { namespace http {
auto con = my->_client.get_connection( uri, ec );
if( ec )
{
FC_ASSERT( !ec, "error: ${e}", ("e",ec.message()) );
FC_ASSERT( !ec, "uri: ${uri} error: ${e}", ("uri", uri) ("e",ec.message()) );
}
my->_client.connect(con);
my->_connected->wait();
Expand Down
8 changes: 8 additions & 0 deletions tests/network/http/websocket_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,20 @@

#include <fc/network/http/websocket.hpp>

#include <fc/log/logger.hpp>
#include <fc/log/console_appender.hpp>

#include <iostream>

BOOST_AUTO_TEST_SUITE(fc_network)

BOOST_AUTO_TEST_CASE(websocket_test)
{
// set up logging
fc::shared_ptr<fc::console_appender> ca(new fc::console_appender);
fc::logger l = fc::logger::get("rpc");
l.add_appender(ca);

fc::http::websocket_client client;
fc::http::websocket_connection_ptr s_conn, c_conn;
int port;
Expand Down