Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 16e3ad5

Browse files
tglsfdctglsfdc
committed
Avoid using %c printf format for potentially non-ASCII characters.
Since %c only passes a C "char" to printf, it's incapable of dealing with multibyte characters. Passing just the first byte of such a character leads to an output string that is visibly not correctly encoded, resulting in undesirable behavior such as encoding conversion failures while sending error messages to clients. We've lived with this issue for a long time because it was inconvenient to avoid in a portable fashion. However, now that we always use our own snprintf code, it's reasonable to use the %.*s format to print just one possibly-multibyte character in a string. (We previously avoided that obvious-looking answer in order to work around glibc's bug #6530, cf commits 54cd4f0 and ed437e2.) Hence, run around and fix a bunch of places that used %c to report a character found in a user-supplied string. For simplicity, I did not touch places that were emitting non-user-facing debug messages, or reporting catalog data that should always be ASCII. (It's also unclear how useful this approach could be in frontend code, where it's less certain that we know what encoding we're dealing with.) In passing, improve a couple of poorly-written error messages in pageinspect/heapfuncs.c. This is a longstanding issue, but I'm hesitant to back-patch because of the impact on translatable message strings. In any case this fix would not work reliably before v12. Tom Lane and Quan Zongliang Discussion: https://postgr.es/m/a120087c-4c88-d9d4-1ec5-808d7a7f133d@gmail.com
1 parent 78c8876 commit 16e3ad5

File tree

7 files changed

+47
-32
lines changed

7 files changed

+47
-32
lines changed

‎contrib/hstore/hstore_io.c

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,9 @@ get_val(HSParser *state, bool ignoreeq, bool *escaped)
8080
}
8181
else if (*(state->ptr) == '=' && !ignoreeq)
8282
{
83-
elog(ERROR, "Syntax error near '%c' at position %d", *(state->ptr), (int32) (state->ptr - state->begin));
83+
elog(ERROR, "Syntax error near \"%.*s\" at position %d",
84+
pg_mblen(state->ptr), state->ptr,
85+
(int32) (state->ptr - state->begin));
8486
}
8587
else if (*(state->ptr) == '\\')
8688
{
@@ -219,7 +221,9 @@ parse_hstore(HSParser *state)
219221
}
220222
else if (!isspace((unsigned char) *(state->ptr)))
221223
{
222-
elog(ERROR, "Syntax error near '%c' at position %d", *(state->ptr), (int32) (state->ptr - state->begin));
224+
elog(ERROR, "Syntax error near \"%.*s\" at position %d",
225+
pg_mblen(state->ptr), state->ptr,
226+
(int32) (state->ptr - state->begin));
223227
}
224228
}
225229
else if (st == WGT)
@@ -234,7 +238,9 @@ parse_hstore(HSParser *state)
234238
}
235239
else
236240
{
237-
elog(ERROR, "Syntax error near '%c' at position %d", *(state->ptr), (int32) (state->ptr - state->begin));
241+
elog(ERROR, "Syntax error near \"%.*s\" at position %d",
242+
pg_mblen(state->ptr), state->ptr,
243+
(int32) (state->ptr - state->begin));
238244
}
239245
}
240246
else if (st == WVAL)
@@ -267,7 +273,9 @@ parse_hstore(HSParser *state)
267273
}
268274
else if (!isspace((unsigned char) *(state->ptr)))
269275
{
270-
elog(ERROR, "Syntax error near '%c' at position %d", *(state->ptr), (int32) (state->ptr - state->begin));
276+
elog(ERROR, "Syntax error near \"%.*s\" at position %d",
277+
pg_mblen(state->ptr), state->ptr,
278+
(int32) (state->ptr - state->begin));
271279
}
272280
}
273281
else

‎contrib/pageinspect/heapfuncs.c

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include "catalog/pg_am_d.h"
3131
#include "catalog/pg_type.h"
3232
#include "funcapi.h"
33+
#include "mb/pg_wchar.h"
3334
#include "miscadmin.h"
3435
#include "pageinspect.h"
3536
#include "port/pg_bitutils.h"
@@ -99,7 +100,8 @@ text_to_bits(char *str, int len)
99100
else
100101
ereport(ERROR,
101102
(errcode(ERRCODE_DATA_CORRUPTED),
102-
errmsg("illegal character '%c' in t_bits string", str[off])));
103+
errmsg("invalid character \"%.*s\" in t_bits string",
104+
pg_mblen(str + off), str + off)));
103105

104106
if (off % 8 == 7)
105107
bits[off / 8] = byte;
@@ -460,14 +462,13 @@ tuple_data_split(PG_FUNCTION_ARGS)
460462
if (!t_bits_str)
461463
ereport(ERROR,
462464
(errcode(ERRCODE_DATA_CORRUPTED),
463-
errmsg("argument of t_bits is null, but it is expected to be null and %d character long",
464-
bits_len)));
465+
errmsg("t_bits string must not be NULL")));
465466

466467
bits_str_len = strlen(t_bits_str);
467468
if (bits_len != bits_str_len)
468469
ereport(ERROR,
469470
(errcode(ERRCODE_DATA_CORRUPTED),
470-
errmsg("unexpected length of t_bits %u, expected %d",
471+
errmsg("unexpected length of t_bits string: %u, expected %u",
471472
bits_str_len, bits_len)));
472473

473474
/* do the conversion */
@@ -478,7 +479,7 @@ tuple_data_split(PG_FUNCTION_ARGS)
478479
if (t_bits_str)
479480
ereport(ERROR,
480481
(errcode(ERRCODE_DATA_CORRUPTED),
481-
errmsg("t_bits string is expected to be NULL, but instead it is %zu bytes length",
482+
errmsg("t_bits string is expected to be NULL, but instead it is %zu bytes long",
482483
strlen(t_bits_str))));
483484
}
484485

‎src/backend/utils/adt/encode.c

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
#include <ctype.h>
1717

18+
#include "mb/pg_wchar.h"
1819
#include "utils/builtins.h"
1920
#include "utils/memutils.h"
2021

@@ -171,17 +172,19 @@ hex_encode(const char *src, size_t len, char *dst)
171172
}
172173

173174
static inline char
174-
get_hex(char c)
175+
get_hex(const char *cp)
175176
{
177+
unsigned char c = (unsigned char) *cp;
176178
int res = -1;
177179

178-
if (c > 0 && c < 127)
179-
res = hexlookup[(unsigned char) c];
180+
if (c < 127)
181+
res = hexlookup[c];
180182

181183
if (res < 0)
182184
ereport(ERROR,
183185
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
184-
errmsg("invalid hexadecimal digit: \"%c\"", c)));
186+
errmsg("invalid hexadecimal digit: \"%.*s\"",
187+
pg_mblen(cp), cp)));
185188

186189
return (char) res;
187190
}
@@ -205,13 +208,15 @@ hex_decode(const char *src, size_t len, char *dst)
205208
s++;
206209
continue;
207210
}
208-
v1 = get_hex(*s++) << 4;
211+
v1 = get_hex(s) << 4;
212+
s++;
209213
if (s >= srcend)
210214
ereport(ERROR,
211215
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
212216
errmsg("invalid hexadecimal data: odd number of digits")));
213217

214-
v2 = get_hex(*s++);
218+
v2 = get_hex(s);
219+
s++;
215220
*p++ = v1 | v2;
216221
}
217222

@@ -338,7 +343,8 @@ pg_base64_decode(const char *src, size_t len, char *dst)
338343
if (b < 0)
339344
ereport(ERROR,
340345
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
341-
errmsg("invalid symbol \"%c\" while decoding base64 sequence", (int) c)));
346+
errmsg("invalid symbol \"%.*s\" found while decoding base64 sequence",
347+
pg_mblen(s - 1), s - 1)));
342348
}
343349
/* add it to buffer */
344350
buf = (buf << 6) + b;

‎src/backend/utils/adt/jsonpath_gram.y

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -526,8 +526,8 @@ makeItemLikeRegex(JsonPathParseItem *expr, JsonPathString *pattern,
526526
ereport(ERROR,
527527
(errcode(ERRCODE_SYNTAX_ERROR),
528528
errmsg("invalid input syntax for type %s", "jsonpath"),
529-
errdetail("unrecognized flag character \"%c\" in LIKE_REGEX predicate",
530-
flags->val[i])));
529+
errdetail("unrecognized flag character \"%.*s\" in LIKE_REGEX predicate",
530+
pg_mblen(flags->val + i), flags->val + i)));
531531
break;
532532
}
533533
}

‎src/backend/utils/adt/regexp.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -423,8 +423,8 @@ parse_re_flags(pg_re_flags *flags, text *opts)
423423
default:
424424
ereport(ERROR,
425425
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
426-
errmsg("invalid regular expression option: \"%c\"",
427-
opt_p[i])));
426+
errmsg("invalid regular expression option: \"%.*s\"",
427+
pg_mblen(opt_p + i), opt_p + i)));
428428
break;
429429
}
430430
}

‎src/backend/utils/adt/varbit.c

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -230,8 +230,8 @@ bit_in(PG_FUNCTION_ARGS)
230230
else if (*sp != '0')
231231
ereport(ERROR,
232232
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
233-
errmsg("\"%c\" is not a valid binary digit",
234-
*sp)));
233+
errmsg("\"%.*s\" is not a valid binary digit",
234+
pg_mblen(sp), sp)));
235235

236236
x >>= 1;
237237
if (x == 0)
@@ -255,8 +255,8 @@ bit_in(PG_FUNCTION_ARGS)
255255
else
256256
ereport(ERROR,
257257
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
258-
errmsg("\"%c\" is not a valid hexadecimal digit",
259-
*sp)));
258+
errmsg("\"%.*s\" is not a valid hexadecimal digit",
259+
pg_mblen(sp), sp)));
260260

261261
if (bc)
262262
{
@@ -531,8 +531,8 @@ varbit_in(PG_FUNCTION_ARGS)
531531
else if (*sp != '0')
532532
ereport(ERROR,
533533
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
534-
errmsg("\"%c\" is not a valid binary digit",
535-
*sp)));
534+
errmsg("\"%.*s\" is not a valid binary digit",
535+
pg_mblen(sp), sp)));
536536

537537
x >>= 1;
538538
if (x == 0)
@@ -556,8 +556,8 @@ varbit_in(PG_FUNCTION_ARGS)
556556
else
557557
ereport(ERROR,
558558
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
559-
errmsg("\"%c\" is not a valid hexadecimal digit",
560-
*sp)));
559+
errmsg("\"%.*s\" is not a valid hexadecimal digit",
560+
pg_mblen(sp), sp)));
561561

562562
if (bc)
563563
{

‎src/backend/utils/adt/varlena.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5586,8 +5586,8 @@ text_format(PG_FUNCTION_ARGS)
55865586
if (strchr("sIL", *cp) == NULL)
55875587
ereport(ERROR,
55885588
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
5589-
errmsg("unrecognized format() type specifier \"%c\"",
5590-
*cp),
5589+
errmsg("unrecognized format() type specifier \"%.*s\"",
5590+
pg_mblen(cp), cp),
55915591
errhint("For a single \"%%\" use \"%%%%\".")));
55925592

55935593
/* If indirect width was specified, get its value */
@@ -5707,8 +5707,8 @@ text_format(PG_FUNCTION_ARGS)
57075707
/* should not get here, because of previous check */
57085708
ereport(ERROR,
57095709
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
5710-
errmsg("unrecognized format() type specifier \"%c\"",
5711-
*cp),
5710+
errmsg("unrecognized format() type specifier \"%.*s\"",
5711+
pg_mblen(cp), cp),
57125712
errhint("For a single \"%%\" use \"%%%%\".")));
57135713
break;
57145714
}

0 commit comments

Comments
 (0)