Split HttpServer::AddRequestHandler into two methods Spec requires handle only subset of HTTPS paths on HTTP port. BUG:24789091 Change-Id: Ic47818003f6a75ae3267a2d3a591d2576c73ac40 Reviewed-on: https://weave-review.googlesource.com/1277 Reviewed-by: Vitaly Buka <vitalybuka@google.com>
diff --git a/libweave/examples/ubuntu/event_http_server.cc b/libweave/examples/ubuntu/event_http_server.cc index 87834f0..f2c136c 100644 --- a/libweave/examples/ubuntu/event_http_server.cc +++ b/libweave/examples/ubuntu/event_http_server.cc
@@ -60,10 +60,9 @@ class HttpServerImpl::RequestImpl : public Request { public: RequestImpl(evhttp_request* req, provider::TaskRunner* task_runner) - : path_{evhttp_request_uri(req)}, task_runner_{task_runner} { - path_ = path_.substr(0, path_.find("?")); - path_ = path_.substr(0, path_.find("#")); + : task_runner_{task_runner} { req_.reset(req); + uri_ = evhttp_request_get_evhttp_uri(req_.get()); std::vector<uint8_t> data(evbuffer_get_length(req_->input_buffer)); evbuffer_remove(req_->input_buffer, data.data(), data.size()); @@ -73,7 +72,10 @@ ~RequestImpl() {} - std::string GetPath() const override { return path_; } + std::string GetPath() const override { + const char* path = evhttp_uri_get_path(uri_); + return path ? path : ""; + } std::string GetFirstHeader(const std::string& name) const override { const char* header = evhttp_find_header(req_->input_headers, name.c_str()); if (!header) @@ -93,11 +95,11 @@ } private: - std::unique_ptr<InputStream> data_; std::unique_ptr<evhttp_request, decltype(&evhttp_cancel_request)> req_{ nullptr, &evhttp_cancel_request}; - std::string path_; provider::TaskRunner* task_runner_{nullptr}; + std::unique_ptr<InputStream> data_; + const evhttp_uri* uri_{nullptr}; }; HttpServerImpl::HttpServerImpl(EventTaskRunner* task_runner) @@ -126,8 +128,6 @@ CHECK(httpsd_); evhttp_set_bevcb(httpsd_.get(), BuffetEventCallback, ctx_.get()); - evhttp_set_gencb(httpd_.get(), ProcessRequestCallback, this); - evhttp_set_gencb(httpsd_.get(), ProcessRequestCallback, this); CHECK_EQ(0, evhttp_bind_socket(httpd_.get(), "0.0.0.0", GetHttpPort())); CHECK_EQ(0, evhttp_bind_socket(httpsd_.get(), "0.0.0.0", GetHttpsPort())); @@ -174,16 +174,15 @@ } void HttpServerImpl::ProcessRequest(evhttp_request* req) { - std::string path = evhttp_request_uri(req); - for (auto i = handlers_.rbegin(); i != handlers_.rend(); ++i) { - if (path.compare(0, i->first.size(), i->first) == 0) { - std::unique_ptr<RequestImpl> request{new RequestImpl{req, task_runner_}}; - i->second.Run(std::move(request)); - return; - } + std::unique_ptr<RequestImpl> request{new RequestImpl{req, task_runner_}}; + std::string path = request->GetPath(); + auto it = handlers_.find(path); + if (it != handlers_.end()) { + return it->second.Run(std::move(request)); } NotFound(req); } + void HttpServerImpl::ProcessReply(std::shared_ptr<RequestImpl> request, int status_code, const std::string& data, @@ -191,9 +190,18 @@ } -void HttpServerImpl::AddRequestHandler(const std::string& path_prefix, - const RequestHandlerCallback& callback) { - handlers_.emplace(path_prefix, callback); +void HttpServerImpl::AddHttpRequestHandler( + const std::string& path, + const RequestHandlerCallback& callback) { + handlers_.emplace(path, callback); + evhttp_set_cb(httpd_.get(), path.c_str(), &ProcessRequestCallback, this); +} + +void HttpServerImpl::AddHttpsRequestHandler( + const std::string& path, + const RequestHandlerCallback& callback) { + handlers_.emplace(path, callback); + evhttp_set_cb(httpsd_.get(), path.c_str(), &ProcessRequestCallback, this); } uint16_t HttpServerImpl::GetHttpPort() const {
diff --git a/libweave/examples/ubuntu/event_http_server.h b/libweave/examples/ubuntu/event_http_server.h index 0f21787..f4fda75 100644 --- a/libweave/examples/ubuntu/event_http_server.h +++ b/libweave/examples/ubuntu/event_http_server.h
@@ -28,8 +28,10 @@ explicit HttpServerImpl(EventTaskRunner* task_runner); - void AddRequestHandler(const std::string& path_prefix, - const RequestHandlerCallback& callback) override; + void AddHttpRequestHandler(const std::string& path_prefix, + const RequestHandlerCallback& callback) override; + void AddHttpsRequestHandler(const std::string& path_prefix, + const RequestHandlerCallback& callback) override; uint16_t GetHttpPort() const override; uint16_t GetHttpsPort() const override; std::vector<uint8_t> GetHttpsCertificateFingerprint() const override;
diff --git a/libweave/include/weave/provider/http_server.h b/libweave/include/weave/provider/http_server.h index 5119229..ce0b684 100644 --- a/libweave/include/weave/provider/http_server.h +++ b/libweave/include/weave/provider/http_server.h
@@ -33,9 +33,13 @@ using RequestHandlerCallback = base::Callback<void(std::unique_ptr<Request> request)>; - // Adds callback called on new http/https requests with the given path prefix. - virtual void AddRequestHandler(const std::string& path_prefix, - const RequestHandlerCallback& callback) = 0; + // Adds callback called on new http/https requests with the given path. + virtual void AddHttpRequestHandler( + const std::string& path, + const RequestHandlerCallback& callback) = 0; + virtual void AddHttpsRequestHandler( + const std::string& path, + const RequestHandlerCallback& callback) = 0; virtual uint16_t GetHttpPort() const = 0; virtual uint16_t GetHttpsPort() const = 0;
diff --git a/libweave/include/weave/provider/test/mock_http_server.h b/libweave/include/weave/provider/test/mock_http_server.h index a2c9ac2..e807f2a 100644 --- a/libweave/include/weave/provider/test/mock_http_server.h +++ b/libweave/include/weave/provider/test/mock_http_server.h
@@ -18,9 +18,10 @@ class MockHttpServer : public HttpServer { public: - MOCK_METHOD2(AddRequestHandler, + MOCK_METHOD2(AddHttpRequestHandler, void(const std::string&, const RequestHandlerCallback&)); - + MOCK_METHOD2(AddHttpsRequestHandler, + void(const std::string&, const RequestHandlerCallback&)); MOCK_CONST_METHOD0(GetHttpPort, uint16_t()); MOCK_CONST_METHOD0(GetHttpsPort, uint16_t()); MOCK_CONST_METHOD0(GetHttpsCertificateFingerprint, std::vector<uint8_t>());
diff --git a/libweave/src/privet/privet_handler.cc b/libweave/src/privet/privet_handler.cc index a7bfee1..297af73 100644 --- a/libweave/src/privet/privet_handler.cc +++ b/libweave/src/privet/privet_handler.cc
@@ -343,6 +343,18 @@ } // namespace +std::vector<std::string> PrivetHandler::GetHttpPaths() const { + // TODO(vitalybuka): Should be subset only. + return GetHttpsPaths(); +} + +std::vector<std::string> PrivetHandler::GetHttpsPaths() const { + std::vector<std::string> result; + for (const auto& pair : handlers_) + result.push_back(pair.first); + return result; +} + PrivetHandler::PrivetHandler(CloudDelegate* cloud, DeviceDelegate* device, SecurityDelegate* security,
diff --git a/libweave/src/privet/privet_handler.h b/libweave/src/privet/privet_handler.h index ce32485..609b7a0 100644 --- a/libweave/src/privet/privet_handler.h +++ b/libweave/src/privet/privet_handler.h
@@ -48,6 +48,9 @@ void OnCommandDefsChanged() override; void OnStateChanged() override; + std::vector<std::string> GetHttpPaths() const; + std::vector<std::string> GetHttpsPaths() const; + // Handles HTTP/HTTPS Privet request. // |api| is the path from the HTTP request, e.g /privet/info. // |auth_header| is the Authentication header from HTTP request.
diff --git a/libweave/src/privet/privet_manager.cc b/libweave/src/privet/privet_manager.cc index e0f36c6..31af8ae 100644 --- a/libweave/src/privet/privet_manager.cc +++ b/libweave/src/privet/privet_manager.cc
@@ -87,9 +87,17 @@ security_.get(), wifi_bootstrap_manager_.get())); - http_server->AddRequestHandler("/privet/", - base::Bind(&Manager::PrivetRequestHandler, - weak_ptr_factory_.GetWeakPtr())); + for (const auto& path : privet_handler_->GetHttpPaths()) { + http_server->AddHttpRequestHandler( + path, base::Bind(&Manager::PrivetRequestHandler, + weak_ptr_factory_.GetWeakPtr())); + } + + for (const auto& path : privet_handler_->GetHttpsPaths()) { + http_server->AddHttpsRequestHandler( + path, base::Bind(&Manager::PrivetRequestHandler, + weak_ptr_factory_.GetWeakPtr())); + } } std::string Manager::GetCurrentlyConnectedSsid() const {
diff --git a/libweave/src/weave_unittest.cc b/libweave/src/weave_unittest.cc index 7be9a24..3b68e3e 100644 --- a/libweave/src/weave_unittest.cc +++ b/libweave/src/weave_unittest.cc
@@ -201,7 +201,13 @@ EXPECT_CALL(http_server_, GetHttpsPort()).WillRepeatedly(Return(12)); EXPECT_CALL(http_server_, GetHttpsCertificateFingerprint()) .WillRepeatedly(Return(std::vector<uint8_t>{1, 2, 3})); - EXPECT_CALL(http_server_, AddRequestHandler(_, _)) + EXPECT_CALL(http_server_, AddHttpRequestHandler(_, _)) + .WillRepeatedly(Invoke( + [this](const std::string& path_prefix, + const provider::HttpServer::RequestHandlerCallback& cb) { + http_server_request_cb_.push_back(cb); + })); + EXPECT_CALL(http_server_, AddHttpsRequestHandler(_, _)) .WillRepeatedly(Invoke( [this](const std::string& path_prefix, const provider::HttpServer::RequestHandlerCallback& cb) {