https://github.com/dingcyrus created https://github.com/llvm/llvm-project/pull/212949
The isUTF8() → fixUTF8() pattern is repeated across the codebase because json::Value(StringRef) and json::ObjectKey(StringRef) assert on invalid input in debug builds. This makes callers working with untrusted strings (e.g. source comments) duplicate the same check-and-fix logic. Add json::safeUTF8(StringRef) which returns a zero-copy Value for valid UTF-8 and a fixed (U+FFFD-substituted) Value for invalid input. The utility consolidates what was previously open-coded in several places: - clang-doc had a local safeJSONString() helper (removed in this patch) - SymbolGraphSerializer used an inline isUTF8 ? s : fixUTF8(s) ternary - lldb duplicated the pattern in several EmplaceSafeString wrappers This is an NFC follow-up to PR #210886. >From c46e8e5324fb44fdf748b54e5a62a23217f532d2 Mon Sep 17 00:00:00 2001 From: Chenguang Ding <[email protected]> Date: Thu, 30 Jul 2026 15:50:34 +0800 Subject: [PATCH] [Support][JSON] Add safeUTF8() helper to sanitize UTF-8 for JSON values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The isUTF8() → fixUTF8() pattern is repeated across the codebase because json::Value(StringRef) and json::ObjectKey(StringRef) assert on invalid input in debug builds. This makes callers working with untrusted strings (e.g. source comments) duplicate the same check-and-fix logic. Add json::safeUTF8(StringRef) which returns a zero-copy Value for valid UTF-8 and a fixed (U+FFFD-substituted) Value for invalid input. The utility consolidates what was previously open-coded in several places: - clang-doc had a local safeJSONString() helper (removed in this patch) - SymbolGraphSerializer used an inline isUTF8 ? s : fixUTF8(s) ternary - lldb duplicated the pattern in several EmplaceSafeString wrappers This is an NFC follow-up to PR #210886. --- clang-tools-extra/clang-doc/JSONGenerator.cpp | 14 ++---- .../Serialization/SymbolGraphSerializer.cpp | 4 +- llvm/include/llvm/Support/JSON.h | 8 ++++ llvm/lib/Support/JSON.cpp | 6 +++ llvm/unittests/Support/JSONTest.cpp | 47 +++++++++++++++++++ 5 files changed, 66 insertions(+), 13 deletions(-) diff --git a/clang-tools-extra/clang-doc/JSONGenerator.cpp b/clang-tools-extra/clang-doc/JSONGenerator.cpp index b4284f8eb4d14..842b17944f937 100644 --- a/clang-tools-extra/clang-doc/JSONGenerator.cpp +++ b/clang-tools-extra/clang-doc/JSONGenerator.cpp @@ -106,12 +106,6 @@ static void insertNonEmpty(StringRef Key, StringRef Value, Object &Obj) { Obj[Key] = Value; } -static json::Value safeJSONString(StringRef S) { - if (LLVM_LIKELY(json::isUTF8(S))) - return S; - return json::fixUTF8(S); -} - static std::string infoTypeToString(InfoType IT) { switch (IT) { case InfoType::IT_default: @@ -239,7 +233,7 @@ static Object serializeComment(const CommentInfo &I, Object &Description) { switch (I.Kind) { case CommentKind::CK_TextComment: { if (!I.Text.empty()) - Obj.insert({commentKindToString(I.Kind), safeJSONString(I.Text)}); + Obj.insert({commentKindToString(I.Kind), json::safeUTF8(I.Text)}); return Obj; } @@ -264,7 +258,7 @@ static Object serializeComment(const CommentInfo &I, Object &Description) { auto &ARef = *ArgsArr.getAsArray(); ARef.reserve(I.Args.size()); for (const auto &Arg : I.Args) - ARef.emplace_back(safeJSONString(Arg)); + ARef.emplace_back(json::safeUTF8(Arg)); Child.insert({"Command", I.Name}); Child.insert({"Args", ArgsArr}); Child.insert({"Children", ChildArr}); @@ -298,7 +292,7 @@ static Object serializeComment(const CommentInfo &I, Object &Description) { case CommentKind::CK_VerbatimBlockLineComment: case CommentKind::CK_VerbatimLineComment: { - Child.insert({"Text", safeJSONString(I.Text)}); + Child.insert({"Text", json::safeUTF8(I.Text)}); Child.insert({"Children", ChildArr}); Obj.insert({commentKindToString(I.Kind), ChildVal}); return Obj; @@ -339,7 +333,7 @@ static Object serializeComment(const CommentInfo &I, Object &Description) { } case CommentKind::CK_Unknown: { - Obj.insert({commentKindToString(I.Kind), safeJSONString(I.Text)}); + Obj.insert({commentKindToString(I.Kind), json::safeUTF8(I.Text)}); return Obj; } } diff --git a/clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp b/clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp index 6228f2e687be2..2e44235963cfc 100644 --- a/clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp +++ b/clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp @@ -264,9 +264,7 @@ std::optional<Object> serializeDocComment(const DocComment &Comment) { Object Line; // Comments in source files may contain invalid UTF-8. JSON values must be // valid UTF-8, so replace any invalid sequences before serializing. - Line["text"] = json::isUTF8(CommentLine.Text) - ? CommentLine.Text - : json::fixUTF8(CommentLine.Text); + Line["text"] = json::safeUTF8(CommentLine.Text); serializeObject(Line, "range", serializeSourceRange(CommentLine.Begin, CommentLine.End)); LinesArray.emplace_back(std::move(Line)); diff --git a/llvm/include/llvm/Support/JSON.h b/llvm/include/llvm/Support/JSON.h index 3b145a69c74cb..0d1607f69d044 100644 --- a/llvm/include/llvm/Support/JSON.h +++ b/llvm/include/llvm/Support/JSON.h @@ -1094,6 +1094,14 @@ inline llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Value &V) { OStream(OS).value(V); return OS; } + +/// If \p S is valid UTF-8, returns a Value that references S (zero-copy). +/// Otherwise, replaces invalid sequences with U+FFFD and returns an owned +/// string Value (calls fixUTF8). This is safe to use with untrusted strings +/// without triggering assertions in debug builds. +/// The returned Value may reference S's data, so S must outlive it. +LLVM_ABI Value safeUTF8(StringRef S); + } // namespace json /// Allow printing json::Value with formatv(). diff --git a/llvm/lib/Support/JSON.cpp b/llvm/lib/Support/JSON.cpp index 23c2542d75810..5883b34349bb3 100644 --- a/llvm/lib/Support/JSON.cpp +++ b/llvm/lib/Support/JSON.cpp @@ -726,6 +726,12 @@ std::string fixUTF8(llvm::StringRef S) { return Res; } +Value safeUTF8(StringRef S) { + if (LLVM_LIKELY(isUTF8(S))) + return S; + return fixUTF8(S); +} + static void quote(llvm::raw_ostream &OS, llvm::StringRef S) { OS << '\"'; for (unsigned char C : S) { diff --git a/llvm/unittests/Support/JSONTest.cpp b/llvm/unittests/Support/JSONTest.cpp index 348f5cb04f9db..c39164ee94d16 100644 --- a/llvm/unittests/Support/JSONTest.cpp +++ b/llvm/unittests/Support/JSONTest.cpp @@ -263,6 +263,53 @@ TEST(JSONTest, UTF8) { } } +TEST(JSONTest, SafeUTF8) { + // Valid UTF-8: safeUTF8 returns a Value with the same string content. + for (StringRef Valid : { + "this is ASCII text", + "thïs tëxt häs BMP chäräctërs", + "𐌶𐌰L𐌾𐍈 C𐍈𐌼𐌴𐍃", + "", + }) { + Value V = safeUTF8(Valid); + ASSERT_EQ(V.kind(), Value::String); + EXPECT_EQ(*V.getAsString(), Valid); + } + + // Invalid UTF-8: safeUTF8 returns a Value with U+FFFD substitutions. + for (auto [Invalid, Expected] : + std::vector<std::pair<const char *, const char *>>{ + {"lone trailing \x81\x82 bytes", "lone trailing \xEF\xBF\xBD\xEF\xBF\xBD bytes"}, + {"missing trailing \xD0 bytes", "missing trailing \xEF\xBF\xBD bytes"}, + {"truncated character \xD0", "truncated character \xEF\xBF\xBD"}, + {"not \xC1\x80 the \xE0\x9f\xBF shortest \xF0\x83\x83\x83 encoding", + "not \xEF\xBF\xBD\xEF\xBF\xBD the \xEF\xBF\xBD\xEF\xBF\xBD\xEF\xBF\xBD shortest \xEF\xBF\xBD\xEF\xBF\xBD\xEF\xBF\xBD\xEF\xBF\xBD encoding"}, + {"surrogate \xED\xA0\x80 invalid \xF4\x90\x80\x80", + "surrogate \xEF\xBF\xBD\xEF\xBF\xBD\xEF\xBF\xBD invalid \xEF\xBF\xBD\xEF\xBF\xBD\xEF\xBF\xBD\xEF\xBF\xBD"}}) { + Value V = safeUTF8(Invalid); + ASSERT_EQ(V.kind(), Value::String); + EXPECT_EQ(*V.getAsString(), Expected); + } + + // safeUTF8 must not trigger assertions in debug mode — this is its + // primary reason for existing. (The test above already covers this, + // but we add an explicit regression check for the clang-doc crash case.) + // Byte 0x97 is invalid UTF-8 and triggered: + // assert(false && "Invalid UTF-8 in value used as JSON") + Value V = safeUTF8("\x97"); + ASSERT_EQ(V.kind(), Value::String); + + // Verify safeUTF8 integrates correctly into Object construction. + Object O; + O["valid"] = safeUTF8("no issue"); + O["fixed"] = safeUTF8("bad \xD0 byte"); + std::string S; + llvm::raw_string_ostream OS(S); + OS << Value(std::move(O)); + EXPECT_NE(S.find("no issue"), std::string::npos); + EXPECT_NE(S.find("bad \xEF\xBF\xBD byte"), std::string::npos); +} + TEST(JSONTest, Inspection) { llvm::Expected<Value> Doc = parse(R"( { _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
