Reviewed-by: Nadav Har'El <n...@scylladb.com>
--
Nadav Har'El
n...@scylladb.com
On Thu, Oct 3, 2019 at 7:25 AM Waldemar Kozaczuk <jwkozac...@gmail.com>
wrote:
> Fixes #1053
>
> Signed-off-by: Waldemar Kozaczuk <jwkozac...@gmail.com>
> ---
> modules/httpserver-api/global_server.cc | 30 ++++++++-----
> modules/httpserver-api/global_server.hh | 9 ++--
> modules/httpserver-api/main.cc | 52 ++++++++++++-----------
> modules/httpserver-api/request_handler.cc | 7 +--
> modules/httpserver-api/request_handler.hh | 5 +--
> modules/httpserver-api/routes.hh | 1 -
> modules/httpserver-api/server.cc | 34 +++++++++++----
> modules/httpserver-api/server.hh | 3 +-
> 8 files changed, 83 insertions(+), 58 deletions(-)
>
> diff --git a/modules/httpserver-api/global_server.cc
> b/modules/httpserver-api/global_server.cc
> index 96965744..f56f4ed4 100644
> --- a/modules/httpserver-api/global_server.cc
> +++ b/modules/httpserver-api/global_server.cc
> @@ -17,6 +17,7 @@
> #include "json/api_docs.hh"
> #include <osv/debug.h>
> #include "transformers.hh"
> +#include <osv/options.hh>
>
> namespace httpserver {
>
> @@ -30,12 +31,15 @@ global_server& global_server::get()
> return *instance;
> }
>
> -bool global_server::run(po::variables_map& _config)
> +bool global_server::run(std::map<std::string,std::vector<std::string>>&
> _config)
> {
> if (get().s != nullptr) {
> return false;
> }
> - std::string config_file_path =
> _config["config-file"].as<std::string>();
> + std::string config_file_path = "/tmp/httpserver.conf";
> + if (options::option_value_exists(_config, "config-file")) {
> + config_file_path = options::extract_option_value(_config,
> "config-file");
> + }
> std::ifstream f(config_file_path);
> if (f.is_open()) {
> try {
> @@ -79,11 +83,15 @@ bool global_server::run(po::variables_map& _config)
> set(_config);
> get().set("ipaddress", "0.0.0.0");
> get().set("port", "8000");
> - get().set("cert", "/etc/pki/server.pem");
> - get().set("key", "/etc/pki/private/server.key");
> - get().set("cacert", "/etc/pki/CA/cacert.pem");
>
> - get().s = new http::server::server(&get().config, &get()._routes);
> + if (get().config.count("ssl")) {
> + get().set("cert", "/etc/pki/server.pem");
> + get().set("key", "/etc/pki/private/server.key");
> + get().set("cacert", "/etc/pki/CA/cacert.pem");
> + }
> +
> + auto port = get().config["port"][0];
> + get().s = new http::server::server(get().config, &get()._routes);
>
> osv::this_application::on_termination_request([&] {
> get().s->close();
> @@ -92,7 +100,7 @@ bool global_server::run(po::variables_map& _config)
> }
> });
>
> - std::cout << "Rest API server running on port " <<
> get().config["port"].as<std::string>() << std::endl;
> + std::cout << "Rest API server running on port " << port << std::endl;
> get().s->run();
> return true;
> }
> @@ -153,7 +161,7 @@ global_server::global_server()
> set_routes();
> }
>
> -global_server& global_server::set(po::variables_map& _config)
> +global_server&
> global_server::set(std::map<std::string,std::vector<std::string>> &_config)
> {
> for (auto i : _config) {
> get().config.insert(std::make_pair(i.first, i.second));
> @@ -164,9 +172,9 @@ global_server& global_server::set(po::variables_map&
> _config)
> global_server& global_server::set(const std::string& key,
> const std::string& _value)
> {
> - boost::any val(_value);
> - boost::program_options::variable_value v(val, false);
> - config.insert(std::make_pair(std::string(key), v));
> + std::vector<std::string> values;
> + values.push_back(_value);
> + config.insert(std::make_pair(std::string(key), values));
> return *this;
> }
>
> diff --git a/modules/httpserver-api/global_server.hh
> b/modules/httpserver-api/global_server.hh
> index 7a8af11a..dc3c2d4a 100644
> --- a/modules/httpserver-api/global_server.hh
> +++ b/modules/httpserver-api/global_server.hh
> @@ -10,13 +10,10 @@
> #include "routes.hh"
> #include "server.hh"
> #include <vector>
> -#include <boost/program_options/variables_map.hpp>
> #include <mutex>
> #include <condition_variable>
> #include <external/x64/misc.bin/usr/include/yaml-cpp/node/iterator.h>
>
> -namespace po = boost::program_options;
> -
> namespace httpserver {
> /**
> * Global server is a singleton class that controls
> @@ -46,7 +43,7 @@ public:
> *
> * @return false if the server already running
> */
> - static bool run(po::variables_map& config);
> + static bool run(std::map<std::string,std::vector<std::string>>&
> config);
>
> /**
> * set an entry in the configuration
> @@ -73,7 +70,7 @@ private:
> static global_server* instance;
> routes _routes;
> http::server::server* s;
> - po::variables_map config;
> + std::map<std::string,std::vector<std::string>> config;
> std::vector<void*> plugins;
>
> /**
> @@ -81,7 +78,7 @@ private:
> * @param config a variable map
> * @return a reference to the server
> */
> - static global_server& set(po::variables_map& config);
> + static global_server&
> set(std::map<std::string,std::vector<std::string>>& config);
>
> };
>
> diff --git a/modules/httpserver-api/main.cc
> b/modules/httpserver-api/main.cc
> index 7eb9c058..2eca5076 100644
> --- a/modules/httpserver-api/main.cc
> +++ b/modules/httpserver-api/main.cc
> @@ -10,43 +10,47 @@
>
> #include "server.hh"
>
> -#include <boost/program_options/options_description.hpp>
> -#include <boost/program_options/variables_map.hpp>
> -#include <boost/program_options/parsers.hpp>
> #include <boost/asio.hpp>
> #include "global_server.hh"
> #include <osv/exception_utils.hh>
> +#include <osv/options.hh>
>
> using namespace httpserver;
>
> -namespace po = boost::program_options;
> +static void usage()
> +{
> + std::cout << "Allowed options:\n";
> + std::cout << " --help produce help
> message\n";
> + std::cout << " --config-file arg (=/tmp/httpserver.conf)\n";
> + std::cout << " configuration
> file path\n";
> + std::cout << " --access-allow arg Set the
> Access-Control-Allow-Origin to\n";
> + std::cout << " *. Note the
> security risk\n";
> + std::cout << " --ipaddress arg set the ip
> address\n";
> + std::cout << " --port arg set the port\n";
> + std::cout << " --cert arg path to
> server's SSL certificate\n";
> + std::cout << " --key arg path to
> server's private key\n";
> + std::cout << " --cacert arg path to CA
> certificate\n";
> + std::cout << " --ssl enable SSL\n\n";
> +}
> +
> +static void handle_parse_error(const std::string &message)
> +{
> + std::cout << message << std::endl;
> + usage();
> + exit(1);
> +}
>
> int main(int argc, char* argv[])
> {
> - po::options_description desc("Allowed options");
> - desc.add_options()
> - ("help", "produce help message")
> - ("config-file",
> po::value<std::string>()->default_value("/tmp/httpserver.conf"),
> "configuration file path")
> - ("access-allow", po::value<std::string>(),
> - "Set the Access-Control-Allow-Origin to *. Note the security
> risk")
> - ("ipaddress", po::value<std::string>(), "set the ip address")
> - ("port", po::value<std::string>(), "set the port")
> - ("cert", po::value<std::string>(), "path to server's SSL
> certificate")
> - ("key", po::value<std::string>(), "path to server's private key")
> - ("cacert", po::value<std::string>(), "path to CA certificate")
> - ("ssl", "enable SSL");
> -
> - po::variables_map config;
> - po::store(po::parse_command_line(argc, argv, desc), config);
> - po::notify(config);
> -
> - if (config.count("help")) {
> - std::cerr << desc << "\n";
> + auto options_values = options::parse_options_values(argc - 1, argv +
> 1, handle_parse_error);
> +
> + if (options::extract_option_flag(options_values, "help",
> handle_parse_error)) {
> + usage();
> return 1;
> }
>
> try {
> - global_server::run(config);
> + global_server::run(options_values);
> } catch (...) {
> std::cerr << "httpserver failed: " << current_what() << std::endl;
> return 1;
> diff --git a/modules/httpserver-api/request_handler.cc
> b/modules/httpserver-api/request_handler.cc
> index e03222ff..8272da80 100644
> --- a/modules/httpserver-api/request_handler.cc
> +++ b/modules/httpserver-api/request_handler.cc
> @@ -20,6 +20,7 @@
> #include <fstream>
> #include <sstream>
> #include <string>
> +#include <osv/options.hh>
>
> namespace http {
>
> @@ -62,13 +63,13 @@ size_t request_handler::update_parameters(request& req)
> return par;
> }
>
> -request_handler::request_handler(httpserver::routes* routes, const
> boost::program_options::variables_map& _config)
> +request_handler::request_handler(httpserver::routes* routes,
> std::map<std::string,std::vector<std::string>>& _config)
> : routes(routes),
> config(_config)
>
> {
> - if (config.count("access-allow")) {
> - const auto s = config["access-allow"].as<std::string>();
> + if (options::option_value_exists(_config, "access-allow")) {
> + const auto s = options::extract_option_value(_config,
> "access-allow");
>
> std::string::size_type b = 0;
> do {
> diff --git a/modules/httpserver-api/request_handler.hh
> b/modules/httpserver-api/request_handler.hh
> index 180782a3..7e055c16 100644
> --- a/modules/httpserver-api/request_handler.hh
> +++ b/modules/httpserver-api/request_handler.hh
> @@ -15,7 +15,6 @@
>
> #include "routes.hh"
>
> -#include <boost/program_options/variables_map.hpp>
> #include <string>
>
> namespace http {
> @@ -40,7 +39,7 @@ public:
> *
> * @param routes the routes object
> */
> - explicit request_handler(httpserver::routes* routes, const
> boost::program_options::variables_map& _config);
> + explicit request_handler(httpserver::routes* routes,
> std::map<std::string,std::vector<std::string>>& _config);
>
> request_handler(const request_handler&) = delete;
>
> @@ -74,7 +73,7 @@ public:
> private:
> httpserver::routes* routes;
> std::vector<std::string> allowed_domains;
> - const boost::program_options::variables_map& config;
> + const std::map<std::string,std::vector<std::string>>& config;
>
> };
>
> diff --git a/modules/httpserver-api/routes.hh
> b/modules/httpserver-api/routes.hh
> index 236940cd..7cff5dbb 100644
> --- a/modules/httpserver-api/routes.hh
> +++ b/modules/httpserver-api/routes.hh
> @@ -14,7 +14,6 @@
> #include "common.hh"
> #include "reply.hh"
>
> -#include <boost/program_options/variables_map.hpp>
> #include <unordered_map>
> #include <vector>
>
> diff --git a/modules/httpserver-api/server.cc
> b/modules/httpserver-api/server.cc
> index 4e8f18a9..709972b2 100644
> --- a/modules/httpserver-api/server.cc
> +++ b/modules/httpserver-api/server.cc
> @@ -17,6 +17,7 @@
>
> #include <utility>
> #include <openssl/ssl.h>
> +#include <osv/options.hh>
>
> namespace http {
>
> @@ -28,17 +29,17 @@ static bool exists(const std::string& path)
> return stat(path.c_str(), &s) == 0;
> }
>
> -server::server(const boost::program_options::variables_map* config,
> +server::server(std::map<std::string,std::vector<std::string>> &config,
> httpserver::routes* routes)
> : io_service_()
> , connection_manager_()
> - , request_handler_(routes, *config)
> + , request_handler_(routes, config)
> {
> // Open the acceptor with the option to reuse the address (i.e.
> SO_REUSEADDR).
> boost::asio::ip::tcp::resolver resolver(io_service_);
> boost::asio::ip::tcp::endpoint endpoint = *resolver.resolve({
> - (*config)["ipaddress"].as<std::string>(),
> - (*config)["port"].as<std::string>()
> + options::extract_option_value(config, "ipaddress"),
> + options::extract_option_value(config, "port")
> });
>
> tcp::acceptor tcp_acceptor(io_service_);
> @@ -47,12 +48,22 @@ server::server(const
> boost::program_options::variables_map* config,
> tcp_acceptor.bind(endpoint);
> tcp_acceptor.listen();
>
> - if (config->count("ssl")) {
> + if (options::extract_option_flag(config, "ssl", [](const std::string
> &message) {
> + std::cerr << message << std::endl;
> + throw std::runtime_error("invalid configuration");
> + })) {
> ensure_openssl_initialized();
>
> - auto ca_cert_path = (*config)["cacert"].as<std::string>();
> - auto cert_path = (*config)["cert"].as<std::string>();
> - auto key_path = (*config)["key"].as<std::string>();
> + auto ca_cert_path =
> options::extract_option_value(config,"cacert");
> + auto cert_path = options::extract_option_value(config,"cert");
> + auto key_path = options::extract_option_value(config,"key");
> +
> + if (!config.empty()) {
> + for (auto option : config) {
> + std::cout << "Unrecognized option: " << option.first <<
> std::endl;
> + }
> + throw std::runtime_error("invalid configuration");
> + }
>
> bool valid = true;
> for (auto& path : {ca_cert_path, cert_path, key_path}) {
> @@ -70,6 +81,13 @@ server::server(const
> boost::program_options::variables_map* config,
> ssl::context ctx = make_ssl_context(ca_cert_path, cert_path,
> key_path);
> acceptor_.reset(new ssl_acceptor(io_service_, std::move(ctx),
> std::move(tcp_acceptor)));
> } else {
> + if (!config.empty()) {
> + for (auto option : config) {
> + std::cout << "Unrecognized option: " << option.first <<
> std::endl;
> + }
> + throw std::runtime_error("invalid configuration");
> + }
> +
> acceptor_.reset(new plain_acceptor(io_service_,
> std::move(tcp_acceptor)));
> }
>
> diff --git a/modules/httpserver-api/server.hh
> b/modules/httpserver-api/server.hh
> index 94f414a0..2da4f574 100644
> --- a/modules/httpserver-api/server.hh
> +++ b/modules/httpserver-api/server.hh
> @@ -17,7 +17,6 @@
> #include "connection.hh"
> #include "transport.hh"
>
> -#include <boost/program_options/variables_map.hpp>
> #include <boost/asio.hpp>
> #include <string>
> #include <memory>
> @@ -43,7 +42,7 @@ public:
> * @param config a configuration object
> * @param routes the routes object
> */
> - explicit server(const boost::program_options::variables_map* config,
> + explicit server(std::map<std::string,std::vector<std::string>>
> &config,
> httpserver::routes* routes);
>
> server(const server&) = delete;
> --
> 2.20.1
>
> --
> You received this message because you are subscribed to the Google Groups
> "OSv Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to osv-dev+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/osv-dev/20191003042437.27978-5-jwkozaczuk%40gmail.com
> .
>
--
You received this message because you are subscribed to the Google Groups "OSv
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to osv-dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/osv-dev/CANEVyjtgLqg10AJm_mOWPsGNmed6WqxHyG4HFtfgKPJXv4%2B8Xw%40mail.gmail.com.
