[svn] commit: r3996 - in /branches/trac446/src/bin/auth: change_user.cc common.h config.cc config.h tests/config_unittest.cc
BIND 10 source code commits
bind10-changes at lists.isc.org
Thu Dec 23 22:17:13 UTC 2010
Author: jinmei
Date: Thu Dec 23 22:17:13 2010
New Revision: 3996
Log:
addressed another review comment:
- allowed commit() to throw an exception (although discouraging it strongly), and catch and convert it in configureAuthServer()
- added a test for that scenario
- changed the type of FatalError to make sure it won't be caught in the middle of server program
Modified:
branches/trac446/src/bin/auth/change_user.cc
branches/trac446/src/bin/auth/common.h
branches/trac446/src/bin/auth/config.cc
branches/trac446/src/bin/auth/config.h
branches/trac446/src/bin/auth/tests/config_unittest.cc
Modified: branches/trac446/src/bin/auth/change_user.cc
==============================================================================
--- branches/trac446/src/bin/auth/change_user.cc (original)
+++ branches/trac446/src/bin/auth/change_user.cc Thu Dec 23 22:17:13 2010
@@ -26,6 +26,7 @@
#include <auth/common.h>
using namespace boost;
+using namespace std;
void
changeUser(const char* const username) {
@@ -42,14 +43,14 @@
}
}
if (runas_pw == NULL) {
- isc_throw(FatalError, "Unknown user name or UID:" << username);
+ throw FatalError("Unknown user name or UID:" + string(username));
}
if (setgid(runas_pw->pw_gid) < 0) {
- isc_throw(FatalError, "setgid() failed: " << strerror(errno));
+ throw FatalError("setgid() failed: " + string(strerror(errno)));
}
if (setuid(runas_pw->pw_uid) < 0) {
- isc_throw(FatalError, "setuid() failed: " << strerror(errno));
+ throw FatalError("setuid() failed: " + string(strerror(errno)));
}
}
Modified: branches/trac446/src/bin/auth/common.h
==============================================================================
--- branches/trac446/src/bin/auth/common.h (original)
+++ branches/trac446/src/bin/auth/common.h Thu Dec 23 22:17:13 2010
@@ -17,12 +17,18 @@
#ifndef __COMMON_H
#define __COMMON_H 1
-#include <exceptions/exceptions.h>
+#include <stdexcept>
+#include <string>
-class FatalError : public isc::Exception {
+/// An exception class that is thrown in an unrecoverable error condition.
+///
+/// This exception should not be caught except at the highest level of
+/// the application only for terminating the program gracefully, and so
+/// it cannot be a derived class of \c isc::Exception.
+class FatalError : public std::runtime_error {
public:
- FatalError(const char* file, size_t line, const char* what) :
- isc::Exception(file, line, what) {}
+ FatalError(const std::string& what) : std::runtime_error(what)
+ {}
};
#endif // __COMMON_H
Modified: branches/trac446/src/bin/auth/config.cc
==============================================================================
--- branches/trac446/src/bin/auth/config.cc (original)
+++ branches/trac446/src/bin/auth/config.cc Thu Dec 23 22:17:13 2010
@@ -30,6 +30,7 @@
#include <auth/auth_srv.h>
#include <auth/config.h>
+#include <auth/common.h>
using namespace std;
using boost::shared_ptr;
@@ -162,6 +163,16 @@
//
}
}
+
+/// A special parser for testing: it throws from commit() despite the
+/// suggested convention of the class interface.
+class ThrowerCommitConfig : public AuthConfigParser {
+public:
+ virtual void build(ConstElementPtr) {} // ignore param, do nothing
+ virtual void commit() {
+ throw 10;
+ }
+};
// This is a generalized version of create function that can create
// an AuthConfigParser object for "internal" use.
@@ -177,6 +188,14 @@
return (new DatasourcesConfig(server));
} else if (internal && config_id == "datasources/memory") {
return (new MemoryDatasourceConfig(server));
+ } else if (config_id == "_commit_throw") {
+ // This is for testing purpose only and should not appear in the
+ // actual configuration syntax. While this could crash the caller
+ // as a result, the server implementation is expected to perform
+ // syntax level validation and should be safe in practice. In future,
+ // we may introduce dynamic registration of configuration parsers,
+ // and then this test can be done in a cleaner and safer way.
+ return (new ThrowerCommitConfig());
} else {
isc_throw(AuthConfigError, "Unknown configuration identifier: " <<
config_id);
@@ -220,7 +239,12 @@
ex.what());
}
- BOOST_FOREACH(ParserPtr parser, parsers) {
- parser->commit();
- }
-}
+ try {
+ BOOST_FOREACH(ParserPtr parser, parsers) {
+ parser->commit();
+ }
+ } catch (...) {
+ throw FatalError("Unrecoverable error: "
+ "a configuration parser threw in commit");
+ }
+}
Modified: branches/trac446/src/bin/auth/config.h
==============================================================================
--- branches/trac446/src/bin/auth/config.h (original)
+++ branches/trac446/src/bin/auth/config.h Thu Dec 23 22:17:13 2010
@@ -54,7 +54,9 @@
/// value(s) to be applied to the server. This method may throw an exception
/// when it encounters an error.
/// The \c commit() method actually applies the new configuration value
-/// to the server. It must not throw an exception.
+/// to the server. It's basically not expected to throw an exception;
+/// any configuration operations that can fail (such as ones involving
+/// resource allocation) should be done in \c build().
///
/// When the destructor is called before \c commit(), the destructor is
/// supposed to make sure the state of the \c AuthSrv object is the same
@@ -119,10 +121,15 @@
/// Apply the prepared configuration value to the server.
///
- /// This method must be exception free, and, as a consequence, it should
- /// normally not involve resource allocation.
+ /// This method is expected to be exception free, and, as a consequence,
+ /// it should normally not involve resource allocation.
/// Typically it would simply perform exception free assignment or swap
/// operation on the value prepared in \c build().
+ /// In some cases, however, it may be very difficult to meet this
+ /// condition in a realistic way, while the failure case should really
+ /// be very rare. In such a case it may throw, and, if the parser is
+ /// called via \c configureAuthServer(), the caller will convert the
+ /// exception as a fatal error.
///
/// This method is expected to be called after \c build(), and only once.
/// The result is undefined otherwise.
@@ -145,7 +152,12 @@
/// a corresponding standard exception will be thrown.
/// Other exceptions may also be thrown, depending on the implementation of
/// the underlying derived class of \c AuthConfigError.
-/// In any case the strong guarantee is provided as described above.
+/// In any case the strong guarantee is provided as described above except
+/// in the very rare cases where the \c commit() method of a parser throws
+/// an exception. If that happens this function converts the exception
+/// into a \c FatalError exception and rethrows it. This exception is
+/// expected to be caught at the highest level of the application to terminate
+/// the program gracefully.
///
/// \param server The \c AuthSrv object to be configured.
/// \param config_set A JSON style configuration to apply to \c server.
Modified: branches/trac446/src/bin/auth/tests/config_unittest.cc
==============================================================================
--- branches/trac446/src/bin/auth/tests/config_unittest.cc (original)
+++ branches/trac446/src/bin/auth/tests/config_unittest.cc Thu Dec 23 22:17:13 2010
@@ -26,6 +26,7 @@
#include <auth/auth_srv.h>
#include <auth/config.h>
+#include <auth/common.h>
#include <testutils/mockups.h>
@@ -101,6 +102,12 @@
TEST_F(AuthConfigTest, unknownConfigVar) {
EXPECT_THROW(createAuthConfigParser(server, "no_such_config_var"),
AuthConfigError);
+}
+
+TEST_F(AuthConfigTest, exceptionFromCommit) {
+ EXPECT_THROW(configureAuthServer(server, Element::fromJSON(
+ "{\"_commit_throw\": 10}")),
+ FatalError);
}
class MemoryDatasrcConfigTest : public AuthConfigTest {
More information about the bind10-changes
mailing list