diff options
31 files changed, 591 insertions, 316 deletions
diff --git a/brep/buildfile b/brep/buildfile index 360f82d..4caf93a 100644 --- a/brep/buildfile +++ b/brep/buildfile @@ -42,17 +42,18 @@ import libs += libstudxml%lib{studxml} gen = {hxx ixx cxx}{ options } src = \ + {hxx cxx}{ database } \ + {hxx cxx}{ database-module } \ {hxx cxx}{ diagnostics } \ - {hxx cxx}{ module } \ - {hxx }{ options-types } \ {hxx cxx}{ mod-package-details } \ {hxx cxx}{ mod-package-search } \ {hxx cxx}{ mod-package-version-details } \ - {hxx cxx}{ page } \ {hxx cxx}{ mod-repository-details } \ {hxx cxx}{ mod-repository-root } \ + {hxx cxx}{ module } \ + {hxx }{ options-types } \ + {hxx cxx}{ page } \ { cxx}{ services } \ - {hxx cxx}{ database } \ {hxx cxx}{ types-parsers } \ {hxx }{ wrapper-traits } \ ../web/{hxx cxx}{ mime-url-encoding } \ diff --git a/brep/database-module b/brep/database-module new file mode 100644 index 0000000..64d5aaf --- /dev/null +++ b/brep/database-module @@ -0,0 +1,55 @@ +// file : brep/database-module -*- C++ -*- +// copyright : Copyright (c) 2014-2016 Code Synthesis Ltd +// license : MIT; see accompanying LICENSE file + +#ifndef BREP_DATABASE_MODULE +#define BREP_DATABASE_MODULE + +#include <odb/forward.hxx> // database + +#include <brep/types> +#include <brep/utility> + +#include <brep/module> +#include <brep/options> + +namespace brep +{ + // A module that utilises the database. Specifically, it will retry the + // request in the face of recoverable database failures (deadlock, loss of + // connection, etc) up to a certain number of times. + // + class database_module: public module + { + protected: + database_module () = default; + + // Create a shallow copy (handling instance) if initialized and a deep + // copy (context exemplar) otherwise. + // + explicit + database_module (const database_module&); + + // Required to avoid getting warning from clang that + // database_module::init() hides module::init() virtual functions. This + // way all functions get to the same scope and become overloaded set. + // + using module::init; + + void + init (const options::db&); + + virtual bool + handle (request&, response&) = 0; + + protected: + size_t retry_; + shared_ptr<odb::core::database> db_; + + private: + virtual bool + handle (request&, response&, log&); + }; +} + +#endif // BREP_DATABASE_MODULE diff --git a/brep/database-module.cxx b/brep/database-module.cxx new file mode 100644 index 0000000..630fd89 --- /dev/null +++ b/brep/database-module.cxx @@ -0,0 +1,50 @@ +// file : brep/database-module.cxx -*- C++ -*- +// copyright : Copyright (c) 2014-2016 Code Synthesis Ltd +// license : MIT; see accompanying LICENSE file + +#include <brep/database-module> + +#include <odb/exceptions.hxx> + +#include <brep/options> +#include <brep/database> + +namespace brep +{ + // While currently the user-defined copy constructor is not required (we + // don't need to deep copy nullptr's), it is a good idea to keep the + // placeholder ready for less trivial cases. + // + database_module:: + database_module (const database_module& r) + : module (r), + retry_ (r.retry_), + db_ (r.initialized_ ? r.db_ : nullptr) + { + } + + void database_module:: + init (const options::db& o) + { + retry_ = o.db_retry (); + db_ = shared_database (o); + } + + bool database_module:: + handle (request& rq, response& rs, log& l) + try + { + return module::handle (rq, rs, l); + } + catch (const odb::recoverable& e) + { + if (retry_-- > 0) + { + MODULE_DIAG; + l1 ([&]{trace << e.what () << "; " << retry_ + 1 << " retries left";}); + throw retry (); + } + + throw; + } +} diff --git a/brep/database.cxx b/brep/database.cxx index 17605c5..1f70881 100644 --- a/brep/database.cxx +++ b/brep/database.cxx @@ -41,11 +41,13 @@ namespace brep } shared_ptr<database> d ( - make_shared<database> (o.db_user (), - o.db_password (), - o.db_name (), - o.db_host (), - o.db_port ())); + make_shared<database> ( + o.db_user (), + o.db_password (), + o.db_name (), + o.db_host (), + o.db_port (), + "options='-c default_transaction_isolation=serializable'")); databases[o] = d; return d; diff --git a/brep/mod-package-details b/brep/mod-package-details index 0a86fc2..4dc6b18 100644 --- a/brep/mod-package-details +++ b/brep/mod-package-details @@ -5,17 +5,15 @@ #ifndef BREP_MOD_PACKAGE_DETAILS #define BREP_MOD_PACKAGE_DETAILS -#include <odb/forward.hxx> // database - #include <brep/types> #include <brep/utility> -#include <brep/module> #include <brep/options> +#include <brep/database-module> namespace brep { - class package_details: public module + class package_details: public database_module { public: package_details () = default; @@ -38,7 +36,6 @@ namespace brep private: shared_ptr<options::package_details> options_; - shared_ptr<odb::core::database> db_; }; } diff --git a/brep/mod-package-details.cxx b/brep/mod-package-details.cxx index a5c2e3e..10bd72c 100644 --- a/brep/mod-package-details.cxx +++ b/brep/mod-package-details.cxx @@ -18,7 +18,6 @@ #include <brep/page> #include <brep/options> #include <brep/package> -#include <brep/database> #include <brep/package-odb> using namespace odb::core; @@ -30,9 +29,8 @@ using namespace brep::cli; // brep::package_details:: package_details (const package_details& r) - : module (r), - options_ (r.initialized_ ? r.options_ : nullptr), - db_ (r.initialized_ ? r.db_ : nullptr) + : database_module (r), + options_ (r.initialized_ ? r.options_ : nullptr) { } @@ -44,10 +42,10 @@ init (scanner& s) options_ = make_shared<options::package_details> ( s, unknown_mode::fail, unknown_mode::fail); + database_module::init (*options_); + if (options_->root ().empty ()) options_->root (dir_path ("/")); - - db_ = shared_database (*options_); } template <typename T> diff --git a/brep/mod-package-search b/brep/mod-package-search index 1730be4..bb762cd 100644 --- a/brep/mod-package-search +++ b/brep/mod-package-search @@ -5,17 +5,15 @@ #ifndef BREP_MOD_PACKAGE_SEARCH #define BREP_MOD_PACKAGE_SEARCH -#include <odb/forward.hxx> // database - #include <brep/types> #include <brep/utility> -#include <brep/module> #include <brep/options> +#include <brep/database-module> namespace brep { - class package_search: public module + class package_search: public database_module { public: package_search () = default; @@ -38,7 +36,6 @@ namespace brep private: shared_ptr<options::package_search> options_; - shared_ptr<odb::core::database> db_; }; } diff --git a/brep/mod-package-search.cxx b/brep/mod-package-search.cxx index 77a06b6..9769783 100644 --- a/brep/mod-package-search.cxx +++ b/brep/mod-package-search.cxx @@ -21,7 +21,6 @@ #include <brep/page> #include <brep/options> #include <brep/package> -#include <brep/database> #include <brep/package-odb> using namespace odb::core; @@ -33,9 +32,8 @@ using namespace brep::cli; // brep::package_search:: package_search (const package_search& r) - : module (r), - options_ (r.initialized_ ? r.options_ : nullptr), - db_ (r.initialized_ ? r.db_ : nullptr) + : database_module (r), + options_ (r.initialized_ ? r.options_ : nullptr) { } @@ -47,11 +45,11 @@ init (scanner& s) options_ = make_shared<options::package_search> ( s, unknown_mode::fail, unknown_mode::fail); + database_module::init (*options_); + if (options_->root ().empty ()) options_->root (dir_path ("/")); - db_ = shared_database (*options_); - // Check that the database schema matches the current one. It's enough to // perform the check in just a single module implementation (and we don't // do in the dispatcher because it doesn't use the database). @@ -94,7 +92,8 @@ handle (request& rq, response& rs) try { name_value_scanner s (rq.parameters ()); - params = params::package_search (s, unknown_mode::fail, unknown_mode::fail); + params = params::package_search ( + s, unknown_mode::fail, unknown_mode::fail); } catch (const unknown_argument& e) { @@ -107,6 +106,7 @@ handle (request& rq, response& rs) ? "" : "?q=" + web::mime_url_encode (squery)); + static const string title ("Packages"); xml::serializer s (rs.content (), title); diff --git a/brep/mod-package-version-details b/brep/mod-package-version-details index cfbcf94..a463511 100644 --- a/brep/mod-package-version-details +++ b/brep/mod-package-version-details @@ -5,17 +5,15 @@ #ifndef BREP_MOD_PACKAGE_VERSION_DETAILS #define BREP_MOD_PACKAGE_VERSION_DETAILS -#include <odb/forward.hxx> // database - #include <brep/types> #include <brep/utility> -#include <brep/module> #include <brep/options> +#include <brep/database-module> namespace brep { - class package_version_details: public module + class package_version_details: public database_module { public: package_version_details () = default; @@ -41,7 +39,6 @@ namespace brep private: shared_ptr<options::package_version_details> options_; - shared_ptr<odb::core::database> db_; }; } diff --git a/brep/mod-package-version-details.cxx b/brep/mod-package-version-details.cxx index 992b829..21da41f 100644 --- a/brep/mod-package-version-details.cxx +++ b/brep/mod-package-version-details.cxx @@ -18,7 +18,6 @@ #include <brep/page> #include <brep/options> #include <brep/package> -#include <brep/database> #include <brep/package-odb> using namespace std; @@ -31,9 +30,8 @@ using namespace brep::cli; // brep::package_version_details:: package_version_details (const package_version_details& r) - : module (r), - options_ (r.initialized_ ? r.options_ : nullptr), - db_ (r.initialized_ ? r.db_ : nullptr) + : database_module (r), + options_ (r.initialized_ ? r.options_ : nullptr) { } @@ -45,10 +43,10 @@ init (scanner& s) options_ = make_shared<options::package_version_details> ( s, unknown_mode::fail, unknown_mode::fail); + database_module::init (*options_); + if (options_->root ().empty ()) options_->root (dir_path ("/")); - - db_ = shared_database (*options_); } bool brep::package_version_details:: diff --git a/brep/mod-repository-details b/brep/mod-repository-details index 411d9e6..49ad629 100644 --- a/brep/mod-repository-details +++ b/brep/mod-repository-details @@ -5,17 +5,15 @@ #ifndef BREP_MOD_REPOSITORY_DETAILS #define BREP_MOD_REPOSITORY_DETAILS -#include <odb/forward.hxx> // database - #include <brep/types> #include <brep/utility> -#include <brep/module> #include <brep/options> +#include <brep/database-module> namespace brep { - class repository_details: public module + class repository_details: public database_module { public: repository_details () = default; @@ -38,7 +36,6 @@ namespace brep private: shared_ptr<options::repository_details> options_; - shared_ptr<odb::core::database> db_; }; } diff --git a/brep/mod-repository-details.cxx b/brep/mod-repository-details.cxx index f040be6..b0e2b95 100644 --- a/brep/mod-repository-details.cxx +++ b/brep/mod-repository-details.cxx @@ -24,7 +24,6 @@ #include <brep/page> #include <brep/options> #include <brep/package> -#include <brep/database> #include <brep/package-odb> using namespace std; @@ -37,9 +36,8 @@ using namespace brep::cli; // brep::repository_details:: repository_details (const repository_details& r) - : module (r), - options_ (r.initialized_ ? r.options_ : nullptr), - db_ (r.initialized_ ? r.db_ : nullptr) + : database_module (r), + options_ (r.initialized_ ? r.options_ : nullptr) { } @@ -51,11 +49,11 @@ init (scanner& s) options_ = make_shared<options::repository_details> ( s, unknown_mode::fail, unknown_mode::fail); + database_module::init (*options_); + if (options_->root ().empty ()) options_->root (dir_path ("/")); - db_ = shared_database (*options_); - tzset (); // To use butl::to_stream() later on. } diff --git a/brep/mod-repository-root.cxx b/brep/mod-repository-root.cxx index 7c27a60..7d0adc9 100644 --- a/brep/mod-repository-root.cxx +++ b/brep/mod-repository-root.cxx @@ -40,7 +40,7 @@ namespace brep cookies () {return request_.cookies ();} virtual istream& - content () {return request_.content ();} + content (bool buffer) {return request_.content (buffer);} private: request& request_; diff --git a/brep/module b/brep/module index 52106cd..3796399 100644 --- a/brep/module +++ b/brep/module @@ -62,7 +62,7 @@ namespace brep const basic_mark info (severity::info, \ this->log_writer_, \ __PRETTY_FUNCTION__); \ - const basic_mark trace (severity::info, \ + const basic_mark trace (severity::trace, \ this->log_writer_, \ __PRETTY_FUNCTION__) @@ -76,7 +76,8 @@ namespace brep // Trace verbosity level. // // 0 - tracing disabled. - // 1 - @@ TODO: document + // 1 - brief information regarding irregular situations, which not being + // an error can be of some interest. // 2 - @@ TODO: document // // While uint8 is more than enough, use uint16 for the ease of printing. diff --git a/brep/module.cxx b/brep/module.cxx index 1257d82..40acad9 100644 --- a/brep/module.cxx +++ b/brep/module.cxx @@ -201,7 +201,9 @@ namespace brep // Read brep::module configuration. // - static option_descriptions od (convert (options::module::description ())); + static option_descriptions od ( + convert (options::module::description ())); + name_values mo (filter (opts, od)); name_value_scanner s (mo); options::module o (s, cli::unknown_mode::fail, cli::unknown_mode::fail); @@ -311,7 +313,12 @@ namespace brep // Considered using lambda for mapping but looks too verbose while can // be a bit safer in runtime. // - static int s[] = {APLOG_ERR, APLOG_WARNING, APLOG_INFO, APLOG_TRACE1}; + // Use APLOG_INFO (as opposed to APLOG_TRACE1) as a mapping for + // severity::trace. "LogLevel trace1" configuration directive switches + // on the avalanche of log messages from various modules. Would be good + // to avoid wading through them. + // + static int s[] = {APLOG_ERR, APLOG_WARNING, APLOG_INFO, APLOG_INFO}; for (const auto& e: d) { diff --git a/brep/options.cli b/brep/options.cli index f3d26d0..bcb335e 100644 --- a/brep/options.cli +++ b/brep/options.cli @@ -54,15 +54,16 @@ namespace brep string db-name = "brep" { "<name>", - "Database name. If not specified, then '\cb{brep}' is used by default." + "Database name. If not specified, then '\cb{brep}' is used by + default." } string db-host { "<host>", - "Database host name, address, or socket. If not specified, then connect - to \cb{localhost} using the operating system-default mechanism - (Unix-domain socket, etc)." + "Database host name, address, or socket. If not specified, then + connect to \cb{localhost} using the operating system-default + mechanism (Unix-domain socket, etc)." } uint16_t db-port = 0 @@ -70,6 +71,14 @@ namespace brep "<port>", "Database port number. If not specified, the default port is used." } + + size_t db-retry = 10 + { + "<num>", + "The maximum number of times to retry database transactions in the + face of recoverable failures (deadlock, loss of connection, etc). The + default is 10." + } }; class page @@ -84,10 +93,10 @@ namespace brep vector<page_menu> menu; { "<label=link>", - "Web page menu. Each entry is displayed in the page header in the order - specified and aligned to the right edge. A link target that starts - with '\cb{/}' or contains '\cb{:}' is used as is. Otherwise, it is - prefixed with the repository web interface root." + "Web page menu. Each entry is displayed in the page header in the + order specified and aligned to the right edge. A link target that + starts with '\cb{/}' or contains '\cb{:}' is used as is. Otherwise, + it is prefixed with the repository web interface root." } }; diff --git a/etc/brep-module.conf b/etc/brep-module.conf index 8525135..05fd278 100644 --- a/etc/brep-module.conf +++ b/etc/brep-module.conf @@ -1,7 +1,7 @@ # Configuration file for the brep module (note: this is not an apache2 .conf -# file but it can be converted to one by prefixing all the options with brep-). -# See brep(1) for detailed description of each configuration option. Commented -# out options indicate their default values. +# file but it can be converted to one by prefixing all the options with +# brep-). See brep(1) for detailed description of each configuration option. +# Commented out options indicate their default values. # # Web page logo. It is displayed in the page header aligned to the left edge. @@ -10,9 +10,9 @@ # logo "" # Web page menu. Each entry is displayed in the page header in the order -# specified and aligned to the right edge. A link target that starts with '/' or -# contains ':' is used as is. Otherwise, it is prefixed with the repository web -# interface root. +# specified and aligned to the right edge. A link target that starts with '/' +# or contains ':' is used as is. Otherwise, it is prefixed with the repository +# web interface root. # menu Packages= menu About=?about @@ -50,6 +50,12 @@ menu About=?about # db-port +# The maximum number of times to retry database transactions in the +# face of recoverable failures (deadlock, loss of connection, etc). +# +# db-retry 10 + + # Trace verbosity. Disabled by default. # -# verbosity 1 +# verbosity 0 diff --git a/load/load.cli b/load/load.cli index 5c30c14..c3aeec4 100644 --- a/load/load.cli +++ b/load/load.cli @@ -96,8 +96,10 @@ class options \cb{0} Successful termination. -\cb{1} \cb{brep-load} or \l{brep-migrate(1)} instance is running. Try +\cb{1} Fatal error. + +\cb{2} \cb{brep-load} or \l{brep-migrate(1)} instance is running. Try again. -\cb{2} Fatal error. +\cb{3} The database recoverable error. Try again. " diff --git a/load/load.cxx b/load/load.cxx index 7786e0e..1a485dd 100644 --- a/load/load.cxx +++ b/load/load.cxx @@ -8,6 +8,7 @@ #include <odb/session.hxx> #include <odb/database.hxx> +#include <odb/exceptions.hxx> #include <odb/transaction.hxx> #include <odb/schema-catalog.hxx> @@ -602,7 +603,8 @@ resolve_dependencies (package& p, database& db) { auto c (*d.constraint); - if (c.min_version && c.max_version && *c.min_version == *c.max_version) + if (c.min_version && c.max_version && + *c.min_version == *c.max_version) { const version& v (*c.min_version); q = q && compare_version_eq (vm, v, v.revision != 0); @@ -755,28 +757,30 @@ try // If the pager failed, assume it has issued some diagnostics. // - return p.wait () ? 0 : 2; + return p.wait () ? 0 : 1; } if (argc < 2) { cerr << "error: configuration file path argument expected" << endl << help_info << endl; - return 2; + return 1; } if (argc > 2) { cerr << "error: unexpected argument encountered" << endl << help_info << endl; - return 2; + return 1; } - odb::pgsql::database db (ops.db_user (), - ops.db_password (), - ops.db_name (), - ops.db_host (), - ops.db_port ()); + odb::pgsql::database db ( + ops.db_user (), + ops.db_password (), + ops.db_name (), + ops.db_host (), + ops.db_port (), + "options='-c default_transaction_isolation=serializable'"); // Prevent several brep-load/migrate instances from updating DB // simultaneously. @@ -791,7 +795,7 @@ try { cerr << "error: database schema differs from the current one" << endl << " info: use brep-migrate to migrate the database" << endl; - return 2; + return 1; } // Load the description of all the internal repositories from the @@ -856,21 +860,26 @@ try catch (const database_locked&) { cerr << "brep-load or brep-migrate instance is running" << endl; - return 1; + return 2; +} +catch (const recoverable& e) +{ + cerr << "database recoverable error: " << e.what () << endl; + return 3; } catch (const cli::exception& e) { cerr << "error: " << e << endl << help_info << endl; - return 2; + return 1; } catch (const failed&) { - return 2; // Diagnostics has already been issued. + return 1; // Diagnostics has already been issued. } // Fully qualified to avoid ambiguity with odb exception. // catch (const std::exception& e) { cerr << "error: " << e.what () << endl; - return 2; + return 1; } diff --git a/migrate/migrate.cli b/migrate/migrate.cli index dcbf667..1d62504 100644 --- a/migrate/migrate.cli +++ b/migrate/migrate.cli @@ -114,8 +114,10 @@ class options \cb{0} Successful termination. -\cb{1} \cb{brep-migrate} or \l{brep-load(1)} instance is running. Try +\cb{1} Fatal error. + +\cb{2} \cb{brep-migrate} or \l{brep-load(1)} instance is running. Try again. -\cb{2} Fatal error. +\cb{3} The database recoverable error. Try again. " diff --git a/migrate/migrate.cxx b/migrate/migrate.cxx index 70ba54f..aa11ecc 100644 --- a/migrate/migrate.cxx +++ b/migrate/migrate.cxx @@ -170,8 +170,8 @@ drop (database& db) const { for (const auto& s: drop_statements_) // If the statement execution fails, the corresponding source file line - // number is not reported. The line number could be usefull for the utility - // implementer only. The errors seen by the end-user should not be + // number is not reported. The line number could be usefull for the + // utility implementer only. The errors seen by the end-user should not be // statement-specific. // db.execute (s); @@ -226,28 +226,30 @@ try // If the pager failed, assume it has issued some diagnostics. // - return p.wait () ? 0 : 2; + return p.wait () ? 0 : 1; } if (argc > 1) { cerr << "error: unexpected argument encountered" << endl << help_info << endl; - return 2; + return 1; } if (ops.recreate () && ops.drop ()) { cerr << "error: inconsistent options specified" << endl << help_info << endl; - return 2; + return 1; } - odb::pgsql::database db (ops.db_user (), - ops.db_password (), - ops.db_name (), - ops.db_host (), - ops.db_port ()); + odb::pgsql::database db ( + ops.db_user (), + ops.db_password (), + ops.db_name (), + ops.db_host (), + ops.db_port (), + "options='-c default_transaction_isolation=serializable'"); // Prevent several brep-migrate/load instances from updating DB // simultaneously. @@ -329,21 +331,26 @@ try catch (const database_locked&) { cerr << "brep-migrate or brep-load instance is running" << endl; - return 1; + return 2; +} +catch (const recoverable& e) +{ + cerr << "database recoverable error: " << e.what () << endl; + return 3; } catch (const cli::exception& e) { cerr << "error: " << e << endl << help_info << endl; - return 2; + return 1; } catch (const failed&) { - return 2; // Diagnostics has already been issued. + return 1; // Diagnostics has already been issued. } // Fully qualified to avoid ambiguity with odb exception. // catch (const std::exception& e) { cerr << "error: " << e.what () << endl; - return 2; + return 1; } diff --git a/tests/load/driver.cxx b/tests/load/driver.cxx index bb46033..c2907b6 100644 --- a/tests/load/driver.cxx +++ b/tests/load/driver.cxx @@ -84,7 +84,13 @@ main (int argc, char* argv[]) // Check persistent objects validity. // - odb::pgsql::database db ("", "", "brep", argv[3], stoul (argv[5])); + odb::pgsql::database db ( + "", + "", + "brep", + argv[3], + stoul (argv[5]), + "options='-c default_transaction_isolation=serializable'"); { session s; diff --git a/web/apache/log b/web/apache/log index 7318e32..291f8de 100644 --- a/web/apache/log +++ b/web/apache/log @@ -5,7 +5,7 @@ #ifndef WEB_APACHE_LOG #define WEB_APACHE_LOG -#include <httpd.h> // request_rec +#include <httpd.h> // request_rec, server_rec #include <http_log.h> #include <http_config.h> // module @@ -44,7 +44,7 @@ namespace web const char* msg) const noexcept { if (file && *file) - file = nullptr; // skip file/line placeholder from log line. + file = nullptr; // Skip file/line placeholder from log line. level = std::min (level, APLOG_TRACE8); @@ -59,7 +59,7 @@ namespace web func, msg); else - // skip function name placeholder from log line + // Skip function name placeholder from log line. // ap_log_error (file, line, diff --git a/web/apache/request b/web/apache/request index ab69dff..9540561 100644 --- a/web/apache/request +++ b/web/apache/request @@ -5,23 +5,14 @@ #ifndef WEB_APACHE_REQUEST #define WEB_APACHE_REQUEST -#include <apr_strings.h> +#include <httpd.h> // request_rec, HTTP_*, OK, M_POST -#include <httpd.h> -#include <http_core.h> -#include <util_script.h> - -#include <ios> #include <chrono> #include <memory> // unique_ptr #include <string> -#include <cassert> #include <istream> #include <ostream> -#include <utility> // move() #include <streambuf> -#include <stdexcept> -#include <exception> #include <web/module> #include <web/apache/stream> @@ -30,23 +21,63 @@ namespace web { namespace apache { + // The state of the request processing, reflecting an interaction with + // Apache API (like reading/writing content function calls), with no + // buffering taken into account. Any state different from the initial + // suppose that some irrevocable interaction with Apache API have + // happened, so request processing should be either completed, or + // reported as failed. State values are ordered in a sense that the + // higher value reflects the more advanced stage of processing, so the + // request current state value may not decrease. + // + enum class request_state + { + // Denotes the initial stage of the request handling. At this stage + // the request line and headers are already parsed by Apache. + // + initial, + + // Reading the request content. + // + reading, + + // Adding the response headers (cookies in particular). + // + headers, + + // Writing the response content. + // + writing + }; + class request: public web::request, public web::response, - public write_state + public stream_state { friend class service; - request (request_rec* rec) noexcept: rec_ (rec) {rec_->status = HTTP_OK;} + request (request_rec* rec) noexcept + : rec_ (rec) + { + rec_->status = HTTP_OK; + } + + request_state + state () const noexcept {return state_;} - // Flush of buffered content. + // Flush the buffered response content if present. The returned value + // should be passed to Apache API on request handler exit. // int flush (); - // Return true if content have been sent to the client, false otherwise. + // Prepare for the request re-processing if possible (no unbuffered + // read/write operations have been done). Throw sequence_error + // otherwise. In particular, the preparation can include the response + // content buffer cleanup, the request content buffer rewind. // - bool - get_write_state () const noexcept {return write_state_;} + void + rewind (); // Get request path. // @@ -56,7 +87,7 @@ namespace web // Get request body data stream. // virtual std::istream& - content (); + content (bool buffer = false); // Get request parameters. // @@ -70,7 +101,8 @@ namespace web // Get response status code. // - status_code status () const noexcept {return rec_->status;} + status_code + status () const noexcept {return rec_->status;} // Set response status code. // @@ -89,10 +121,11 @@ namespace web virtual void cookie (const char* name, const char* value, - const std::chrono::seconds* max_age = 0, - const char* path = 0, - const char* domain = 0, - bool secure = false); + const std::chrono::seconds* max_age = nullptr, + const char* path = nullptr, + const char* domain = nullptr, + bool secure = false, + bool buffer = true); private: // Get application/x-www-form-urlencoded form data. @@ -103,37 +136,35 @@ namespace web void parse_parameters (const char* args); + // Advance the request processing state. Noop if new state is equal to + // the current one. Throw sequence_error if the new state is less then + // the current one. Can throw invalid_request if HTTP request is + // malformed. + // + void + state (request_state); + + // stream_state members implementation. + // virtual void - set_write_state () - { - if (!write_state_) - { - // Preparing to write a response read and discard request - // body if any. - // - int r (ap_discard_request_body (rec_)); - - if (r != OK) - { - throw invalid_request (r); - } - - write_state_ = true; - } - } + set_read_state () {state (request_state::reading);} + + virtual void + set_write_state () {state (request_state::writing);} private: request_rec* rec_; - bool buffer_ {true}; - bool write_state_ {false}; - std::unique_ptr<std::streambuf> out_buf_; - std::unique_ptr<std::ostream> out_; - std::unique_ptr<std::streambuf> in_buf_; - std::unique_ptr<std::istream> in_; + request_state state_ = request_state::initial; + path_type path_; std::unique_ptr<name_values> parameters_; std::unique_ptr<name_values> cookies_; std::unique_ptr<std::string> form_data_; + std::unique_ptr<std::streambuf> in_buf_; + std::unique_ptr<std::istream> in_; + + std::unique_ptr<std::streambuf> out_buf_; + std::unique_ptr<std::ostream> out_; }; } } diff --git a/web/apache/request.cxx b/web/apache/request.cxx index 2e03190..b0b4d61 100644 --- a/web/apache/request.cxx +++ b/web/apache/request.cxx @@ -4,22 +4,30 @@ #include <web/apache/request> -#include <apr_tables.h> +#include <apr_tables.h> // apr_table_*, apr_array_header_t +#include <apr_strings.h> // apr_pstrdup() + +#include <httpd.h> // request_rec, HTTP_*, OK +#include <http_protocol.h> // ap_*() #include <strings.h> // strcasecmp() -#include <ios> -#include <ctime> +#include <ctime> // strftime(), time_t #include <chrono> #include <memory> // unique_ptr +#include <string> +#include <cassert> #include <sstream> #include <ostream> #include <istream> -#include <cstring> +#include <cstring> // str*(), size_t #include <utility> // move() -#include <stdexcept> +#include <stdexcept> // invalid_argument +#include <exception> // current_exception() #include <streambuf> +#include <butl/optional> + #include <web/mime-url-encoding> using namespace std; @@ -28,16 +36,92 @@ namespace web { namespace apache { + void request:: + state (request_state s) + { + assert (s != request_state::initial); + + if (s == state_) + return; // Noop. + + if (s < state_) + { + // Can't "unwind" irrevocable interaction with Apache API. + // + static const char* names[] = { + "initial", "reading", "headers", "writing"}; + + string str ("web::apache::request::set_state: "); + str += names[static_cast<size_t> (state_)]; + str += " to "; + str += names[static_cast<size_t> (s)]; + + throw sequence_error (move (str)); + } + + if (s == request_state::reading) + { + // Prepare request content for reading. + // + int r (ap_setup_client_block (rec_, REQUEST_CHUNKED_DECHUNK)); + + if (r != OK) + throw invalid_request (r); + } + else if (s > request_state::reading && state_ <= request_state::reading) + { + // Read request content if any, discard whatever is received. + // + int r (ap_discard_request_body (rec_)); + + if (r != OK) + throw invalid_request (r); + } + + state_ = s; + } + + void request:: + rewind () + { + // @@ Request content buffering, and response cookies buffering are not + // supported yet. When done will be possible to rewind in broader + // range of cases. + // + + if (state_ == request_state::initial || + + // Form data have been read. Lucky case, can rewind. + // + (state_ == request_state::reading && + dynamic_cast<stringbuf*> (in_buf_.get ()) != nullptr)) + { + out_.reset (); + out_buf_.reset (); + + rec_->status = HTTP_OK; + + ap_set_content_type (rec_, nullptr); + + if (in_) + in_->seekg (0); + } + else + throw sequence_error ("web::apache::request::rewind"); + } + istream& request:: - content () + content (bool buffer) { + assert (!buffer); // Request content buffering is not implemented yet. + if (!in_) { unique_ptr<streambuf> in_buf (new istreambuf (rec_, *this)); in_.reset (new istream (in_buf.get ())); in_buf_ = move (in_buf); - in_->exceptions (ios::failbit | ios::badbit); + in_->exceptions (istream::failbit | istream::badbit); // Save form data now otherwise will not be available to do later // when data already read from stream. @@ -135,17 +219,29 @@ namespace web ostream& request:: content (status_code status, const string& type, bool buffer) { - if (out_ && status == rec_->status && buffer == buffer_ && + if (out_ && + + // Same status code. + // + status == rec_->status && + + // Same buffering flag. + // + buffer == + (dynamic_cast<stringbuf*> (out_buf_.get ()) != nullptr) && + + // Same content type. + // strcasecmp (rec_->content_type ? rec_->content_type : "", type.c_str ()) == 0) { + // No change, return the existing stream. + // return *out_; } - if (get_write_state ()) - { - throw sequence_error ("::web::apache::request::content"); - } + if (state_ >= request_state::writing) + throw sequence_error ("web::apache::request::content"); if (!buffer) // Request body will be discarded prior first byte of content is @@ -161,9 +257,8 @@ namespace web out_.reset (new ostream (out_buf.get ())); out_buf_ = move (out_buf); - out_->exceptions (ios::eofbit | ios::failbit | ios::badbit); + out_->exceptions (ostream::eofbit | ostream::failbit | ostream::badbit); - buffer_ = buffer; rec_->status = status; ap_set_content_type ( @@ -182,13 +277,10 @@ namespace web // where no sense to throw but still need to signal apache a // proper status code. // - if (get_write_state () && !current_exception ()) - { - throw sequence_error ("::web::apache::request::status"); - } + if (state_ >= request_state::writing && !current_exception ()) + throw sequence_error ("web::apache::request::status"); rec_->status = status; - buffer_ = true; out_.reset (); out_buf_.reset (); ap_set_content_type (rec_, nullptr); @@ -201,14 +293,10 @@ namespace web const chrono::seconds* max_age, const char* path, const char* domain, - bool secure) + bool secure, + bool buffer) { - if (get_write_state ()) - { - // Too late to send cookie if content is already written. - // - throw sequence_error ("::web::apache::request::cookie"); - } + assert (!buffer); // Cookie buffering is not implemented yet. ostringstream s; mime_url_encode (name, s); @@ -230,20 +318,15 @@ namespace web } if (path) - { s << ";Path=" << path; - } if (domain) - { s << ";Domain=" << domain; - } if (secure) - { s << ";Secure"; - } + state (request_state::headers); apr_table_add (rec_->err_headers_out, "Set-Cookie", s.str ().c_str ()); } diff --git a/web/apache/request.ixx b/web/apache/request.ixx index 8a3b32b..821aaba 100644 --- a/web/apache/request.ixx +++ b/web/apache/request.ixx @@ -4,10 +4,12 @@ #include <strings.h> // strncasecmp() -#include <iomanip> +#include <apr_tables.h> // apr_table_* + +#include <http_protocol.h> // ap_*() + #include <sstream> -#include <cstring> -#include <cstdlib> +#include <utility> // move() namespace web { @@ -16,35 +18,34 @@ namespace web inline int request:: flush () { - if (buffer_ && out_buf_) + if (std::stringbuf* b = dynamic_cast<std::stringbuf*> (out_buf_.get ())) { - auto b (dynamic_cast<std::stringbuf*> (out_buf_.get ())); - assert (b); - + // Response content is buffered. + // std::string s (b->str ()); if (!s.empty ()) { - // Before writing response read and discard request body if any. - // - int r (ap_discard_request_body (rec_)); - - if (r == OK) + try { - set_write_state (); + state (request_state::writing); if (ap_rwrite (s.c_str (), s.length (), rec_) < 0) rec_->status = HTTP_REQUEST_TIME_OUT; } - else - rec_->status = r; + catch (const invalid_request& e) + { + rec_->status = e.status; + } } out_.reset (); out_buf_.reset (); } - return rec_->status == HTTP_OK || get_write_state () ? OK : rec_->status; + return rec_->status == HTTP_OK || state_ >= request_state::writing + ? OK + : rec_->status; } inline const std::string& request:: @@ -64,19 +65,21 @@ namespace web std::istream& istr (content ()); // Do not throw when eofbit is set (end of stream reached), and - // when failbit is set (getline() failed to extract any character). + // when failbit is set (getline() failed to extract any + // character). // - istr.exceptions (std::ios::badbit); + istr.exceptions (std::istream::badbit); std::getline (istr, *form_data_); - // Make this data the content of the input stream. + // Make this data the content of the input stream, so it's + // available for the application as well. // std::unique_ptr<std::streambuf> in_buf ( new std::stringbuf (*form_data_)); in_.reset (new std::istream (in_buf.get ())); in_buf_ = std::move (in_buf); - in_->exceptions (std::ios::failbit | std::ios::badbit); + in_->exceptions (std::istream::failbit | std::istream::badbit); } } } diff --git a/web/apache/service b/web/apache/service index 4c0d395..165ff90 100644 --- a/web/apache/service +++ b/web/apache/service @@ -5,8 +5,11 @@ #ifndef WEB_APACHE_SERVICE #define WEB_APACHE_SERVICE -#include <httpd.h> -#include <http_config.h> // module, ap_hook_*() +#include <apr_pools.h> // apr_pool_t +#include <apr_hooks.h> // APR_HOOK_* + +#include <httpd.h> // request_rec, server_rec, HTTP_*, DECLINED +#include <http_config.h> // module, cmd_parms, ap_hook_*() #include <map> #include <memory> // unique_ptr @@ -130,7 +133,8 @@ namespace web // The worker_initializer() function is called right after Apache // worker process is started. Called for every new process spawned. // - ap_hook_child_init (&worker_initializer<M>, NULL, NULL, APR_HOOK_LAST); + ap_hook_child_init ( + &worker_initializer<M>, NULL, NULL, APR_HOOK_LAST); // The request_handler () function is called for each client request. // @@ -254,7 +258,8 @@ namespace web parse_option (cmd_parms* parms, void* conf, const char* args) noexcept; const char* - add_option (context_id id, const char* name, optional<std::string> value); + add_option ( + context_id id, const char* name, optional<std::string> value); void finalize_config (server_rec*); diff --git a/web/apache/service.cxx b/web/apache/service.cxx index 2ca8cf0..da44530 100644 --- a/web/apache/service.cxx +++ b/web/apache/service.cxx @@ -4,10 +4,10 @@ #include <web/apache/service> -#include <apr_pools.h> +#include <apr_pools.h> // apr_palloc() -#include <httpd.h> -#include <http_config.h> +#include <httpd.h> // server_rec +#include <http_config.h> // command_rec, cmd_*, ap_get_module_config() #include <memory> // unique_ptr #include <string> @@ -16,7 +16,10 @@ #include <cstring> // strlen() #include <exception> +#include <butl/optional> + #include <web/module> +#include <web/apache/log> using namespace std; @@ -29,12 +32,12 @@ namespace web { assert (cmds == nullptr); - // Fill apache module directive definitions. Directives share - // common name space in apache configuration file, so to prevent name - // clash have to form directive name as a combination of module and - // option names: <module name>-<option name>. This why for option - // bar of module foo the corresponding directive will appear in apache - // configuration file as foo-bar. + // Fill apache module directive definitions. Directives share common + // name space in apache configuration file, so to prevent name clash + // have to form directive name as a combination of module and option + // names: <module name>-<option name>. This why for option bar of module + // foo the corresponding directive will appear in apache configuration + // file as foo-bar. // const option_descriptions& od (exemplar_.options ()); unique_ptr<command_rec[]> directives (new command_rec[od.size () + 1]); @@ -42,7 +45,8 @@ namespace web for (const auto& o: od) { - auto i (option_descriptions_.emplace (name_ + "-" + o.first, o.second)); + auto i ( + option_descriptions_.emplace (name_ + "-" + o.first, o.second)); assert (i.second); *d++ = @@ -73,8 +77,8 @@ namespace web create_server_context (apr_pool_t* pool, server_rec*) noexcept { // Create the object using the configuration memory pool provided by the - // Apache API. The lifetime of the object is equal to the lifetime of the - // pool. + // Apache API. The lifetime of the object is equal to the lifetime of + // the pool. // void* p (apr_palloc (pool, sizeof (context))); assert (p != nullptr); @@ -104,10 +108,10 @@ namespace web service& srv (*reinterpret_cast<service*> (parms->cmd->cmd_data)); if (srv.options_parsed_) - // Apache have started the second pass of its messy initialization cycle - // (more details at http://wiki.apache.org/httpd/ModuleLife). This time - // we are parsing for real. Cleanup the existing config, and start - // building the new one. + // Apache have started the second pass of its messy initialization + // cycle (more details at http://wiki.apache.org/httpd/ModuleLife). + // This time we are parsing for real. Cleanup the existing config, and + // start building the new one. // srv.clear_config (); @@ -214,8 +218,12 @@ namespace web // context options as a result of the merge_server_context() calls. // for (const auto& o: options_) - if (o.first->server != nullptr) // Is a directory configuration context. + { + // Is a directory configuration context. + // + if (o.first->server != nullptr) complement (o.first, o.first->server); + } options_parsed_ = true; } diff --git a/web/apache/service.txx b/web/apache/service.txx index b57befc..961c19b 100644 --- a/web/apache/service.txx +++ b/web/apache/service.txx @@ -3,9 +3,9 @@ // license : MIT; see accompanying LICENSE file #include <unistd.h> // getppid() -#include <signal.h> // kill() +#include <signal.h> // kill(), SIGTERM -#include <http_log.h> +#include <http_log.h> // APLOG_* #include <utility> // move() #include <exception> @@ -132,21 +132,31 @@ namespace web const M* e (dynamic_cast<const M*> (exemplar)); assert (e != nullptr); - M m (*e); - - if (static_cast<module&> (m).handle (rq, rq, lg)) - return rq.flush (); + for (M m (*e);;) + { + try + { + if (static_cast<module&> (m).handle (rq, rq, lg)) + return rq.flush (); - if (!rq.get_write_state ()) - return DECLINED; + if (rq.state () == request_state::initial) + return DECLINED; - lg.write (nullptr, 0, func_name.c_str (), APLOG_ERR, - "handling declined while unbuffered content " - "has been written"); + lg.write (nullptr, 0, func_name.c_str (), APLOG_ERR, + "handling declined being partially executed"); + break; + } + catch (const module::retry&) + { + // Retry to handle the request. + // + rq.rewind (); + } + } } catch (const invalid_request& e) { - if (!e.content.empty () && !rq.get_write_state ()) + if (!e.content.empty () && rq.state () < request_state::writing) { try { @@ -165,11 +175,12 @@ namespace web { lg.write (nullptr, 0, func_name.c_str (), APLOG_ERR, e.what ()); - if (*e.what () && !rq.get_write_state ()) + if (*e.what () && rq.state () < request_state::writing) { try { - rq.content (HTTP_INTERNAL_SERVER_ERROR, "text/plain;charset=utf-8") + rq.content ( + HTTP_INTERNAL_SERVER_ERROR, "text/plain;charset=utf-8") << e.what (); return rq.flush (); @@ -184,11 +195,12 @@ namespace web { lg.write (nullptr, 0, func_name.c_str (), APLOG_ERR, "unknown error"); - if (!rq.get_write_state ()) + if (rq.state () < request_state::writing) { try { - rq.content (HTTP_INTERNAL_SERVER_ERROR, "text/plain;charset=utf-8") + rq.content ( + HTTP_INTERNAL_SERVER_ERROR, "text/plain;charset=utf-8") << "unknown error"; return rq.flush (); diff --git a/web/apache/stream b/web/apache/stream index 2ea9b7e..f05ea08 100644 --- a/web/apache/stream +++ b/web/apache/stream @@ -5,44 +5,56 @@ #ifndef WEB_APACHE_STREAM #define WEB_APACHE_STREAM -#include <httpd.h> -#include <http_protocol.h> +#include <httpd.h> // request_rec, HTTP_* +#include <http_protocol.h> // ap_*() #include <ios> // streamsize #include <vector> -#include <cstring> // memmove() +#include <cstring> // memmove(), size_t #include <streambuf> #include <algorithm> // min(), max() -#include <web/module> +#include <web/module> // invalid_request namespace web { namespace apache { - // Object of a class implementing this interface is intended for - // keeping the state of writing response to the client. + // Object of a class implementing this interface is intended for keeping + // the state of communication with the client. // - struct write_state + struct stream_state { - // Called by ostreambuf methods when some content to be written to the - // client. + // Called by istreambuf functions when content is about to be read from + // the client. Can throw invalid_request or sequence_error. // virtual void - set_write_state () = 0; + set_read_state () = 0; - // Called to check if any data have already been written to the client. - // Such checks required for some operations which are impossible to - // execute after response is partially written. + // Called by ostreambuf functions when some content is about to be + // written to the client. Can throw invalid_request or sequence_error. // - virtual bool - get_write_state () const noexcept = 0; + virtual void + set_write_state () = 0; + }; + + // Base class for ostreambuf and istreambuf. References request and + // communication state structures. + // + class rbuf: public std::streambuf + { + protected: + rbuf (request_rec* r, stream_state& s): rec_ (r), state_ (s) {} + + protected: + request_rec* rec_; + stream_state& state_; }; - class ostreambuf: public std::streambuf + class ostreambuf: public rbuf { public: - ostreambuf (request_rec* rec, write_state& ws): rec_ (rec), ws_ (ws) {} + ostreambuf (request_rec* r, stream_state& s): rbuf (r, s) {} private: virtual int_type @@ -50,7 +62,7 @@ namespace web { if (c != traits_type::eof ()) { - ws_.set_write_state (); + state_.set_write_state (); char chr (c); @@ -67,12 +79,10 @@ namespace web virtual std::streamsize xsputn (const char* s, std::streamsize num) { - ws_.set_write_state (); + state_.set_write_state (); if (ap_rwrite (s, num, rec_) < 0) - { throw invalid_request (HTTP_REQUEST_TIME_OUT); - } return num; } @@ -81,59 +91,37 @@ namespace web sync () { if (ap_rflush (rec_) < 0) - { throw invalid_request (HTTP_REQUEST_TIME_OUT); - } return 0; } - - private: - request_rec* rec_; - write_state& ws_; }; - class istreambuf: public std::streambuf + class istreambuf: public rbuf { public: - istreambuf (request_rec* rec, - write_state& ws, + istreambuf (request_rec* r, + stream_state& s, size_t bufsize = 1024, size_t putback = 1) - : rec_ (rec), - ws_ (ws), + : rbuf (r, s), bufsize_ (std::max (bufsize, (size_t)1)), putback_ (std::min (putback, bufsize_ - 1)), buf_ (bufsize_) { - if (ws_.get_write_state ()) - { - throw sequence_error ("::web::apache::istreambuf::istreambuf"); - } - char* p (buf_.data () + putback_); setg (p, p, p); - - int status (ap_setup_client_block (rec_, REQUEST_CHUNKED_DECHUNK)); - - if (status != OK) - { - throw invalid_request (status); - } } private: virtual int_type underflow () { - if (ws_.get_write_state ()) - { - throw sequence_error ("::web::apache::istreambuf::underflow"); - } - if (gptr () < egptr ()) return traits_type::to_int_type (*gptr ()); + state_.set_read_state (); + size_t pb (std::min ((size_t)(gptr () - eback ()), putback_)); std::memmove (buf_.data () + putback_ - pb, gptr () - pb, pb); @@ -141,22 +129,16 @@ namespace web int rb (ap_get_client_block (rec_, p, bufsize_ - putback_)); if (rb == 0) - { return traits_type::eof (); - } if (rb < 0) - { throw invalid_request (HTTP_REQUEST_TIME_OUT); - } setg (p - pb, p, p + rb); return traits_type::to_int_type (*gptr ()); } private: - request_rec* rec_; - write_state& ws_; size_t bufsize_; size_t putback_; std::vector<char> buf_; @@ -111,13 +111,13 @@ namespace web virtual const name_values& cookies () = 0; - // Get the stream to read the request content from. Note that - // reading content after any unbuffered content has been written - // is undefined behavior. The implementation may detect it and - // throw sequence_error but is not required to do so. + // Get the stream to read the request content from. If the buffer argument + // is false, then reading content after any unbuffered content has been + // written or after a retry is undefined behavior. The implementation may + // detect this and throw sequence_error but is not required to do so. // virtual std::istream& - content () = 0; + content (bool buffer = false) = 0; }; class response @@ -160,10 +160,11 @@ namespace web virtual void cookie (const char* name, const char* value, - const std::chrono::seconds* max_age = 0, - const char* path = 0, - const char* domain = 0, - bool secure = false) = 0; + const std::chrono::seconds* max_age = nullptr, + const char* path = nullptr, + const char* domain = nullptr, + bool secure = false, + bool buffer = true) = 0; }; // A web server logging backend. The module can use it to log @@ -229,15 +230,26 @@ namespace web // Return false if decline to handle the request. If handling have been // declined after any unbuffered content has been written, then the // implementation shall terminate the response in a suitable but - // unspecified manner. Any exception other than invalid_request described - // above that leaves this function is treated by the web server - // implementation as an internal server error (500). Similar to - // invalid_request, it will try to return the status and description - // (obtained by calling what() on std::exception) to the client, if - // possible. The description is assume to be encoded in UTF-8. The - // implementation may provide a configuration option to omit the - // description from the response, for security/privacy reasons. + // unspecified manner. // + // Throw retry if need to retry handling the request. The retry will + // happen on the same instance of the module and the implementation is + // expected to "rewind" the request and response objects to their initial + // state. This is only guaranteed to be possible if the relevant functions + // in the request and response objects were called in buffered mode (the + // buffer argument was true). + // + // Any exception other than retry and invalid_request described above that + // leaves this function is treated by the web server implementation as an + // internal server error (500). Similar to invalid_request, it will try to + // return the status and description (obtained by calling what() on + // std::exception) to the client, if possible. The description is assume + // to be encoded in UTF-8. The implementation may provide a configuration + // option to omit the description from the response, for security/privacy + // reasons. + // + struct retry {}; + virtual bool handle (request&, response&, log&) = 0; }; |