fix(base): an Option<const char*> owns the string it is given

base/option.h

@@ -15,13 +15,15 @@
 #include <base/meta/traits.h>
 #include <base/strings/format.h>
 #include <base/strings/number_parse.h>
+#include <base/strings/xstring.h>
 
 namespace base {
 namespace detail {
 
 // Parse `text` into `out`. Returns false (leaving `out` untouched) when the
-// text is not a valid value for T. Covers bool, the integral and floating
-// types, and const char* (which is bound to the source string, not copied).
+// text is not a valid value for T. Covers bool and the integral and floating
+// types; a const char* option does not come through here, because it has to
+// own what it is given (Option::ParseFromString).
 template <typename T>
 inline bool ParseOption(const char* text, T& out) {
   if constexpr (base::is_same_v<T, bool>) {
@@ -44,9 +46,6 @@ inline bool ParseOption(const char* text, T& out) {
       return true;
     }
     return false;
-  } else if constexpr (base::is_same_v<T, const char*>) {
-    out = text;
-    return true;
   } else if constexpr (base::is_integral_v<T>) {
     i64 v = 0;
     if (!base::ParseInteger(text, v, /*base_radix=*/0)) return false;
@@ -124,6 +123,18 @@ class BASE_EXPORT OptionBase : public InitChain<OptionBase> {
 // programmatic default. Declare options at namespace scope, not as
 // function-local statics: InitOptionsFromEnv() can only populate options that
 // were already constructed (and thus registered) by the time it runs.
+namespace detail {
+// The buffer a const char* option keeps its value in. Empty for every other T,
+// and empty bases/members cost nothing here, so an Option<int> is the same
+// size it always was.
+template <typename T>
+struct OwnedText {};
+template <>
+struct OwnedText<const char*> {
+  StringU8 text;
+};
+}  // namespace detail
+
 template <typename T>
 class Option : public OptionBase {
  public:
@@ -149,14 +160,32 @@ class Option : public OptionBase {
 
  protected:
   bool ParseFromString(const char* text) override {
-    return detail::ParseOption(text, value_);
+    if constexpr (base::is_same_v<T, const char*>) {
+      // A string option owns its value. What SetFromString is handed is
+      // whatever its caller had at the time -- an environment variable read
+      // into a local, one line of a config file, a name off a command line --
+      // and none of those are still there when the option is next read, so
+      // pointing at the caller's buffer is a use-after-free waiting for the
+      // first reader.
+      if (!text) return false;
+      owned_.text = reinterpret_cast<const char8_t*>(text);
+      value_ = reinterpret_cast<const char*>(owned_.text.c_str());
+      return true;
+    } else {
+      return detail::ParseOption(text, value_);
+    }
   }
 
-  void RestoreDefault() override { value_ = default_; }
+  void RestoreDefault() override {
+    value_ = default_;
+    // The default is a literal the option does not own; drop what it did.
+    if constexpr (base::is_same_v<T, const char*>) owned_.text.clear();
+  }
 
  private:
   T value_;
   const T default_;
+  [[no_unique_address]] detail::OwnedText<T> owned_;
 };
 
 // Populate every registered option that names an environment variable from the

base/option_test.cc

@@ -0,0 +1,86 @@
+// Copyright (C) 2026 Vincent Hengel.
+// For licensing information see LICENSE at the root of this distribution.
+
+#include <base/option.h>
+#include <base/environment_variables.h>
+#include <base/strings/xstring.h>
+
+#include <gtest/gtest.h>
+
+namespace {
+
+base::Option<bool> kFlag{"opt.flag", false, "BASE_OPT_FLAG", "a boolean"};
+base::Option<int> kCount{"opt.count", 7, "BASE_OPT_COUNT", "a number"};
+base::Option<float> kScale{"opt.scale", 1.0f, "BASE_OPT_SCALE"};
+base::Option<const char*> kName{"opt.name", "default", "BASE_OPT_NAME"};
+
+class OptionTest : public ::testing::Test {
+ protected:
+  void SetUp() override { TearDown(); }
+  void TearDown() override {
+    kFlag.Reset();
+    kCount.Reset();
+    kScale.Reset();
+    kName.Reset();
+    base::DeleteEnvironmentVariable(u8"BASE_OPT_FLAG");
+    base::DeleteEnvironmentVariable(u8"BASE_OPT_COUNT");
+    base::DeleteEnvironmentVariable(u8"BASE_OPT_SCALE");
+    base::DeleteEnvironmentVariable(u8"BASE_OPT_NAME");
+  }
+};
+
+TEST_F(OptionTest, ParsesEachType) {
+  EXPECT_TRUE(kFlag.SetFromString("on"));
+  EXPECT_TRUE(kCount.SetFromString("128"));
+  EXPECT_TRUE(kScale.SetFromString("0.25"));
+  EXPECT_TRUE(kFlag.get());
+  EXPECT_EQ(kCount.get(), 128);
+  EXPECT_FLOAT_EQ(kScale.get(), 0.25f);
+}
+
+// The whole reason a string option copies: SetFromString is handed whatever
+// its caller had at the time, and the option is read long after that is gone.
+TEST_F(OptionTest, StringValueOutlivesTheCallersBuffer) {
+  {
+    base::String text("from-a-temporary");
+    ASSERT_TRUE(kName.SetFromString(text.c_str()));
+  }
+  EXPECT_STREQ(kName.get(), "from-a-temporary");
+}
+
+// InitOptionsFromEnv reads each variable into a local, which is the same trap
+// one call further out.
+TEST_F(OptionTest, EnvironmentOverrideOutlivesTheRead) {
+  ASSERT_TRUE(base::SetEnvironmentVariable(u8"BASE_OPT_NAME", u8"from-the-env"));
+  ASSERT_TRUE(base::SetEnvironmentVariable(u8"BASE_OPT_COUNT", u8"42"));
+
+  EXPECT_GE(base::InitOptionsFromEnv(), 2u);
+  EXPECT_STREQ(kName.get(), "from-the-env");
+  EXPECT_EQ(kCount.get(), 42);
+  EXPECT_TRUE(kName.overridden());
+}
+
+TEST_F(OptionTest, ResetDropsTheOwnedString) {
+  {
+    base::String text("temporary");
+    ASSERT_TRUE(kName.SetFromString(text.c_str()));
+  }
+  kName.Reset();
+  EXPECT_STREQ(kName.get(), "default");
+  EXPECT_FALSE(kName.overridden());
+}
+
+TEST_F(OptionTest, RejectsWhatDoesNotParse) {
+  ASSERT_TRUE(kCount.SetFromString("9"));
+  EXPECT_FALSE(kCount.SetFromString("not-a-number"));
+  EXPECT_EQ(kCount.get(), 9);  // a bad value leaves the old one alone
+  EXPECT_FALSE(kName.SetFromString(nullptr));
+}
+
+// A string option pays for its buffer; nothing else should.
+TEST_F(OptionTest, NonStringOptionsCarryNoStorage) {
+  EXPECT_EQ(sizeof(base::Option<int>), sizeof(base::Option<unsigned>));
+  EXPECT_LT(sizeof(base::Option<int>), sizeof(base::Option<const char*>));
+}
+
+}  // namespace