Fix bug #62759: Buggy grapheme_substr() on edge case

diff --git a/NEWS b/NEWS
index 5c6f6b92be85597a95f097e7e17e568a95739be9..9ec6740f2dd79321907ddcb56178e35a0f00cbbb 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -10,6 +10,9 @@ PHP                                                                        NEWS
   . Fixed bug #65066 (Cli server not responsive when responding with 422 http
     status code). (Adam)
 
+- Intl: 
+  . Fixed bug #62759: Buggy grapheme_substr() on edge case. (Stas)
+
 - Sockets:
   . Implemented FR #63472 (Setting SO_BINDTODEVICE with socket_set_option). 
     (Damjan Cvetko)

diff --git a/ext/intl/grapheme/grapheme_string.c b/ext/intl/grapheme/grapheme_string.c
index 475bbe4184791d1c2828083ec3361f876437ec7a..1b7327e0012f74ab18501e1ba8233e92a9a6039f 100644 (file)
--- a/ext/intl/grapheme/grapheme_string.c
+++ b/ext/intl/grapheme/grapheme_string.c
@@ -434,6 +434,7 @@ PHP_FUNCTION(grapheme_substr)
                grapheme_substr_ascii((char *)str, str_len, start, length, ZEND_NUM_ARGS(), (char **) &sub_str, &sub_str_len);
 
                if ( NULL == sub_str ) {
+                       intl_error_set( NULL, U_ILLEGAL_ARGUMENT_ERROR, "grapheme_substr: invalid parameters", 1 TSRMLS_CC );
                        RETURN_FALSE;
                }
 
@@ -530,6 +531,15 @@ PHP_FUNCTION(grapheme_substr)
                RETURN_STRINGL(((char *)sub_str), sub_str_len, 0);
        }
 
+       if(length == 0) {
+               /* empty length - we've validated start, we can return "" now */
+               if (ustr) {
+                       efree(ustr);
+               }
+               ubrk_close(bi);
+               RETURN_EMPTY_STRING();          
+       }
+
        /* find the end point of the string to return */
 
        if ( length < 0 ) {
@@ -554,25 +564,31 @@ PHP_FUNCTION(grapheme_substr)
                length += iter_val;
        }
 
+       ubrk_close(bi);
+
        if ( UBRK_DONE == sub_str_end_pos) {
                if(length < 0) {
-
                        intl_error_set( NULL, U_ILLEGAL_ARGUMENT_ERROR, "grapheme_substr: length not contained in string", 1 TSRMLS_CC );
 
                        efree(ustr);
-                       ubrk_close(bi);
                        RETURN_FALSE;
                } else {
                        sub_str_end_pos = ustr_len;
                }
        }
+       
+       if(sub_str_start_pos > sub_str_end_pos) {
+               intl_error_set( NULL, U_ILLEGAL_ARGUMENT_ERROR, "grapheme_substr: length is beyond start", 1 TSRMLS_CC );
+
+               efree(ustr);
+               RETURN_FALSE;
+       }
 
        sub_str = NULL;
        status = U_ZERO_ERROR;
        intl_convert_utf16_to_utf8((char **)&sub_str, &sub_str_len, ustr + sub_str_start_pos, ( sub_str_end_pos - sub_str_start_pos ), &status);
 
        efree( ustr );
-       ubrk_close( bi );
 
        if ( U_FAILURE( status ) ) {
                /* Set global error code. */

diff --git a/ext/intl/tests/bug62759.phpt b/ext/intl/tests/bug62759.phpt
new file mode 100644 (file)
index 0000000..d4126b7
--- /dev/null
+++ b/ext/intl/tests/bug62759.phpt
@@ -0,0 +1,24 @@
+--TEST--
+Bug #62759: Buggy grapheme_substr() on edge case
+--SKIPIF--
+<?php if( !extension_loaded( 'intl' ) ) print 'skip'; ?>
+--FILE--
+<?php
+var_dump(substr('deja', 1, -4));
+var_dump(substr('deja', -1, 0));
+var_dump(grapheme_substr('deja', 1, -4));
+var_dump(intl_get_error_message());
+var_dump(grapheme_substr('deja', -1, 0));
+var_dump(grapheme_substr('déjà', 1, -4));
+var_dump(intl_get_error_message());
+var_dump(grapheme_substr('déjà', -1, 0));
+?>
+--EXPECT--
+bool(false)
+string(0) ""
+bool(false)
+string(61) "grapheme_substr: invalid parameters: U_ILLEGAL_ARGUMENT_ERROR"
+string(0) ""
+bool(false)
+string(65) "grapheme_substr: length is beyond start: U_ILLEGAL_ARGUMENT_ERROR"
+string(0) ""