Skip to content

Commit 503c0ad

Browse files
jeff-davisjeff-davis
committed
Fix convert_case(), introduced in 5c40364.
Check source length before checking for NUL terminator to avoid reading one byte past the string end. Also fix unreachable bug when caller does not expect NUL-terminated result. Add unit test coverage of convert_case() in case_test.c, which makes it easier to reproduce the valgrind failure. Discussion: https://postgr.es/m/7a9fd36d-7a38-4dc2-e676-fc939491a95a@gmail.com Reported-by: Alexander Lakhin
1 parent 3330a8d commit 503c0ad

File tree

2 files changed

+101
-6
lines changed

2 files changed

+101
-6
lines changed

‎src/common/unicode/case_test.c‎

Lines changed: 98 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,9 @@ icu_test_simple(pg_wchar code)
4848
}
4949
}
5050

51+
/*
52+
* Exhaustively compare case mappings with the results from ICU.
53+
*/
5154
static void
5255
test_icu(void)
5356
{
@@ -82,9 +85,100 @@ test_icu(void)
8285
}
8386
#endif
8487

85-
/*
86-
* Exhaustively compare case mappings with the results from libc and ICU.
87-
*/
88+
static void
89+
test_strlower(const char *test_string, const char *expected)
90+
{
91+
size_t src1len = strlen(test_string);
92+
size_t src2len = -1; /* NUL-terminated */
93+
size_t dst1len = strlen(expected);
94+
size_t dst2len = strlen(expected) + 1; /* NUL-terminated */
95+
char *src1 = malloc(src1len);
96+
char *dst1 = malloc(dst1len);
97+
char *src2 = strdup(test_string);
98+
char *dst2 = malloc(dst2len);
99+
size_t needed;
100+
101+
memcpy(src1, test_string, src1len); /* not NUL-terminated */
102+
103+
/* neither source nor destination are NUL-terminated */
104+
memset(dst1, 0x7F, dst1len);
105+
needed = unicode_strlower(dst1, dst1len, src1, src1len);
106+
if (needed != strlen(expected))
107+
{
108+
printf("case_test: convert_case test1 FAILURE: needed %zu\n", needed);
109+
exit(1);
110+
}
111+
if (memcmp(dst1, expected, dst1len) != 0)
112+
{
113+
printf("case_test: convert_case test1 FAILURE: test: '%s' result: '%.*s' expected: '%s'\n",
114+
test_string, (int) dst1len, dst1, expected);
115+
exit(1);
116+
}
117+
118+
/* destination is NUL-terminated and source is not */
119+
memset(dst2, 0x7F, dst2len);
120+
needed = unicode_strlower(dst2, dst2len, src1, src1len);
121+
if (needed != strlen(expected))
122+
{
123+
printf("case_test: convert_case test2 FAILURE: needed %zu\n", needed);
124+
exit(1);
125+
}
126+
if (strcmp(dst2, expected) != 0)
127+
{
128+
printf("case_test: convert_case test2 FAILURE: test: '%s' result: '%s' expected: '%s'\n",
129+
test_string, dst2, expected);
130+
exit(1);
131+
}
132+
133+
/* source is NUL-terminated and destination is not */
134+
memset(dst1, 0x7F, dst1len);
135+
needed = unicode_strlower(dst1, dst1len, src2, src2len);
136+
if (needed != strlen(expected))
137+
{
138+
printf("case_test: convert_case test3 FAILURE: needed %zu\n", needed);
139+
exit(1);
140+
}
141+
if (memcmp(dst1, expected, dst1len) != 0)
142+
{
143+
printf("case_test: convert_case test3 FAILURE: test: '%s' result: '%.*s' expected: '%s'\n",
144+
test_string, (int) dst1len, dst1, expected);
145+
exit(1);
146+
}
147+
148+
/* both source and destination are NUL-terminated */
149+
memset(dst2, 0x7F, dst2len);
150+
needed = unicode_strlower(dst2, dst2len, src2, src2len);
151+
if (needed != strlen(expected))
152+
{
153+
printf("case_test: convert_case test4 FAILURE: needed %zu\n", needed);
154+
exit(1);
155+
}
156+
if (strcmp(dst2, expected) != 0)
157+
{
158+
printf("case_test: convert_case test4 FAILURE: test: '%s' result: '%s' expected: '%s'\n",
159+
test_string, dst2, expected);
160+
exit(1);
161+
}
162+
163+
free(src1);
164+
free(dst1);
165+
free(src2);
166+
free(dst2);
167+
}
168+
169+
static void
170+
test_convert_case()
171+
{
172+
/* test string with no case changes */
173+
test_strlower("√∞", "√∞");
174+
/* test string with case changes */
175+
test_strlower("ABC", "abc");
176+
/* test string with case changes and byte length changes */
177+
test_strlower("ȺȺȺ", "ⱥⱥⱥ");
178+
179+
printf("case_test: convert_case: success\n");
180+
}
181+
88182
int
89183
main(int argc, char **argv)
90184
{
@@ -96,5 +190,6 @@ main(int argc, char **argv)
96190
printf("case_test: ICU not available; skipping\n");
97191
#endif
98192

193+
test_convert_case();
99194
exit(0);
100195
}

‎src/common/unicode_case.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ convert_case(char *dst, size_t dstsize, const char *src, ssize_t srclen,
104104
size_t srcoff = 0;
105105
size_t result_len = 0;
106106

107-
while (src[srcoff] != '\0' && (srclen < 0 || srcoff < srclen))
107+
while ((srclen < 0 || srcoff < srclen) && src[srcoff] != '\0')
108108
{
109109
pg_wchar u1 = utf8_to_unicode((unsigned char *) src + srcoff);
110110
int u1len = unicode_utf8len(u1);
@@ -115,15 +115,15 @@ convert_case(char *dst, size_t dstsize, const char *src, ssize_t srclen,
115115
pg_wchar u2 = casemap->simplemap[casekind];
116116
pg_wchar u2len = unicode_utf8len(u2);
117117

118-
if (result_len + u2len < dstsize)
118+
if (result_len + u2len <= dstsize)
119119
unicode_to_utf8(u2, (unsigned char *) dst + result_len);
120120

121121
result_len += u2len;
122122
}
123123
else
124124
{
125125
/* no mapping; copy bytes from src */
126-
if (result_len + u1len < dstsize)
126+
if (result_len + u1len <= dstsize)
127127
memcpy(dst + result_len, src + srcoff, u1len);
128128

129129
result_len += u1len;

0 commit comments

Comments
 (0)