Check progress status format string validity first and fall back to default if it's invalid

CMakeLists.txt

@@ -275,6 +275,7 @@ if(BUILD_TESTING)
     src/missing_deps_test.cc
     src/ninja_test.cc
     src/state_test.cc
+    src/status_printer_test.cc
     src/string_piece_util_test.cc
     src/subprocess_test.cc
     src/test.cc

src/build_test.cc

@@ -2258,35 +2258,6 @@ TEST_F(BuildTest, DepsGccWithEmptyDepfileErrorsOut) {
   ASSERT_EQ(1u, command_runner_.commands_ran_.size());
 }
 
-TEST_F(BuildTest, StatusFormatElapsed_e) {
-  status_.BuildStarted();
-  // Before any task is done, the elapsed time must be zero.
-  EXPECT_EQ("[%/e0.000]", status_.FormatProgressStatus("[%%/e%e]", 0));
-}
-
-TEST_F(BuildTest, StatusFormatElapsed_w) {
-  status_.BuildStarted();
-  // Before any task is done, the elapsed time must be zero.
-  EXPECT_EQ("[%/e00:00]", status_.FormatProgressStatus("[%%/e%w]", 0));
-}
-
-TEST_F(BuildTest, StatusFormatETA) {
-  status_.BuildStarted();
-  // Before any task is done, the ETA time must be unknown.
-  EXPECT_EQ("[%/E?]", status_.FormatProgressStatus("[%%/E%E]", 0));
-}
-
-TEST_F(BuildTest, StatusFormatTimeProgress) {
-  status_.BuildStarted();
-  // Before any task is done, the percentage of elapsed time must be zero.
-  EXPECT_EQ("[%/p  0%]", status_.FormatProgressStatus("[%%/p%p]", 0));
-}
-
-TEST_F(BuildTest, StatusFormatReplacePlaceholder) {
-  EXPECT_EQ("[%/s0/t0/r0/u0/f0]",
-            status_.FormatProgressStatus("[%%/s%s/t%t/r%r/u%u/f%f]", 0));
-}
-
 TEST_F(BuildTest, FailedDepsParse) {
   ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
 "build bad_deps.o: cat in1\n"

src/status.h

@@ -39,9 +39,9 @@ struct Status {
   /// (which is the default).
   virtual void SetExplanations(Explanations*) = 0;
 
-  virtual void Info(const char* msg, ...) = 0;
-  virtual void Warning(const char* msg, ...) = 0;
-  virtual void Error(const char* msg, ...) = 0;
+  virtual void Info(const char* msg, ...) const = 0;
+  virtual void Warning(const char* msg, ...) const = 0;
+  virtual void Error(const char* msg, ...) const = 0;
 
   virtual ~Status() { }
 

src/status_printer.cc

@@ -42,15 +42,24 @@ Status* Status::factory(const BuildConfig& config) {
 
 StatusPrinter::StatusPrinter(const BuildConfig& config)
     : config_(config), started_edges_(0), finished_edges_(0), total_edges_(0),
-      running_edges_(0), progress_status_format_(NULL),
-      current_rate_(config.parallelism) {
+      running_edges_(0), current_rate_(config.parallelism) {
   // Don't do anything fancy in verbose mode.
   if (config_.verbosity != BuildConfig::NORMAL)
     printer_.set_smart_terminal(false);
 
+  // The progress status format to use by default
+  static const char kDefaultProgressStatusFormat[] = "[%f/%t] ";
+
   progress_status_format_ = getenv("NINJA_STATUS");
+
+  std::string error_output;
+  if (progress_status_format_ &&
+      !IsValidProgressStatus(progress_status_format_, &error_output)) {
+    Warning("%s", error_output.c_str());
+    progress_status_format_ = nullptr;
+  }
   if (!progress_status_format_)
-    progress_status_format_ = "[%f/%t] ";
+    progress_status_format_ = kDefaultProgressStatusFormat;
 }
 
 void StatusPrinter::EdgeAddedToPlan(const Edge* edge) {
@@ -257,6 +266,45 @@ void StatusPrinter::BuildFinished() {
   printer_.PrintOnNewLine("");
 }
 
+bool StatusPrinter::IsValidProgressStatus(const char* progress_status_format,
+                                          string* error_output) {
+  for (const char* s = progress_status_format; *s != '\0'; ++s) {
+    if (*s != '%')
+      continue;
+
+    ++s;
+    switch (*s) {
+    case 's':  // The number of started edges.
+    case 't':  // The total number of edges that must be run to complete the
+               // build.
+    case 'p':  // The percentage of finished edges.
+    case 'r':  // The number of currently running edges.
+    case 'u':  // The number of remaining edges to start.
+    case 'f':  // The number of finished edges.
+    case 'o':  // Overall rate of finished edges per second
+    case 'c':  // Current rate of finished edges per second (average over builds
+    case 'e':  // Elapsed time in seconds.
+    case 'E':  // Remaining time (ETA) in seconds.
+    case 'w':  // Elapsed time in [h:]mm:ss format.
+    case 'W':  // Remaining time (ETA) in [h:]mm:ss format.
+    case 'P':  // The percentage (in ppp% format) of time elapsed out of
+               // predicted total runtime.
+    case '%':  // A plain `%` character.
+      break;
+
+    default:
+      if (error_output) {
+        char buffer[64];
+        snprintf(buffer, sizeof(buffer),
+                 "unknown placeholder '%%%c' in $NINJA_STATUS", *s);
+        *error_output = buffer;
+      }
+      return false;
+    }
+  }
+  return true;
+}
+
 string StatusPrinter::FormatProgressStatus(const char* progress_status_format,
                                            int64_t time_millis) const {
   string out;
@@ -390,7 +438,6 @@ string StatusPrinter::FormatProgressStatus(const char* progress_status_format,
       }
 
       default:
-        Fatal("unknown placeholder '%%%c' in $NINJA_STATUS", *s);
         return "";
       }
     } else {
@@ -437,21 +484,21 @@ void StatusPrinter::PrintStatus(const Edge* edge, int64_t time_millis) {
                  force_full_command ? LinePrinter::FULL : LinePrinter::ELIDE);
 }
 
-void StatusPrinter::Warning(const char* msg, ...) {
+void StatusPrinter::Warning(const char* msg, ...) const {
   va_list ap;
   va_start(ap, msg);
   ::Warning(msg, ap);
   va_end(ap);
 }
 
-void StatusPrinter::Error(const char* msg, ...) {
+void StatusPrinter::Error(const char* msg, ...) const {
   va_list ap;
   va_start(ap, msg);
   ::Error(msg, ap);
   va_end(ap);
 }
 
-void StatusPrinter::Info(const char* msg, ...) {
+void StatusPrinter::Info(const char* msg, ...) const {
   va_list ap;
   va_start(ap, msg);
   ::Info(msg, ap);

src/status_printer.h

@@ -36,9 +36,18 @@ struct StatusPrinter : Status {
   void BuildStarted() override;
   void BuildFinished() override;
 
-  void Info(const char* msg, ...) override;
-  void Warning(const char* msg, ...) override;
-  void Error(const char* msg, ...) override;
+  void Info(const char* msg, ...) const override;
+  void Warning(const char* msg, ...) const override;
+  void Error(const char* msg, ...) const override;
+
+  /// Validate the given status format string to verify it only contains valid
+  /// escape sequences. On failure, an error message is copied to error_output
+  /// if it isn't null.
+  /// @param progress_status_format The format of the progress status.
+  /// @param error_output Where to write the error message on failure.
+  /// @return Whether the string is a valid progress status format.
+  static bool IsValidProgressStatus(const char* progress_status_format,
+                                    std::string* error_output = nullptr);
 
   /// Format the progress status string by replacing the placeholders.
   /// See the user manual for more information about the available
@@ -91,7 +100,7 @@ struct StatusPrinter : Status {
   Explanations* explanations_ = nullptr;
 
   /// The custom progress status format to use.
-  const char* progress_status_format_;
+  const char* progress_status_format_ = nullptr;
 
   template <size_t S>
   void SnprintfRate(double rate, char (&buf)[S], const char* format) const {

src/status_printer_test.cc

@@ -0,0 +1,70 @@
+// Copyright 2024 Google Inc. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "status_printer.h"
+
+#include "build.h"
+#include "test.h"
+
+struct StatusPrinterTest : public testing::Test {
+  StatusPrinterTest() : config_(MakeConfig()), status_(config_) {}
+
+  virtual void SetUp() { status_.BuildStarted(); }
+
+  static BuildConfig MakeConfig() {
+    BuildConfig config;
+    config.verbosity = BuildConfig::QUIET;
+    return config;
+  }
+
+  BuildConfig config_;
+  StatusPrinter status_;
+};
+
+TEST_F(StatusPrinterTest, StatusFormatElapsed_e) {
+  // Before any task is done, the elapsed time must be zero.
+  EXPECT_EQ("[%/e0.000]", status_.FormatProgressStatus("[%%/e%e]", 0));
+}
+
+TEST_F(StatusPrinterTest, StatusFormatElapsed_w) {
+  // Before any task is done, the elapsed time must be zero.
+  EXPECT_EQ("[%/e00:00]", status_.FormatProgressStatus("[%%/e%w]", 0));
+}
+
+TEST_F(StatusPrinterTest, StatusFormatETA) {
+  // Before any task is done, the ETA time must be unknown.
+  EXPECT_EQ("[%/E?]", status_.FormatProgressStatus("[%%/E%E]", 0));
+}
+
+TEST_F(StatusPrinterTest, StatusFormatTimeProgress) {
+  // Before any task is done, the percentage of elapsed time must be zero.
+  EXPECT_EQ("[%/p  0%]", status_.FormatProgressStatus("[%%/p%p]", 0));
+}
+
+TEST_F(StatusPrinterTest, StatusFormatReplacePlaceholder) {
+  EXPECT_EQ("[%/s0/t0/r0/u0/f0]",
+            status_.FormatProgressStatus("[%%/s%s/t%t/r%r/u%u/f%f]", 0));
+}
+
+TEST_F(StatusPrinterTest, StatusFormatValidator) {
+  EXPECT_TRUE(StatusPrinter::IsValidProgressStatus("[%f/%t] "));
+  {
+    std::string error_output;
+    EXPECT_FALSE(
+        StatusPrinter::IsValidProgressStatus("[%f/%X] ", &error_output));
+    EXPECT_EQ("unknown placeholder '%X' in $NINJA_STATUS", error_output);
+  }
+
+  EXPECT_EQ("", status_.FormatProgressStatus("[%f/%X] ", 0));
+}