pw_string: redo pw::string::Length
Moves and renames the existing pw::string::Length to
pw::string::internal::ClampedLength. Instead two new helpers are added:
1) pw::string::NullTerminatedLength(...) which returns the length IFF
the string is null terminated.
2) pw::string::ClampedCString(...) which returns a string_view of
the clamped string. This is considered safer compared to strnlen_s
and the existing internal::ClampedLength implementation, since
sting_view does not require null termination.
Change-Id: Ie6486df13b205332633f3970109ca97f578e6993
Requires: pigweed-internal:12460
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/43463
Reviewed-by: Ewout van Bekkum <ewout@google.com>
Reviewed-by: Wyatt Hepler <hepler@google.com>
Reviewed-by: Keir Mierle <keir@google.com>
Pigweed-Auto-Submit: Ewout van Bekkum <ewout@google.com>
Commit-Queue: Auto-Submit <auto-submit@pigweed.google.com.iam.gserviceaccount.com>
diff --git a/pw_assert_basic/BUILD.gn b/pw_assert_basic/BUILD.gn
index 34125d9..b0a82da 100644
--- a/pw_assert_basic/BUILD.gn
+++ b/pw_assert_basic/BUILD.gn
@@ -35,6 +35,7 @@
# TODO(pwbug/372): Reorganize this.
group("pw_assert_basic.impl") {
public_deps = [
+ dir_pw_result,
dir_pw_string,
dir_pw_sys_io,
]
@@ -72,7 +73,10 @@
check_includes = false
# Depend on the include path instead of the library to avoid circular deps.
- configs = [ "$dir_pw_string:public_include_path" ]
+ configs = [
+ "$dir_pw_string:public_include_path",
+ "$dir_pw_result:public_include_path",
+ ]
deps = [
":handler.facade",
"$dir_pw_assert:facade",
diff --git a/pw_fuzzer/BUILD.gn b/pw_fuzzer/BUILD.gn
index 1359db4..91cbd08 100644
--- a/pw_fuzzer/BUILD.gn
+++ b/pw_fuzzer/BUILD.gn
@@ -80,7 +80,10 @@
# Sample fuzzer
pw_fuzzer("toy_fuzzer") {
sources = [ "examples/toy_fuzzer.cc" ]
- deps = [ "$dir_pw_string" ]
+ deps = [
+ "$dir_pw_result",
+ "$dir_pw_string",
+ ]
}
pw_test_group("tests") {
diff --git a/pw_fuzzer/examples/toy_fuzzer.cc b/pw_fuzzer/examples/toy_fuzzer.cc
index bcbaef1..58ae461 100644
--- a/pw_fuzzer/examples/toy_fuzzer.cc
+++ b/pw_fuzzer/examples/toy_fuzzer.cc
@@ -22,7 +22,9 @@
#include <cstddef>
#include <cstdint>
#include <cstring>
+#include <span>
+#include "pw_result/result.h"
#include "pw_string/util.h"
namespace {
@@ -62,24 +64,26 @@
// The fuzz target function
extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
// We want to split our input into two strings.
- const char* word1 = reinterpret_cast<const char*>(data);
+ const std::span<const char> input(reinterpret_cast<const char*>(data), size);
// If that's not feasible, toss this input. The fuzzer will quickly learn that
// inputs without null-terminators are uninteresting.
- size_t offset = pw::string::Length(word1, size) + 1;
- if (offset >= size) {
+ const pw::Result<size_t> possible_word1_size =
+ pw::string::NullTerminatedLength(input);
+ if (!possible_word1_size.ok()) {
return 0;
}
+ const std::span<const char> word1 =
+ input.first(possible_word1_size.value() + 1);
// Actually, inputs without TWO null terminators are uninteresting.
- const char* word2 = reinterpret_cast<const char*>(&data[offset]);
- size -= offset;
- if (pw::string::Length(word2, size) == size) {
+ std::span<const char> remaining_input = input.subspan(word1.size());
+ if (!pw::string::NullTerminatedLength(remaining_input).ok()) {
return 0;
}
// Call the code we're targeting!
- toy_example(word1, word2);
+ toy_example(word1.data(), remaining_input.data());
// By convention, the fuzzer always returns zero.
return 0;
diff --git a/pw_kvs/public/pw_kvs/key.h b/pw_kvs/public/pw_kvs/key.h
index 62b8532..f449f05 100644
--- a/pw_kvs/public/pw_kvs/key.h
+++ b/pw_kvs/public/pw_kvs/key.h
@@ -21,7 +21,7 @@
#include <string_view>
#endif // __cplusplus >= 201703L
-#include "pw_string/util.h"
+#include "pw_string/internal/length.h"
namespace pw {
namespace kvs {
@@ -35,7 +35,8 @@
constexpr Key(const Key&) = default;
constexpr Key(const char* str)
: str_{str},
- length_{string::Length(str, std::numeric_limits<size_t>::max())} {}
+ length_{string::internal::ClampedLength(
+ str, std::numeric_limits<size_t>::max())} {}
constexpr Key(const char* str, size_t len) : str_{str}, length_{len} {}
Key(const std::string& str) : str_{str.data()}, length_{str.length()} {}
diff --git a/pw_result/BUILD.gn b/pw_result/BUILD.gn
index 7192cbf..aa09d75 100644
--- a/pw_result/BUILD.gn
+++ b/pw_result/BUILD.gn
@@ -19,12 +19,12 @@
import("$dir_pw_docgen/docs.gni")
import("$dir_pw_unit_test/test.gni")
-config("default_config") {
+config("public_include_path") {
include_dirs = [ "public" ]
}
pw_source_set("pw_result") {
- public_configs = [ ":default_config" ]
+ public_configs = [ ":public_include_path" ]
public_deps = [
"$dir_pw_assert",
"$dir_pw_preprocessor",
diff --git a/pw_string/BUILD b/pw_string/BUILD
index 572a16d..29d04b4 100644
--- a/pw_string/BUILD
+++ b/pw_string/BUILD
@@ -31,6 +31,7 @@
],
hdrs = [
"public/pw_string/format.h",
+ "public/pw_string/internal/length.h",
"public/pw_string/string_builder.h",
"public/pw_string/to_string.h",
"public/pw_string/type_to_string.h",
@@ -42,6 +43,7 @@
"//pw_span",
"//pw_assert",
"//pw_status",
+ "//pw_result",
],
)
diff --git a/pw_string/BUILD.gn b/pw_string/BUILD.gn
index caba610..0742938 100644
--- a/pw_string/BUILD.gn
+++ b/pw_string/BUILD.gn
@@ -27,6 +27,7 @@
public_configs = [ ":public_include_path" ]
public = [
"public/pw_string/format.h",
+ "public/pw_string/internal/length.h",
"public/pw_string/string_builder.h",
"public/pw_string/to_string.h",
"public/pw_string/type_to_string.h",
@@ -40,6 +41,7 @@
public_deps = [
"$dir_pw_assert",
"$dir_pw_preprocessor",
+ "$dir_pw_result",
"$dir_pw_status",
]
}
diff --git a/pw_string/CMakeLists.txt b/pw_string/CMakeLists.txt
index 8b66bc0..a4d12d3 100644
--- a/pw_string/CMakeLists.txt
+++ b/pw_string/CMakeLists.txt
@@ -18,6 +18,7 @@
PUBLIC_DEPS
pw_assert
pw_preprocessor
+ pw_result
pw_span
pw_status
)
diff --git a/pw_string/docs.rst b/pw_string/docs.rst
index 07028df..5bb10f0 100644
--- a/pw_string/docs.rst
+++ b/pw_string/docs.rst
@@ -34,16 +34,41 @@
.. include:: format_size_report
-pw::string::Length
-==================
-The ``pw::string::Length`` function provides a safer alternative to
-``std::strlen`` in case a string is extremely long and/or potentially not
-null-terminated. It is a constexpr version of C11's ``strnlen_s``.
+Safe Length Checking
+====================
+This module provides two safer alternatives to ``std::strlen`` in case the
+string is extremely long and/or potentially not null-terminated.
-.. cpp:function:: constexpr size_t Length(const char* str, size_t max_len)
+First, a constexpr alternative to C11's ``strnlen_s`` is offerred through
+:cpp:func:`pw::string::ClampedCString`. This does not return a length by
+design and instead returns a string_view which does not require
+null-termination.
- Calculates the length of a null-terminated string up to the specified maximum
- length. If str is nullptr, returns 0.
+Second, a constexpr specialized form is offered where null termination is
+required through :cpp:func:`pw::string::NullTerminatedLength`. This will only
+return a length if the string is null-terminated.
+
+.. cpp:function:: constexpr std::string_view pw::string::ClampedCString(std::span<const char> str)
+.. cpp:function:: constexpr std::string_view pw::string::ClampedCString(const char* str, size_t max_len)
+
+ Safe alternative to the string_view constructor to avoid the risk of an
+ unbounded implicit or explicit use of strlen.
+
+ This is strongly recommended over using something like C11's strnlen_s as
+ a string_view does not require null-termination.
+
+.. cpp:function:: constexpr pw::Result<size_t> pw::string::NullTerminatedLength(std::span<const char> str)
+.. cpp:function:: pw::Result<size_t> pw::string::NullTerminatedLength(const char* str, size_t max_len)
+
+ Safe alternative to strlen to calculate the null-terminated length of the
+ string within the specified span, excluding the null terminator. Like C11's
+ strnlen_s, the scan for the null-terminator is bounded.
+
+ Returns:
+ null-terminated length of the string excluding the null terminator.
+ OutOfRange - if the string is not null-terminated.
+
+ Precondition: The string shall be at a valid pointer.
pw::string::Copy
================
diff --git a/pw_string/public/pw_string/internal/length.h b/pw_string/public/pw_string/internal/length.h
new file mode 100644
index 0000000..a338517
--- /dev/null
+++ b/pw_string/public/pw_string/internal/length.h
@@ -0,0 +1,46 @@
+// Copyright 2021 The Pigweed Authors
+//
+// 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
+//
+// https://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.
+#pragma once
+
+#include <cstddef>
+
+namespace pw {
+namespace string {
+namespace internal {
+
+// Calculates the length of a null-terminated string up to the specified maximum
+// length. If str is nullptr, returns 0.
+//
+// This function is a constexpr version of C11's strnlen_s.
+//
+// WARNING: this will return a length even if the string is NOT null-terminated!
+// ClampedCString and NullTerminatedLength are recommended which are built on
+// top of this.
+constexpr size_t ClampedLength(const char* str, size_t max_len) {
+ size_t length = 0;
+
+ if (str != nullptr) {
+ for (; length < max_len; ++length) {
+ if (str[length] == '\0') {
+ break;
+ }
+ }
+ }
+
+ return length;
+}
+
+} // namespace internal
+} // namespace string
+} // namespace pw
diff --git a/pw_string/public/pw_string/util.h b/pw_string/public/pw_string/util.h
index 1909401..6b58308 100644
--- a/pw_string/public/pw_string/util.h
+++ b/pw_string/public/pw_string/util.h
@@ -18,30 +18,53 @@
#include <string_view>
#include "pw_assert/assert.h"
+#include "pw_result/result.h"
#include "pw_status/status.h"
#include "pw_status/status_with_size.h"
+#include "pw_string/internal/length.h"
namespace pw {
namespace string {
-// Calculates the length of a null-terminated string up to the specified maximum
-// length. If str is nullptr, returns 0.
+// Safe alternative to the string_view constructor to avoid the risk of an
+// unbounded implicit or explicit use of strlen.
//
-// This function is a constexpr version of C11's strnlen_s.
-constexpr size_t Length(const char* str, size_t max_len) {
- size_t length = 0;
+// This is strongly recommended over using something like C11's strnlen_s as
+// a string_view does not require null-termination.
+constexpr std::string_view ClampedCString(std::span<const char> str) {
+ return std::string_view(str.data(),
+ internal::ClampedLength(str.data(), str.size()));
+}
- if (str != nullptr) {
- for (; length < max_len; ++length) {
- if (str[length] == '\0') {
- break;
- }
- }
+constexpr std::string_view ClampedCString(const char* str, size_t max_len) {
+ return ClampedCString(std::span<const char>(str, max_len));
+}
+
+// Safe alternative to strlen to calculate the null-terminated length of the
+// string within the specified span, excluding the null terminator. Like C11's
+// strnlen_s, the scan for the null-terminator is bounded.
+//
+// Returns:
+// null-terminated length of the string excluding the null terminator.
+// OutOfRange - if the string is not null-terminated.
+//
+// Precondition: The string shall be at a valid pointer.
+constexpr pw::Result<size_t> NullTerminatedLength(std::span<const char> str) {
+ PW_DASSERT(str.data() != nullptr);
+
+ const size_t length = internal::ClampedLength(str.data(), str.size());
+ if (length == str.size()) {
+ return Status::OutOfRange();
}
return length;
}
+constexpr pw::Result<size_t> NullTerminatedLength(const char* str,
+ size_t max_len) {
+ return NullTerminatedLength(std::span<const char>(str, max_len));
+}
+
// Copies the source string to the dest, truncating if the full string does not
// fit. Always null terminates if dest.size() or num > 0.
//
@@ -66,7 +89,7 @@
constexpr StatusWithSize Copy(const char* source, std::span<char> dest) {
PW_DASSERT(source != nullptr);
- return Copy(std::string_view(source, Length(source, dest.size())), dest);
+ return Copy(ClampedCString(source, dest.size()), dest);
}
constexpr StatusWithSize Copy(const char* source, char* dest, size_t num) {
diff --git a/pw_string/string_builder.cc b/pw_string/string_builder.cc
index 60879a9..7eff7d2 100644
--- a/pw_string/string_builder.cc
+++ b/pw_string/string_builder.cc
@@ -44,7 +44,7 @@
// Use buffer_.size() - size() as the maximum length so that strings too long
// to fit in the buffer will request one character too many, which sets the
// status to RESOURCE_EXHAUSTED.
- return append(str, string::Length(str, buffer_.size() - size()));
+ return append(string::ClampedCString(str, buffer_.size() - size()));
}
StringBuilder& StringBuilder::append(const std::string_view& str) {
diff --git a/pw_string/util_test.cc b/pw_string/util_test.cc
index 86c8562..b22749c 100644
--- a/pw_string/util_test.cc
+++ b/pw_string/util_test.cc
@@ -19,23 +19,85 @@
namespace pw::string {
namespace {
-TEST(Length, Nullptr_Returns0) { EXPECT_EQ(0u, Length(nullptr, 100)); }
+using namespace std::literals::string_view_literals;
-TEST(Length, EmptyString_Returns0) {
- EXPECT_EQ(0u, Length("", 0));
- EXPECT_EQ(0u, Length("", 100));
+TEST(ClampedLength, Nullptr_Returns0) {
+ EXPECT_EQ(0u, internal::ClampedLength(nullptr, 100));
}
-TEST(Length, MaxLongerThanString_ReturnsStrlen) {
- EXPECT_EQ(5u, Length("12345", 100));
+TEST(ClampedLength, EmptyString_Returns0) {
+ EXPECT_EQ(0u, internal::ClampedLength("", 0));
+ EXPECT_EQ(0u, internal::ClampedLength("", 100));
}
-TEST(Length, StringMaxLongerThanMax_ReturnsMax) {
- EXPECT_EQ(0u, Length("12345", 0));
- EXPECT_EQ(4u, Length("12345", 4));
+TEST(ClampedLength, MaxLongerThanString_ReturnsStrlen) {
+ EXPECT_EQ(5u, internal::ClampedLength("12345", 100));
}
-TEST(Length, LengthEqualsMax) { EXPECT_EQ(5u, Length("12345", 5)); }
+TEST(ClampedLength, StringMaxLongerThanMax_ReturnsMax) {
+ EXPECT_EQ(0u, internal::ClampedLength("12345", 0));
+ EXPECT_EQ(4u, internal::ClampedLength("12345", 4));
+}
+
+TEST(ClampedLength, LengthEqualsMax) {
+ EXPECT_EQ(5u, internal::ClampedLength("12345", 5));
+}
+
+TEST(ClampedCString, NullPtr_ReturnsEmpty) {
+ EXPECT_TRUE(ClampedCString(nullptr, 100).empty());
+}
+
+TEST(ClampedCString, EmptyString_Returns0) {
+ EXPECT_TRUE(ClampedCString("", 0).empty());
+ EXPECT_TRUE(ClampedCString("", 100).empty());
+}
+
+TEST(ClampedCString, MaxLongerThanString_ReturnsStr) {
+ static constexpr char kInput[] = "12345";
+ const std::string_view result = ClampedCString(kInput, 100);
+ EXPECT_EQ(result.size(), strlen(kInput));
+ EXPECT_EQ(result.data(), &kInput[0]);
+}
+
+TEST(ClampedCString, StringMaxLongerThanMax_ClampsView) {
+ static constexpr char kInput[] = "12345";
+
+ EXPECT_TRUE(ClampedCString(kInput, 0).empty());
+
+ const std::string_view result = ClampedCString(kInput, 4);
+ EXPECT_EQ(result.size(), 4u);
+ EXPECT_EQ(result.data(), &kInput[0]);
+}
+
+TEST(ClampedCString, FullStringView) {
+ static constexpr char kInput[] = "12345";
+ const std::string_view result = ClampedCString(kInput);
+ EXPECT_EQ(result.size(), strlen(kInput));
+ EXPECT_EQ(result.data(), &kInput[0]);
+}
+
+TEST(NullTerminatedLength, EmptyString_RequiresNullTerminator) {
+ EXPECT_TRUE(NullTerminatedLength("", 0).status().IsOutOfRange());
+
+ ASSERT_TRUE(NullTerminatedLength("", 100).status().ok());
+ EXPECT_EQ(0u, NullTerminatedLength("", 100).value());
+}
+
+TEST(NullTerminatedLength, MaxLongerThanString_ReturnsStrlen) {
+ ASSERT_TRUE(NullTerminatedLength("12345", 100).status().ok());
+ EXPECT_EQ(5u, NullTerminatedLength("12345", 100).value());
+}
+
+TEST(NullTerminatedLength, StringMaxLongerThanMax_Fails) {
+ EXPECT_TRUE(NullTerminatedLength("12345", 0).status().IsOutOfRange());
+ EXPECT_TRUE(NullTerminatedLength("12345", 4).status().IsOutOfRange());
+}
+
+TEST(NullTerminatedLength, LengthEqualsMax) {
+ static constexpr char kInput[] = "12345";
+ ASSERT_TRUE(NullTerminatedLength(kInput).ok());
+ EXPECT_EQ(5u, NullTerminatedLength(kInput).value());
+}
class TestWithBuffer : public ::testing::Test {
protected:
@@ -48,8 +110,6 @@
class CopyTest : public TestWithBuffer {};
-using namespace std::literals::string_view_literals;
-
TEST_F(CopyTest, EmptyStringView_WritesNullTerminator) {
EXPECT_EQ(0u, Copy("", buffer_).size());
EXPECT_EQ('\0', buffer_[0]);
