Summary:
This restore's format()'s behavior of crashing on invalid format
strings, and adds an alternative formatChecked() function to throw
instead of crashing.
Format strings are commonly programmer defined, and fixed at compile
time. Bad format strings are usually programmer errors, and crashing is
desirable to help catch these bugs early.
However, it is also useful to support dynamic format strings which are
not guaranteed to be well formed at compile time. formatChecked() makes
it safe to use dynamic format strings, as a bad format strings will not
crash the program.
This does change the throwing/crashing behavior slightly: the old
format() code also used to crash if the format string referred to a
non-existent key in one of the argument containers. I removed this,
since it seems like the argument containers are likely to be dynamic.
I also changed the code to crash on std::range_errors as well. Various
problems in format string arguments are caught in the Conv.h code, which
throws range_errors. The old crashing code did not crash on these
errors, but it seems useful to do so. The only minor concern here is
that this may also crash unintentionally if the Output callback throws a
range_error. This seems low-risk, but we can remove this behavior if
needed.
Test Plan:
Updated the BogusFormatString test to check both format() and
formatChecked().
Reviewed By: tudorb@fb.com
FB internal diff:
D1144301
"Exactly one argument required in container mode");
}
+template <bool containerMode, class... Args>
+void Formatter<containerMode, Args...>::handleFormatStrError() const {
+ if (crashOnError_) {
+ LOG(FATAL) << "folly::format: bad format string \"" << str_ << "\": " <<
+ folly::exceptionStr(std::current_exception());
+ }
+ throw;
+}
+
template <bool containerMode, class... Args>
template <class Output>
void Formatter<containerMode, Args...>::operator()(Output& out) const {
+ // Catch BadFormatArg and range_error exceptions, and call
+ // handleFormatStrError().
+ //
+ // These exception types indicate a problem with the format string. Most
+ // format strings are string literals specified by the programmer. If they
+ // have a problem, this is usually a programmer bug. We want to crash to
+ // ensure that these are found early on during development.
+ //
+ // BadFormatArg is thrown by the Format.h code, while range_error is thrown
+ // by Conv.h, which is used in several places in our format string
+ // processing.
+ //
+ // (Note: This behavior is slightly dangerous. If the Output object throws a
+ // BadFormatArg or a range_error, we will also crash the program, even if it
+ // wasn't an issue with the format string. This seems highly unlikely
+ // though, and none of our current Output objects can throw these errors.)
+ //
+ // We also throw out_of_range errors if the format string references an
+ // argument that isn't present (or a key that isn't present in one of the
+ // argument containers). However, at the moment we don't crash on these
+ // errors, as it is likely that the container is dynamic at runtime.
+ try {
+ appendOutput(out);
+ } catch (const BadFormatArg& ex) {
+ handleFormatStrError();
+ } catch (const std::range_error& ex) {
+ handleFormatStrError();
+ }
+}
+
+template <bool containerMode, class... Args>
+template <class Output>
+void Formatter<containerMode, Args...>::appendOutput(Output& out) const {
auto p = str_.begin();
auto end = str_.end();
p = q;
if (p == end || *p != '}') {
- throw std::invalid_argument(
- "folly::format: single '}' in format string");
+ throw BadFormatArg("folly::format: single '}' in format string");
}
++p;
}
p = q + 1;
if (p == end) {
- throw std::invalid_argument(
- "folly::format: '}' at end of format string");
+ throw BadFormatArg("folly::format: '}' at end of format string");
}
// "{{" -> "{"
// Format string
q = static_cast<const char*>(memchr(p, '}', end - p));
if (q == nullptr) {
- throw std::invalid_argument("folly::format: missing ending '}'");
+ throw BadFormatArg("folly::format: missing ending '}'");
}
FormatArg arg(StringPiece(p, q));
p = q + 1;
}
if (hasDefaultArgIndex && hasExplicitArgIndex) {
- throw std::invalid_argument(
+ throw BadFormatArg(
"folly::format: may not have both default and explicit arg indexes");
}
template <bool containerMode, class... Args>
class Formatter {
- template <class... A>
- friend Formatter<false, A...> format(StringPiece fmt, A&&... arg);
- template <class C>
- friend Formatter<true, C> vformat(StringPiece fmt, C&& container);
public:
+ /*
+ * Change whether or not Formatter should crash or throw exceptions if the
+ * format string is invalid.
+ *
+ * Crashing is desirable for literal format strings that are fixed at compile
+ * time. Errors in the format string are generally programmer bugs, and
+ * should be caught early on in development. Crashing helps ensure these
+ * problems are noticed.
+ */
+ void setCrashOnError(bool crash) {
+ crashOnError_ = crash;
+ }
+
/**
* Append to output. out(StringPiece sp) may be called (more than once)
*/
typename std::decay<Args>::type>...> ValueTuple;
static constexpr size_t valueCount = std::tuple_size<ValueTuple>::value;
+ void handleFormatStrError() const FOLLY_NORETURN;
+ template <class Output>
+ void appendOutput(Output& out) const;
+
template <size_t K, class Callback>
typename std::enable_if<K == valueCount>::type
doFormatFrom(size_t i, FormatArg& arg, Callback& cb) const {
StringPiece str_;
ValueTuple values_;
+ bool crashOnError_{true};
+
+ template <class... A>
+ friend Formatter<false, A...> format(StringPiece fmt, A&&... arg);
+ template <class... A>
+ friend Formatter<false, A...> formatChecked(StringPiece fmt, A&&... arg);
+ template <class C>
+ friend Formatter<true, C> vformat(StringPiece fmt, C&& container);
+ template <class C>
+ friend Formatter<true, C> vformatChecked(StringPiece fmt, C&& container);
};
/**
* std::string formatted = format("{} {}", 23, 42).str();
* LOG(INFO) << format("{} {}", 23, 42);
* writeTo(stdout, format("{} {}", 23, 42));
+ *
+ * Note that format() will crash the program if the format string is invalid.
+ * Normally, the format string is a fixed string literal specified by the
+ * programmer. Invalid format strings are normally programmer bugs, and should
+ * be caught early on during development. Crashing helps ensure these bugs are
+ * found.
+ *
+ * Use formatChecked() if you have a dynamic format string (for example, a user
+ * supplied value). formatChecked() will throw an exception rather than
+ * crashing the program.
*/
template <class... Args>
Formatter<false, Args...> format(StringPiece fmt, Args&&... args) {
fmt, std::forward<Args>(args)...);
}
+/**
+ * Create a formatter object from a dynamic format string.
+ *
+ * This is identical to format(), but throws an exception if the format string
+ * is invalid, rather than aborting the program. This allows it to be used
+ * with user-specified format strings which are not guaranteed to be well
+ * formed.
+ */
+template <class... Args>
+Formatter<false, Args...> formatChecked(StringPiece fmt, Args&&... args) {
+ Formatter<false, Args...> f(fmt, std::forward<Args>(args)...);
+ f.setCrashOnError(false);
+ return f;
+}
+
/**
* Create a formatter object that takes one argument (of container type)
* and uses that container to get argument values from.
fmt, std::forward<Container>(container));
}
+/**
+ * Create a formatter object from a dynamic format string.
+ *
+ * This is identical to vformat(), but throws an exception if the format string
+ * is invalid, rather than aborting the program. This allows it to be used
+ * with user-specified format strings which are not guaranteed to be well
+ * formed.
+ */
+template <class Container>
+Formatter<true, Container> vformatChecked(StringPiece fmt,
+ Container&& container) {
+ Formatter<true, Container> f(fmt, std::forward<Container>(container));
+ f.setCrashOnError(false);
+ return f;
+}
+
/**
* Append formatted output to a string.
*
namespace folly {
+class BadFormatArg : public std::invalid_argument {
+ public:
+ explicit BadFormatArg(const std::string& msg)
+ : std::invalid_argument(msg) {}
+};
+
/**
* Parsed format argument.
*/
template <typename... Args>
inline void FormatArg::error(Args&&... args) const {
- throw
std::invalid_argument(errorStr(std::forward<Args>(args)...));
+ throw
BadFormatArg(errorStr(std::forward<Args>(args)...));
}
template <bool emptyOk>
return vformat(fmt, c).str();
}
+template <class... Args>
+std::string fstrChecked(StringPiece fmt, Args&&... args) {
+ return formatChecked(fmt, std::forward<Args>(args)...).str();
+}
+
+template <class C>
+std::string vstrChecked(StringPiece fmt, const C& c) {
+ return vformatChecked(fmt, c).str();
+}
+
template <class Uint>
void compareOctal(Uint u) {
char buf1[detail::kMaxOctalLength + 1];
TEST(Format, Unformatted) {
Opaque o;
EXPECT_NE("", fstr("{}", &o));
- EXPECT_THROW(fstr("{0[0]}", &o), std::invalid_argument);
+ EXPECT_DEATH(fstr("{0[0]}", &o), "No formatter available for this type");
+ EXPECT_THROW(fstrChecked("{0[0]}", &o), std::invalid_argument);
}
TEST(Format, Nested) {
std::vector<int> ints{1, 2, 3, 4, 5};
EXPECT_EQ("1 3 5", fstr("{0[0]} {0[2]} {0[4]}", ints));
EXPECT_THROW(fstr("{[5]}", ints), std::out_of_range);
+ EXPECT_THROW(fstrChecked("{[5]}", ints), std::out_of_range);
std::map<std::string, int> map{{"hello", 0}, {"world", 1}};
EXPECT_EQ("hello = 0", fstr("hello = {[hello]}", map));
EXPECT_THROW(fstr("{[nope]}", map), std::out_of_range);
EXPECT_THROW(vstr("{nope}", map), std::out_of_range);
+ EXPECT_THROW(vstrChecked("{nope}", map), std::out_of_range);
}
TEST(Format, BogusFormatString) {
- EXPECT_THROW(fstr("}"), std::invalid_argument);
- EXPECT_THROW(fstr("foo}bar"), std::invalid_argument);
- EXPECT_THROW(fstr("foo{bar"), std::invalid_argument);
- EXPECT_THROW(fstr("{[test]"), std::invalid_argument);
+ // format() will crash the program if the format string is invalid.
+ EXPECT_DEATH(fstr("}"), "single '}' in format string");
+ EXPECT_DEATH(fstr("foo}bar"), "single '}' in format string");
+ EXPECT_DEATH(fstr("foo{bar"), "missing ending '}'");
+ EXPECT_DEATH(fstr("{[test]"), "missing ending '}'");
+ EXPECT_DEATH(fstr("{-1.3}"), "argument index must be non-negative");
+ EXPECT_DEATH(fstr("{1.3}", 0, 1, 2), "index not allowed");
+ EXPECT_DEATH(fstr("{0} {} {1}", 0, 1, 2),
+ "may not have both default and explicit arg indexes");
+
+ // formatChecked() should throw exceptions rather than crashing the program
+ EXPECT_THROW(fstrChecked("}"), std::invalid_argument);
+ EXPECT_THROW(fstrChecked("foo}bar"), std::invalid_argument);
+ EXPECT_THROW(fstrChecked("foo{bar"), std::invalid_argument);
+ EXPECT_THROW(fstrChecked("{[test]"), std::invalid_argument);
+ EXPECT_THROW(fstrChecked("{-1.3}"), std::invalid_argument);
+ EXPECT_THROW(fstrChecked("{1.3}", 0, 1, 2), std::invalid_argument);
+ EXPECT_THROW(fstrChecked("{0} {} {1}", 0, 1, 2), std::invalid_argument);
// This one fails in detail::enforceWhitespace(), which throws
// std::range_error
- EXPECT_THROW(fstr("{0[test}"), std::exception);
+ EXPECT_DEATH(fstr("{0[test}"), "Non-whitespace: \\[");
+ EXPECT_THROW(fstrChecked("{0[test}"), std::exception);
}
int main(int argc, char *argv[]) {
projects / folly.git / commitdiff