From 917cc8a595c19201f83673c26923472cab4b9884 Mon Sep 17 00:00:00 2001 From: Philip Hofstetter Date: Fri, 9 Oct 2020 11:55:33 +0200 Subject: [PATCH 1/2] intl: report more information about message pattern parse errors the message patterns can be pretty complex, so reporting a generic U_PARSE_ERROR without any additional information makes it needlessly hard to fix erroneous patterns. This commit makes use of the additional UParseError* parameter to umsg_open to retrieve more details about the parse error to report that to the user via intl_get_error_message() --- ext/intl/msgformat/msgformat_format.c | 18 +++++++++++++++++- ext/intl/tests/msgfmt_fail2.phpt | 6 +++--- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/ext/intl/msgformat/msgformat_format.c b/ext/intl/msgformat/msgformat_format.c index 1e4c49609ec33..0f422d23014b1 100644 --- a/ext/intl/msgformat/msgformat_format.c +++ b/ext/intl/msgformat/msgformat_format.c @@ -80,6 +80,7 @@ PHP_FUNCTION( msgfmt_format_message ) size_t slocale_len = 0; MessageFormatter_object mf; MessageFormatter_object *mfo = &mf; + UParseError parse_error; /* Parse parameters. */ if( zend_parse_method_parameters( ZEND_NUM_ARGS(), getThis(), "ssa", @@ -119,10 +120,25 @@ PHP_FUNCTION( msgfmt_format_message ) #endif /* Create an ICU message formatter. */ - MSG_FORMAT_OBJECT(mfo) = umsg_open(spattern, spattern_len, slocale, NULL, &INTL_DATA_ERROR_CODE(mfo)); + MSG_FORMAT_OBJECT(mfo) = umsg_open(spattern, spattern_len, slocale, &parse_error, &INTL_DATA_ERROR_CODE(mfo)); if(spattern && spattern_len) { efree(spattern); } + + if (INTL_DATA_ERROR_CODE( mfo ) == U_PATTERN_SYNTAX_ERROR) { + char *msg = NULL; + smart_str parse_error_str; + parse_error_str = intl_parse_error_to_string( &parse_error ); + spprintf( &msg, 0, "pattern syntax error (%s)", parse_error_str.s? ZSTR_VAL(parse_error_str.s) : "unknown parser error" ); + smart_str_free( &parse_error_str ); + + intl_error_set_code( NULL, INTL_DATA_ERROR_CODE( mfo ) ); + intl_errors_set_custom_msg( INTL_DATA_ERROR_P( mfo ), msg, 1 ); + + efree( msg ); + RETURN_FALSE; + } + INTL_METHOD_CHECK_STATUS(mfo, "Creating message formatter failed"); msgfmt_do_format(mfo, args, return_value); diff --git a/ext/intl/tests/msgfmt_fail2.phpt b/ext/intl/tests/msgfmt_fail2.phpt index 4061eab3dc4d1..3a4d7f20b822e 100644 --- a/ext/intl/tests/msgfmt_fail2.phpt +++ b/ext/intl/tests/msgfmt_fail2.phpt @@ -146,9 +146,9 @@ TypeError: msgfmt_create(): Argument #1 ($locale) must be of type string, array 'U_ZERO_ERROR' IntlException: Constructor failed in %s on line %d -'msgfmt_create: message formatter creation failed: U_PATTERN_SYNTAX_ERROR' -'msgfmt_create: message formatter creation failed: U_PATTERN_SYNTAX_ERROR' -'msgfmt_create: message formatter creation failed: U_PATTERN_SYNTAX_ERROR' +'pattern syntax error (parse error at offset 1, after "{", before or at "0,choice}"): U_PATTERN_SYNTAX_ERROR' +'pattern syntax error (parse error at offset 1, after "{", before or at "0,choice}"): U_PATTERN_SYNTAX_ERROR' +'pattern syntax error (parse error at offset 1, after "{", before or at "0,choice}"): U_PATTERN_SYNTAX_ERROR' IntlException: Constructor failed in %s on line %d 'msgfmt_create: message formatter creation failed: U_UNMATCHED_BRACES' From 6aac1478c83627466320901666344b05403fcb4d Mon Sep 17 00:00:00 2001 From: Philip Hofstetter Date: Fri, 9 Oct 2020 11:58:38 +0200 Subject: [PATCH 2/2] better error reporting from IntlMessage constructor All possible failures when calling IntlMessage::__construct() would be masked away with a generic "Constructor failed" message. This would include invalid patterns. This commit makes sure that the underlying error that caused the constructor failure is reported as part of the IntlException error message. Additionally, similar to the previous commit, this also uses this improved error reporting facility to also report details on parsing errors while trying to parse the message pattern --- ext/intl/msgformat/msgformat.c | 22 ++++++++++++++++++++-- ext/intl/tests/msgfmt_fail2.phpt | 10 +++++----- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/ext/intl/msgformat/msgformat.c b/ext/intl/msgformat/msgformat.c index cb625ed0372f2..39448ec7891f4 100644 --- a/ext/intl/msgformat/msgformat.c +++ b/ext/intl/msgformat/msgformat.c @@ -34,6 +34,8 @@ static int msgfmt_ctor(INTERNAL_FUNCTION_PARAMETERS) int spattern_len = 0; zval* object; MessageFormatter_object* mfo; + UParseError parse_error; + intl_error_reset( NULL ); object = return_value; @@ -74,12 +76,26 @@ static int msgfmt_ctor(INTERNAL_FUNCTION_PARAMETERS) (mfo)->mf_data.orig_format_len = pattern_len; /* Create an ICU message formatter. */ - MSG_FORMAT_OBJECT(mfo) = umsg_open(spattern, spattern_len, locale, NULL, &INTL_DATA_ERROR_CODE(mfo)); + MSG_FORMAT_OBJECT(mfo) = umsg_open(spattern, spattern_len, locale, &parse_error, &INTL_DATA_ERROR_CODE(mfo)); if(spattern) { efree(spattern); } + if (INTL_DATA_ERROR_CODE( mfo ) == U_PATTERN_SYNTAX_ERROR) { + char *msg = NULL; + smart_str parse_error_str; + parse_error_str = intl_parse_error_to_string( &parse_error ); + spprintf( &msg, 0, "pattern syntax error (%s)", parse_error_str.s? ZSTR_VAL(parse_error_str.s) : "unknown parser error" ); + smart_str_free( &parse_error_str ); + + intl_error_set_code( NULL, INTL_DATA_ERROR_CODE( mfo ) ); + intl_errors_set_custom_msg( INTL_DATA_ERROR_P( mfo ), msg, 1 ); + + efree( msg ); + return FAILURE; + } + INTL_CTOR_CHECK_STATUS(mfo, "msgfmt_create: message formatter creation failed"); return SUCCESS; } @@ -105,7 +121,9 @@ PHP_METHOD( MessageFormatter, __construct ) return_value = ZEND_THIS; if (msgfmt_ctor(INTERNAL_FUNCTION_PARAM_PASSTHRU) == FAILURE) { if (!EG(exception)) { - zend_throw_exception(IntlException_ce_ptr, "Constructor failed", 0); + zend_string *err = intl_error_get_message(NULL); + zend_throw_exception(IntlException_ce_ptr, ZSTR_VAL(err), intl_error_get_code(NULL)); + zend_string_release_ex(err, 0); } } zend_restore_error_handling(&error_handling); diff --git a/ext/intl/tests/msgfmt_fail2.phpt b/ext/intl/tests/msgfmt_fail2.phpt index 3a4d7f20b822e..f604de0727694 100644 --- a/ext/intl/tests/msgfmt_fail2.phpt +++ b/ext/intl/tests/msgfmt_fail2.phpt @@ -126,12 +126,12 @@ ArgumentCountError: msgfmt_create() expects exactly 2 arguments, 1 given in %s o ArgumentCountError: MessageFormatter::create() expects exactly 2 arguments, 1 given in %s on line %d 'U_ZERO_ERROR' -IntlException: Constructor failed in %s on line %d +IntlException: msgfmt_create: message formatter creation failed: U_ILLEGAL_ARGUMENT_ERROR in %s on line %d 'msgfmt_create: message formatter creation failed: U_ILLEGAL_ARGUMENT_ERROR' 'msgfmt_create: message formatter creation failed: U_ILLEGAL_ARGUMENT_ERROR' 'msgfmt_create: message formatter creation failed: U_ILLEGAL_ARGUMENT_ERROR' -IntlException: Constructor failed in %s on line %d +IntlException: msgfmt_create: message formatter creation failed: U_ILLEGAL_ARGUMENT_ERROR in %s on line %d 'msgfmt_create: message formatter creation failed: U_ILLEGAL_ARGUMENT_ERROR' 'msgfmt_create: message formatter creation failed: U_ILLEGAL_ARGUMENT_ERROR' 'msgfmt_create: message formatter creation failed: U_ILLEGAL_ARGUMENT_ERROR' @@ -145,17 +145,17 @@ TypeError: MessageFormatter::create(): Argument #1 ($locale) must be of type str TypeError: msgfmt_create(): Argument #1 ($locale) must be of type string, array given in %s on line %d 'U_ZERO_ERROR' -IntlException: Constructor failed in %s on line %d +IntlException: pattern syntax error (parse error at offset 1, after "{", before or at "0,choice}"): U_PATTERN_SYNTAX_ERROR in %s on line %d 'pattern syntax error (parse error at offset 1, after "{", before or at "0,choice}"): U_PATTERN_SYNTAX_ERROR' 'pattern syntax error (parse error at offset 1, after "{", before or at "0,choice}"): U_PATTERN_SYNTAX_ERROR' 'pattern syntax error (parse error at offset 1, after "{", before or at "0,choice}"): U_PATTERN_SYNTAX_ERROR' -IntlException: Constructor failed in %s on line %d +IntlException: msgfmt_create: message formatter creation failed: U_UNMATCHED_BRACES in %s on line %d 'msgfmt_create: message formatter creation failed: U_UNMATCHED_BRACES' 'msgfmt_create: message formatter creation failed: U_UNMATCHED_BRACES' 'msgfmt_create: message formatter creation failed: U_UNMATCHED_BRACES' -IntlException: Constructor failed in %s on line %d +IntlException: msgfmt_create: error converting pattern to UTF-16: U_INVALID_CHAR_FOUND in %s on line %d 'msgfmt_create: error converting pattern to UTF-16: U_INVALID_CHAR_FOUND' 'msgfmt_create: error converting pattern to UTF-16: U_INVALID_CHAR_FOUND' 'msgfmt_create: error converting pattern to UTF-16: U_INVALID_CHAR_FOUND'