Re: [LTP] [PATCH 05/15] ujson: reader: Combine UTF-16 surrogate pairs

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> RFC 8259 specifies that characters outside the BMP may be represented
> as a surrogate pair (\uD800-\uDBFF followed by \uDC00-\uDFFF).
> Currently, each \uXXXX escape is converted independently to UTF-8,
> producing invalid surrogate code point sequences instead of combining
> them into a single UTF-8 sequence. Lone surrogates are also improperly
> accepted. Combine valid surrogate pairs and reject unpaired surrogates.
> 
> Also return the UTF-8 length instead of the code point when no output
> buffer is given. The caller treats 0 as failure, so \u0000 made
> copy_str() fail without setting an error message.
> 
> Fixes: c50a0e0751c8 ("libs: Vendor ujson library")
> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
> ---
>  libs/ujson/ujson_reader.c | 28 +++++++++++++++++++++++++++-
>  1 file changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/libs/ujson/ujson_reader.c b/libs/ujson/ujson_reader.c
> index 2219c5801..0975ca1a1 100644
> --- a/libs/ujson/ujson_reader.c
> +++ b/libs/ujson/ujson_reader.c
> @@ -134,8 +134,34 @@ static unsigned int parse_ucode_esc(ujson_reader *buf, char *str,
>  	if (ucode < 0)
>  		return 0;
>  
> +	if (ucode >= 0xd800 && ucode <= 0xdbff) {
> +		int32_t low;
> +
> +		if (peekb(buf) != '\\' || peekb_off(buf, 1) != 'u') {
> +			ujson_err(buf, "Unpaired high surrogate");
> +			return 0;
> +		}
> +
> +		getb(buf);
> +		getb(buf);
> +
> +		low = parse_ucode_cp(buf);
> +		if (low < 0)
> +			return 0;
> +
> +		if (low < 0xdc00 || low > 0xdfff) {
> +			ujson_err(buf, "Invalid low surrogate in escape sequence");
> +			return 0;
> +		}
> +
> +		ucode = 0x10000 + (((ucode - 0xd800) << 10) | (low - 0xdc00));
> +	} else if (ucode >= 0xdc00 && ucode <= 0xdfff) {
> +		ujson_err(buf, "Unpaired low surrogate");
> +		return 0;
> +	}

I would put this code first, not in an elase branch.


        if (ucode >= 0xdc00 && ucode <= 0xdfff) {
		ujson_err(buf, "Unpaired low surrogate");
		return 0;
	}

	if (ucode >= 0xd800 && ucode <= 0xdbff) {
	    ...
	}

Also it's unlikely that we will hit this in LTP, unless someone adds
emojis or hieroglyphs in test strings.

>  	if (!str)
> -		return ucode;
> +		return ujson_utf8_bytes(ucode);

This is a good bugfix, should have gone in a separate patch. Without
this we may overshoot the required size for the buffer by order of a
magnitude.

>  	if (ujson_utf8_bytes(ucode) + 1 >= len - off) {
>  		ujson_err(buf, "String buffer too short!");
> 
> -- 
> 2.51.0
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp