Skip to content

Commit 2dd7c5b

Browse files
jkaliasclaude
andcommitted
Rename Utf8 to Unicode and address review findings (#25)
Review follow-up on the non-BMP conversion fix. Behavior is unchanged apart from one error message and the two documentation corrections. - Rename Utf8 to Unicode, since the class also carries the UTF-16 conversions, and give all four methods one naming shape: Utf8FromCodePoints, CodePointsFromUtf8, Utf16FromCodePoints, CodePointsFromUtf16. Files and test suites renamed to match. - Collapse the four duplicated element-wise cast loops in the WideCodec specializations into a single cast_each helper. - static_assert the supported wchar_t widths, so an unsupported width reports a portability limit rather than an incomplete type inside a typedef. - Rename reject to throw_invalid_unicode and mark it [[noreturn]], so call sites read as terminating without consulting the callee. - Derive the surrogate range from the UTF-16 bounds instead of repeating D800/DFFF as a second pair of constants at char32_t width. - Name CESU-8 as the likely cause in the decode error, and document that text written by a pre-fix build where wchar_t is 16 bits no longer reads back. That data was stored ill-formed; because encoding and decoding were wrong in the same way, it only surfaces now. - Correct the README throw guarantee to TEXT values: a BLOB in a MEMBER_TEXT column is skipped like NULL and never decoded. Also note the one platform difference that survives, that wstring counts code units, so L"\U0001F600".size() is 2 where wchar_t is 16 bits and 1 where it is 32. Verified on macOS Clang at C++11 and C++20: 124/124 tests pass in both, clang-format clean on the changed files. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent e0f8f8f commit 2dd7c5b

5 files changed

Lines changed: 113 additions & 93 deletions

File tree

‎README.md‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -144,8 +144,22 @@ extension blocks, and text round-trips unchanged on every supported platform reg
144144
whether `wchar_t` is 16 bits (Windows) or 32 bits (Linux, macOS).
145145

146146
Text that is not valid Unicode is rejected rather than silently altered: converting a
147-
`std::wstring` containing unpaired UTF-16 surrogates, or reading a column whose bytes are not
148-
well-formed UTF-8, throws `std::range_error`.
147+
`std::wstring` containing unpaired UTF-16 surrogates, or reading a TEXT value whose bytes are
148+
not well-formed UTF-8, throws `std::range_error`. A column holding a BLOB where a
149+
`MEMBER_TEXT` field is declared is skipped like a `NULL`, leaving the member untouched, so its
150+
bytes are never decoded.
151+
152+
Note that a `std::wstring` still counts code *units*, not code points, so `size()` and indexing
153+
differ across platforms for text above U+FFFF: `L"\U0001F600".size()` is 2 where `wchar_t` is 16
154+
bits and 1 where it is 32 bits. What round-trips identically is the text itself.
155+
156+
#### Reading databases written before this fix
157+
158+
Earlier versions stored text above U+FFFF as CESU-8 when built where `wchar_t` is 16 bits
159+
(Windows), encoding each surrogate half separately. Reading such a value now throws
160+
`std::range_error` rather than returning it. Because the old encode and decode were wrong in the
161+
same way, that data appeared to round-trip on Windows and was written ill-formed to the file;
162+
databases written on Linux or macOS, and any text within the BMP, are unaffected.
149163

150164
### Nullable fields
151165

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,13 +46,18 @@ namespace sqlite_reflection {
4646
/// overlong encodings, truncated sequences, stray or invalid lead and continuation bytes,
4747
/// surrogate code points encoded directly (CESU-8), unpaired UTF-16 surrogates, and anything
4848
/// above U+10FFFF.
49-
class REFLECTION_EXPORT Utf8 {
49+
///
50+
/// Note that rejecting CESU-8 means text written by a pre-fix build of this library, where
51+
/// wchar_t is 16 bits, no longer reads back. That data was stored ill-formed; because encoding
52+
/// and decoding were wrong in the same way it used to survive a round trip on Windows, so the
53+
/// break surfaces only now. See the text encoding section of README.md.
54+
class REFLECTION_EXPORT Unicode {
5055
public:
5156
/// Encodes Unicode code points as UTF-8
52-
static std::string FromCodePoints(const char32_t* code_points, size_t count);
57+
static std::string Utf8FromCodePoints(const char32_t* code_points, size_t count);
5358

5459
/// Decodes UTF-8 into Unicode code points
55-
static std::u32string ToCodePoints(const char* utf8_string, size_t byte_count);
60+
static std::u32string CodePointsFromUtf8(const char* utf8_string, size_t byte_count);
5661

5762
/// Encodes Unicode code points as UTF-16 code units, using a surrogate pair above U+FFFF
5863
static std::u16string Utf16FromCodePoints(const char32_t* code_points, size_t count);

‎src/string_utilities.cc‎

Lines changed: 28 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -24,61 +24,60 @@
2424

2525
#include <numeric>
2626

27-
#include "internal/utf8.h"
27+
#include "internal/unicode.h"
2828

2929
using namespace sqlite_reflection;
3030

3131
namespace {
32+
// Copies a string of code units into a string of a different unit type, one unit at a time. The
33+
// casts are value-preserving in the directions used below, except for a negative wchar_t on a
34+
// platform where it is signed, which widens to a value above U+10FFFF and is then rejected by
35+
// the validation in Unicode.
36+
template <typename To, typename From>
37+
To cast_each(const From& source) {
38+
To result;
39+
result.reserve(source.size());
40+
for (size_t i = 0; i < source.size(); ++i) {
41+
result.push_back(static_cast<typename To::value_type>(source[i]));
42+
}
43+
return result;
44+
}
45+
3246
// wchar_t holds UTF-16 code units where it is 16 bits wide (Windows) and Unicode code points
3347
// where it is 32 bits wide (Linux, macOS). Selecting the path by width is what keeps text above
3448
// U+FFFF identical across platforms; a single facet cannot serve both, which is the defect this
35-
// replaced. See internal/utf8.h.
49+
// replaced. See internal/unicode.h.
3650
template <size_t WideCharSize>
3751
struct WideCodec;
3852

3953
template <>
4054
struct WideCodec<2> {
4155
static std::u32string ToCodePoints(const std::wstring& wide_string) {
42-
std::u16string code_units;
43-
code_units.reserve(wide_string.size());
44-
for (size_t i = 0; i < wide_string.size(); ++i) {
45-
code_units.push_back(static_cast<char16_t>(wide_string[i]));
46-
}
47-
return Utf8::CodePointsFromUtf16(code_units.data(), code_units.size());
56+
const auto code_units = cast_each<std::u16string>(wide_string);
57+
return Unicode::CodePointsFromUtf16(code_units.data(), code_units.size());
4858
}
4959

5060
static std::wstring FromCodePoints(const std::u32string& code_points) {
51-
const auto code_units = Utf8::Utf16FromCodePoints(code_points.data(), code_points.size());
52-
std::wstring wide_string;
53-
wide_string.reserve(code_units.size());
54-
for (size_t i = 0; i < code_units.size(); ++i) {
55-
wide_string.push_back(static_cast<wchar_t>(code_units[i]));
56-
}
57-
return wide_string;
61+
const auto code_units = Unicode::Utf16FromCodePoints(code_points.data(), code_points.size());
62+
return cast_each<std::wstring>(code_units);
5863
}
5964
};
6065

6166
template <>
6267
struct WideCodec<4> {
6368
static std::u32string ToCodePoints(const std::wstring& wide_string) {
64-
std::u32string code_points;
65-
code_points.reserve(wide_string.size());
66-
for (size_t i = 0; i < wide_string.size(); ++i) {
67-
code_points.push_back(static_cast<char32_t>(wide_string[i]));
68-
}
69-
return code_points;
69+
return cast_each<std::u32string>(wide_string);
7070
}
7171

7272
static std::wstring FromCodePoints(const std::u32string& code_points) {
73-
std::wstring wide_string;
74-
wide_string.reserve(code_points.size());
75-
for (size_t i = 0; i < code_points.size(); ++i) {
76-
wide_string.push_back(static_cast<wchar_t>(code_points[i]));
77-
}
78-
return wide_string;
73+
return cast_each<std::wstring>(code_points);
7974
}
8075
};
8176

77+
// Only the two widths above are implemented; without this the failure is an incomplete-type
78+
// error inside the typedef rather than a statement about portability.
79+
static_assert(sizeof(wchar_t) == 2 || sizeof(wchar_t) == 4, "sqlite_reflection supports wchar_t of 16 or 32 bits only");
80+
8281
typedef WideCodec<sizeof(wchar_t)> PlatformWideCodec;
8382
} // namespace
8483

@@ -98,11 +97,11 @@ std::string StringUtilities::FromDouble(double value) {
9897

9998
std::string StringUtilities::ToUtf8(const std::wstring& wide_string) {
10099
const auto code_points = PlatformWideCodec::ToCodePoints(wide_string);
101-
return Utf8::FromCodePoints(code_points.data(), code_points.size());
100+
return Unicode::Utf8FromCodePoints(code_points.data(), code_points.size());
102101
}
103102

104103
std::wstring StringUtilities::FromUtf8(const char* utf8_string, size_t byte_count) {
105-
const auto code_points = Utf8::ToCodePoints(utf8_string, byte_count);
104+
const auto code_points = Unicode::CodePointsFromUtf8(utf8_string, byte_count);
106105
return PlatformWideCodec::FromCodePoints(code_points);
107106
}
108107

src/utf8.cc renamed to src/unicode.cc

Lines changed: 24 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -20,39 +20,39 @@
2020
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
2121
// SOFTWARE.
2222

23-
#include "internal/utf8.h"
23+
#include "internal/unicode.h"
2424

2525
#include <stdexcept>
2626

2727
using namespace sqlite_reflection;
2828

2929
namespace {
3030
const char32_t kMaxCodePoint = 0x10FFFF;
31-
const char32_t kFirstSurrogate = 0xD800;
32-
const char32_t kLastSurrogate = 0xDFFF;
3331
const char32_t kFirstSupplementary = 0x10000;
3432

3533
const char16_t kFirstHighSurrogate = 0xD800;
3634
const char16_t kLastHighSurrogate = 0xDBFF;
3735
const char16_t kFirstLowSurrogate = 0xDC00;
3836
const char16_t kLastLowSurrogate = 0xDFFF;
3937

40-
void reject(const char* reason) {
38+
[[noreturn]] void throw_invalid_unicode(const char* reason) {
4139
throw std::range_error(std::string("invalid Unicode input: ") + reason);
4240
}
4341

42+
// The surrogate range is exactly the first high through the last low surrogate; the char16_t
43+
// operands widen to char32_t for the comparison.
4444
bool is_surrogate(char32_t code_point) {
45-
return code_point >= kFirstSurrogate && code_point <= kLastSurrogate;
45+
return code_point >= kFirstHighSurrogate && code_point <= kLastLowSurrogate;
4646
}
4747

4848
// A code point that no valid encoding may carry: surrogates are reserved for UTF-16 pairing
4949
// and must never appear on their own, and nothing above U+10FFFF exists.
5050
void reject_if_not_scalar_value(char32_t code_point) {
5151
if (is_surrogate(code_point)) {
52-
reject("surrogate code point");
52+
throw_invalid_unicode("surrogate code point");
5353
}
5454
if (code_point > kMaxCodePoint) {
55-
reject("code point above U+10FFFF");
55+
throw_invalid_unicode("code point above U+10FFFF");
5656
}
5757
}
5858

@@ -83,21 +83,23 @@ size_t sequence_length(unsigned char lead) {
8383
// code point above U+10FFFF. The lead byte alone cannot exclude these (Unicode Table 3-7).
8484
void reject_if_invalid_second_byte(unsigned char lead, unsigned char second) {
8585
if (lead == 0xE0 && second < 0xA0) {
86-
reject("overlong three-byte sequence");
86+
throw_invalid_unicode("overlong three-byte sequence");
8787
}
8888
if (lead == 0xED && second > 0x9F) {
89-
reject("surrogate code point encoded in UTF-8");
89+
throw_invalid_unicode(
90+
"surrogate code point encoded in UTF-8 (CESU-8), as written by a pre-fix build of this "
91+
"library where wchar_t is 16 bits");
9092
}
9193
if (lead == 0xF0 && second < 0x90) {
92-
reject("overlong four-byte sequence");
94+
throw_invalid_unicode("overlong four-byte sequence");
9395
}
9496
if (lead == 0xF4 && second > 0x8F) {
95-
reject("code point above U+10FFFF");
97+
throw_invalid_unicode("code point above U+10FFFF");
9698
}
9799
}
98100
} // namespace
99101

100-
std::string Utf8::FromCodePoints(const char32_t* code_points, size_t count) {
102+
std::string Unicode::Utf8FromCodePoints(const char32_t* code_points, size_t count) {
101103
std::string utf8_string;
102104
utf8_string.reserve(count);
103105

@@ -125,7 +127,7 @@ std::string Utf8::FromCodePoints(const char32_t* code_points, size_t count) {
125127
return utf8_string;
126128
}
127129

128-
std::u32string Utf8::ToCodePoints(const char* utf8_string, size_t byte_count) {
130+
std::u32string Unicode::CodePointsFromUtf8(const char* utf8_string, size_t byte_count) {
129131
std::u32string code_points;
130132
code_points.reserve(byte_count);
131133

@@ -134,10 +136,10 @@ std::u32string Utf8::ToCodePoints(const char* utf8_string, size_t byte_count) {
134136
const auto lead = static_cast<unsigned char>(utf8_string[i]);
135137
const size_t length = sequence_length(lead);
136138
if (length == 0) {
137-
reject("byte cannot start a UTF-8 sequence");
139+
throw_invalid_unicode("byte cannot start a UTF-8 sequence");
138140
}
139141
if (i + length > byte_count) {
140-
reject("truncated UTF-8 sequence");
142+
throw_invalid_unicode("truncated UTF-8 sequence");
141143
}
142144

143145
if (length == 1) {
@@ -148,7 +150,7 @@ std::u32string Utf8::ToCodePoints(const char* utf8_string, size_t byte_count) {
148150

149151
const auto second = static_cast<unsigned char>(utf8_string[i + 1]);
150152
if (!is_continuation(second)) {
151-
reject("missing UTF-8 continuation byte");
153+
throw_invalid_unicode("missing UTF-8 continuation byte");
152154
}
153155
reject_if_invalid_second_byte(lead, second);
154156

@@ -164,7 +166,7 @@ std::u32string Utf8::ToCodePoints(const char* utf8_string, size_t byte_count) {
164166
for (size_t j = 1; j < length; ++j) {
165167
const auto byte = static_cast<unsigned char>(utf8_string[i + j]);
166168
if (!is_continuation(byte)) {
167-
reject("missing UTF-8 continuation byte");
169+
throw_invalid_unicode("missing UTF-8 continuation byte");
168170
}
169171
code_point = (code_point << 6) | static_cast<char32_t>(byte & 0x3F);
170172
}
@@ -177,7 +179,7 @@ std::u32string Utf8::ToCodePoints(const char* utf8_string, size_t byte_count) {
177179
return code_points;
178180
}
179181

180-
std::u16string Utf8::Utf16FromCodePoints(const char32_t* code_points, size_t count) {
182+
std::u16string Unicode::Utf16FromCodePoints(const char32_t* code_points, size_t count) {
181183
std::u16string utf16_string;
182184
utf16_string.reserve(count);
183185

@@ -198,7 +200,7 @@ std::u16string Utf8::Utf16FromCodePoints(const char32_t* code_points, size_t cou
198200
return utf16_string;
199201
}
200202

201-
std::u32string Utf8::CodePointsFromUtf16(const char16_t* utf16_string, size_t count) {
203+
std::u32string Unicode::CodePointsFromUtf16(const char16_t* utf16_string, size_t count) {
202204
std::u32string code_points;
203205
code_points.reserve(count);
204206

@@ -207,7 +209,7 @@ std::u32string Utf8::CodePointsFromUtf16(const char16_t* utf16_string, size_t co
207209
const char16_t unit = utf16_string[i];
208210

209211
if (unit >= kFirstLowSurrogate && unit <= kLastLowSurrogate) {
210-
reject("unpaired low surrogate");
212+
throw_invalid_unicode("unpaired low surrogate");
211213
}
212214

213215
if (unit < kFirstHighSurrogate || unit > kLastHighSurrogate) {
@@ -217,12 +219,12 @@ std::u32string Utf8::CodePointsFromUtf16(const char16_t* utf16_string, size_t co
217219
}
218220

219221
if (i + 1 >= count) {
220-
reject("high surrogate at end of input");
222+
throw_invalid_unicode("high surrogate at end of input");
221223
}
222224

223225
const char16_t low = utf16_string[i + 1];
224226
if (low < kFirstLowSurrogate || low > kLastLowSurrogate) {
225-
reject("high surrogate not followed by a low surrogate");
227+
throw_invalid_unicode("high surrogate not followed by a low surrogate");
226228
}
227229

228230
const char32_t high_bits = static_cast<char32_t>(unit - kFirstHighSurrogate) << 10;

0 commit comments

Comments
 (0)