rpcdaemon: add HTTP gzip compression (no streaming) #1909
Conversation
lupin012
force-pushed
the
http_compression_no_streaming
branch
from
831e849
to
06c3797
Compare
canepat
changed the title
| @@ -190,10 +192,11 @@ Task<void> Connection::handle_actual_request(const boost::beast::http::request<b | |||
| vary_ = req[boost::beast::http::field::vary]; | |||
| origin_ = req[boost::beast::http::field::origin]; | |||
| method_ = req.method(); | |||
| auto encoding = req[boost::beast::http::field::accept_encoding]; | |||
|
|
|||
| auto rsp_content = co_await request_handler_.handle(req.body()); | |||
| if (rsp_content) { | |||
There was a problem hiding this comment.
The definition of local variable encoding should be moved inside this code branch for better readability
| @@ -190,10 +192,11 @@ Task<void> Connection::handle_actual_request(const boost::beast::http::request<b | |||
| vary_ = req[boost::beast::http::field::vary]; | |||
| origin_ = req[boost::beast::http::field::origin]; | |||
| method_ = req.method(); | |||
| auto encoding = req[boost::beast::http::field::accept_encoding]; | |||
There was a problem hiding this comment.
encoding should be declared const (next PR is OK for this and all following suggestions)
|
|
||
| auto rsp_content = co_await request_handler_.handle(req.body()); | ||
| if (rsp_content) { | ||
| co_await do_write(rsp_content->append("\n"), boost::beast::http::status::ok); | ||
| co_await do_write(rsp_content->append("\n"), boost::beast::http::status::ok, !encoding.empty()); |
There was a problem hiding this comment.
The value of the Accept-Encoding field should be passed to the do_write method instead of a bool. This will allow for checking within do_write which kind of content compression has been requested and accept/reject it depending on the supported compression on our side
| compress_data(content, compressed_data); | ||
| } catch (const std::exception& e) { | ||
| SILK_ERROR << "Connection::compress_data exception: " << e.what(); | ||
| throw; |
There was a problem hiding this comment.
Not sure we do really want to raise an exception here, probably we could directly return an Internal server error code or something similar
| @@ -114,6 +116,8 @@ class Connection : public StreamWriter { | |||
| std::string vary_; | |||
| std::string origin_; | |||
| boost::beast::http::verb method_{boost::beast::http::verb::unknown}; | |||
|
|
|||
| char temp_compressed_buffer_[10 * 1024 * 1024]; | |||
There was a problem hiding this comment.
We should use std::string or std::vector<char> here
No description provided.