]> granicus.if.org Git - php/commitdiff
Fix Intl constructor leaks
authorNikita Popov <nikic@php.net>
Fri, 17 Apr 2015 08:26:26 +0000 (10:26 +0200)
committerNikita Popov <nikic@php.net>
Fri, 17 Apr 2015 08:33:59 +0000 (10:33 +0200)
Drop the Z_OBJ(return_value) = NULL hack and return status code
from ctor function instead.

ext/intl/breakiterator/breakiterator_methods.cpp
ext/intl/collator/collator_create.c
ext/intl/dateformat/dateformat_create.cpp
ext/intl/formatter/formatter_main.c
ext/intl/intl_data.h
ext/intl/msgformat/msgformat.c
ext/intl/resourcebundle/resourcebundle_class.c
ext/intl/spoofchecker/spoofchecker_create.c
ext/intl/transliterator/transliterator_methods.c

index baab5a682d68e5bdc3512997fad9583c6b84d316..ce855ebacd0e5798d519fdb8fe7f502ceba95b07 100644 (file)
@@ -162,11 +162,11 @@ U_CFUNC PHP_FUNCTION(breakiter_set_text)
        BREAKITER_METHOD_FETCH_OBJECT;
 
        ut = utext_openUTF8(ut, text->val, text->len, BREAKITER_ERROR_CODE_P(bio));
-       INTL_CTOR_CHECK_STATUS(bio, "breakiter_set_text: error opening UText");
+       INTL_METHOD_CHECK_STATUS_OR_NULL(bio, "breakiter_set_text: error opening UText");
 
        bio->biter->setText(ut, BREAKITER_ERROR_CODE(bio));
        utext_close(ut); /* ICU shallow clones the UText */
-       INTL_CTOR_CHECK_STATUS(bio, "breakiter_set_text: error calling "
+       INTL_METHOD_CHECK_STATUS_OR_NULL(bio, "breakiter_set_text: error calling "
                "BreakIterator::setText()");
 
        /* When ICU clones the UText, it does not copy the buffer, so we have to
index b6ad4502db0dd620fc520d0a9388d8fb7bbf6bb3..d32a4c637f0b24c44cf9f5b542d898ca01c4e9f1 100644 (file)
@@ -25,7 +25,7 @@
 #include "intl_data.h"
 
 /* {{{ */
-static void collator_ctor(INTERNAL_FUNCTION_PARAMETERS, zend_bool is_constructor)
+static int collator_ctor(INTERNAL_FUNCTION_PARAMETERS, zend_bool is_constructor)
 {
        const char*      locale;
        size_t           locale_len = 0;
@@ -41,11 +41,10 @@ static void collator_ctor(INTERNAL_FUNCTION_PARAMETERS, zend_bool is_constructor
        {
                intl_error_set( NULL, U_ILLEGAL_ARGUMENT_ERROR,
                        "collator_create: unable to parse input params", 0 );
-               zval_dtor(return_value);
-               RETURN_NULL();
+               return FAILURE;
        }
 
-       INTL_CHECK_LOCALE_LEN_OBJ(locale_len, return_value);
+       INTL_CHECK_LOCALE_LEN_OR_FAILURE(locale_len);
        COLLATOR_METHOD_FETCH_OBJECT;
 
        if(locale_len == 0) {
@@ -55,6 +54,7 @@ static void collator_ctor(INTERNAL_FUNCTION_PARAMETERS, zend_bool is_constructor
        /* Open ICU collator. */
        co->ucoll = ucol_open( locale, COLLATOR_ERROR_CODE_P( co ) );
        INTL_CTOR_CHECK_STATUS(co, "collator_create: unable to open ICU collator");
+       return SUCCESS;
 }
 /* }}} */
 
@@ -64,7 +64,10 @@ static void collator_ctor(INTERNAL_FUNCTION_PARAMETERS, zend_bool is_constructor
 PHP_FUNCTION( collator_create )
 {
        object_init_ex( return_value, Collator_ce_ptr );
-       collator_ctor(INTERNAL_FUNCTION_PARAM_PASSTHRU, 0);
+       if (collator_ctor(INTERNAL_FUNCTION_PARAM_PASSTHRU, 0) == FAILURE) {
+               zval_ptr_dtor(return_value);
+               RETURN_NULL();
+       }
 }
 /* }}} */
 
@@ -77,8 +80,7 @@ PHP_METHOD( Collator, __construct )
 
        zend_replace_error_handling(EH_THROW, IntlException_ce_ptr, &error_handling);
        return_value = getThis();
-       collator_ctor(INTERNAL_FUNCTION_PARAM_PASSTHRU, 1);
-       if (Z_TYPE_P(return_value) == IS_OBJECT && Z_OBJ_P(return_value) == NULL) {
+       if (collator_ctor(INTERNAL_FUNCTION_PARAM_PASSTHRU, 1) == FAILURE) {
                if (!EG(exception)) {
                        zend_throw_exception(IntlException_ce_ptr, "Constructor failed", 0);
                }
index afc182131d6e7a486c4c41e57fea2cc4f105ae96..1999b6a8c135c69cddf43916a9973278460eacb4 100644 (file)
@@ -37,7 +37,7 @@ extern "C" {
 #include "zend_exceptions.h"
 
 /* {{{ */
-static void datefmt_ctor(INTERNAL_FUNCTION_PARAMETERS, zend_bool is_constructor)
+static int datefmt_ctor(INTERNAL_FUNCTION_PARAMETERS, zend_bool is_constructor)
 {
        zval            *object;
 
@@ -58,7 +58,7 @@ static void datefmt_ctor(INTERNAL_FUNCTION_PARAMETERS, zend_bool is_constructor)
     UChar*      svalue                 = NULL;         /* UTF-16 pattern_str */
     int32_t     slength                        = 0;
        IntlDateFormatter_object* dfo;
-  int zpp_flags = is_constructor ? ZEND_PARSE_PARAMS_THROW : 0;
+       int zpp_flags = is_constructor ? ZEND_PARSE_PARAMS_THROW : 0;
 
        intl_error_reset(NULL);
        object = return_value;
@@ -68,11 +68,10 @@ static void datefmt_ctor(INTERNAL_FUNCTION_PARAMETERS, zend_bool is_constructor)
                        &calendar_zv, &pattern_str, &pattern_str_len) == FAILURE) {
                intl_error_set( NULL, U_ILLEGAL_ARGUMENT_ERROR, "datefmt_create: "
                                "unable to parse input parameters", 0);
-               Z_OBJ_P(return_value) = NULL;
-               return;
+               return FAILURE;
     }
 
-       INTL_CHECK_LOCALE_LEN_OBJ(locale_len, return_value);
+       INTL_CHECK_LOCALE_LEN_OR_FAILURE(locale_len);
        if (locale_len == 0) {
                locale_str = intl_locale_get_default();
        }
@@ -83,7 +82,7 @@ static void datefmt_ctor(INTERNAL_FUNCTION_PARAMETERS, zend_bool is_constructor)
        if (DATE_FORMAT_OBJECT(dfo) != NULL) {
                intl_errors_set(INTL_DATA_ERROR_P(dfo), U_ILLEGAL_ARGUMENT_ERROR,
                                "datefmt_create: cannot call constructor twice", 0);
-               return;
+               return FAILURE;
        }
 
        /* process calendar */
@@ -162,10 +161,8 @@ error:
        if (calendar != NULL && calendar_owned) {
                delete calendar;
        }
-       if (U_FAILURE(intl_error_get_code(NULL))) {
-               /* free_object handles partially constructed instances fine */
-               Z_OBJ_P(return_value) = NULL;
-       }
+
+       return U_FAILURE(intl_error_get_code(NULL)) ? FAILURE : SUCCESS;
 }
 /* }}} */
 
@@ -177,8 +174,8 @@ error:
 U_CFUNC PHP_FUNCTION( datefmt_create )
 {
     object_init_ex( return_value, IntlDateFormatter_ce_ptr );
-       datefmt_ctor(INTERNAL_FUNCTION_PARAM_PASSTHRU, 0);
-       if (Z_TYPE_P(return_value) == IS_OBJECT && Z_OBJ_P(return_value) == NULL) {
+       if (datefmt_ctor(INTERNAL_FUNCTION_PARAM_PASSTHRU, 0) == FAILURE) {
+               zval_ptr_dtor(return_value);
                RETURN_NULL();
        }
 }
@@ -195,8 +192,7 @@ U_CFUNC PHP_METHOD( IntlDateFormatter, __construct )
        /* return_value param is being changed, therefore we will always return
         * NULL here */
        return_value = getThis();
-       datefmt_ctor(INTERNAL_FUNCTION_PARAM_PASSTHRU, 1);
-       if (Z_TYPE_P(return_value) == IS_OBJECT && Z_OBJ_P(return_value) == NULL) {
+       if (datefmt_ctor(INTERNAL_FUNCTION_PARAM_PASSTHRU, 1) == FAILURE) {
                if (!EG(exception)) {
                        zend_throw_exception(IntlException_ce_ptr, "Constructor failed", 0);
                }
index 5b7e6340539d5a0a591756aaccdd257927775818..bf0d2a80b7ddf1768d8145cbce7958b27e9320d7 100644 (file)
@@ -25,7 +25,7 @@
 #include "intl_convert.h"
 
 /* {{{ */
-static void numfmt_ctor(INTERNAL_FUNCTION_PARAMETERS, zend_bool is_constructor)
+static int numfmt_ctor(INTERNAL_FUNCTION_PARAMETERS, zend_bool is_constructor)
 {
        const char* locale;
        char*       pattern = NULL;
@@ -42,11 +42,10 @@ static void numfmt_ctor(INTERNAL_FUNCTION_PARAMETERS, zend_bool is_constructor)
        {
                intl_error_set( NULL, U_ILLEGAL_ARGUMENT_ERROR,
                        "numfmt_create: unable to parse input parameters", 0 );
-               Z_OBJ_P(return_value) = NULL;
-               return;
+               return FAILURE;
        }
 
-       INTL_CHECK_LOCALE_LEN_OBJ(locale_len, return_value);
+       INTL_CHECK_LOCALE_LEN_OR_FAILURE(locale_len);
        object = return_value;
        FORMATTER_METHOD_FETCH_OBJECT_NO_CHECK;
 
@@ -68,6 +67,7 @@ static void numfmt_ctor(INTERNAL_FUNCTION_PARAMETERS, zend_bool is_constructor)
        }
 
        INTL_CTOR_CHECK_STATUS(nfo, "numfmt_create: number formatter creation failed");
+       return SUCCESS;
 }
 /* }}} */
 
@@ -79,8 +79,8 @@ static void numfmt_ctor(INTERNAL_FUNCTION_PARAMETERS, zend_bool is_constructor)
 PHP_FUNCTION( numfmt_create )
 {
        object_init_ex( return_value, NumberFormatter_ce_ptr );
-       numfmt_ctor(INTERNAL_FUNCTION_PARAM_PASSTHRU, 0);
-       if (Z_TYPE_P(return_value) == IS_OBJECT && Z_OBJ_P(return_value) == NULL) {
+       if (numfmt_ctor(INTERNAL_FUNCTION_PARAM_PASSTHRU, 0) == FAILURE) {
+               zval_ptr_dtor(return_value);
                RETURN_NULL();
        }
 }
@@ -95,8 +95,7 @@ PHP_METHOD( NumberFormatter, __construct )
 
        zend_replace_error_handling(EH_THROW, IntlException_ce_ptr, &error_handling);
        return_value = getThis();
-       numfmt_ctor(INTERNAL_FUNCTION_PARAM_PASSTHRU, 1);
-       if (Z_TYPE_P(return_value) == IS_OBJECT && Z_OBJ_P(return_value) == NULL) {
+       if (numfmt_ctor(INTERNAL_FUNCTION_PARAM_PASSTHRU, 1) == FAILURE) {
                if (!EG(exception)) {
                        zend_throw_exception(IntlException_ce_ptr, "Constructor failed", 0);
                }
index 6001aa128513cc408ec6e4f09afeed6d78a57550..8a1163904756b0cb8cbaad7a0608894b25faba87 100644 (file)
@@ -54,7 +54,7 @@ typedef struct _intl_data {
         RETURN_FALSE;                                                                                                          \
     }
 
-/* Check status in object, if error - exit */
+/* Check status in object, if error return false */
 #define INTL_METHOD_CHECK_STATUS(obj, msg)                                                                                     \
     intl_error_set_code( NULL, INTL_DATA_ERROR_CODE((obj)) );                          \
     if( U_FAILURE( INTL_DATA_ERROR_CODE((obj)) ) )                                                                     \
@@ -63,19 +63,23 @@ typedef struct _intl_data {
         RETURN_FALSE;                                                                          \
     }
 
-/* Check status, if error - destroy value and exit */
+/* Check status in object, if error return null */
+#define INTL_METHOD_CHECK_STATUS_OR_NULL(obj, msg)                                                                     \
+    intl_error_set_code( NULL, INTL_DATA_ERROR_CODE((obj)) );                                          \
+    if( U_FAILURE( INTL_DATA_ERROR_CODE((obj)) ) )                                                                     \
+    {                                                                                                                                                          \
+        intl_errors_set_custom_msg( INTL_DATA_ERROR_P((obj)), msg, 0 );                                \
+        zval_ptr_dtor(return_value);                                                                                           \
+        RETURN_NULL();                                                                                                                         \
+    }
+
+/* Check status in object, if error return FAILURE */
 #define INTL_CTOR_CHECK_STATUS(obj, msg)                                                                                       \
-    intl_error_set_code( NULL, INTL_DATA_ERROR_CODE((obj)) );                          \
+    intl_error_set_code( NULL, INTL_DATA_ERROR_CODE((obj)) );                                          \
     if( U_FAILURE( INTL_DATA_ERROR_CODE((obj)) ) )                                                                     \
     {                                                                                                                                                          \
-        intl_errors_set_custom_msg( INTL_DATA_ERROR_P((obj)), msg, 0 );        \
-               /* yes, this is ugly, but it alreay is */                                                                       \
-               if (return_value != getThis()) {                                                                                        \
-                       zval_dtor(return_value);                                                                                                \
-                       RETURN_NULL();                                                                                                                  \
-               }                                                                                                                                                       \
-               Z_OBJ_P(return_value) = NULL;                                                                                           \
-               return;                                                                                                                                         \
+        intl_errors_set_custom_msg( INTL_DATA_ERROR_P((obj)), msg, 0 );                                \
+        return FAILURE;                                                                                                                                \
     }
 
 #define INTL_METHOD_RETVAL_UTF8(obj, ustring, ulen, free_it)                                                                   \
@@ -100,14 +104,11 @@ typedef struct _intl_data {
                RETURN_NULL();                                                                                                                                  \
        }
 
-#define INTL_CHECK_LOCALE_LEN_OBJ(locale_len, object)                                                                  \
+#define INTL_CHECK_LOCALE_LEN_OR_FAILURE(locale_len)                                                                   \
        if((locale_len) > INTL_MAX_LOCALE_LEN) {                                                                                        \
                intl_error_set( NULL, U_ILLEGAL_ARGUMENT_ERROR,                                                                 \
                "Locale string too long, should be no longer than 80 characters", 0 );                  \
-               zval_dtor(object);                                                                                                                              \
-               ZVAL_NULL(object);                                                                                                                              \
-               RETURN_NULL();                                                                                                                                  \
+               return FAILURE;                                                                                                                                 \
        }
 
-
 #endif // INTL_DATA_H
index 2675aca5b8e66de8679e8c594d75ef03fcfa8ab0..e0919ec42b2442d37454ebef82c413300ec12828 100644 (file)
@@ -26,7 +26,7 @@
 #include "intl_convert.h"
 
 /* {{{ */
-static void msgfmt_ctor(INTERNAL_FUNCTION_PARAMETERS, zend_bool is_constructor)
+static int msgfmt_ctor(INTERNAL_FUNCTION_PARAMETERS, zend_bool is_constructor)
 {
        const char* locale;
        char*       pattern;
@@ -45,11 +45,10 @@ static void msgfmt_ctor(INTERNAL_FUNCTION_PARAMETERS, zend_bool is_constructor)
        {
                intl_error_set( NULL, U_ILLEGAL_ARGUMENT_ERROR,
                        "msgfmt_create: unable to parse input parameters", 0 );
-               Z_OBJ_P(return_value) = NULL;
-               return;
+               return FAILURE;
        }
 
-       INTL_CHECK_LOCALE_LEN_OBJ(locale_len, return_value);
+       INTL_CHECK_LOCALE_LEN_OR_FAILURE(locale_len);
        MSG_FORMAT_METHOD_FETCH_OBJECT_NO_CHECK;
 
        /* Convert pattern (if specified) to UTF-16. */
@@ -86,6 +85,7 @@ static void msgfmt_ctor(INTERNAL_FUNCTION_PARAMETERS, zend_bool is_constructor)
        }
 
        INTL_CTOR_CHECK_STATUS(mfo, "msgfmt_create: message formatter creation failed");
+       return SUCCESS;
 }
 /* }}} */
 
@@ -97,8 +97,8 @@ static void msgfmt_ctor(INTERNAL_FUNCTION_PARAMETERS, zend_bool is_constructor)
 PHP_FUNCTION( msgfmt_create )
 {
        object_init_ex( return_value, MessageFormatter_ce_ptr );
-       msgfmt_ctor(INTERNAL_FUNCTION_PARAM_PASSTHRU, 0);
-       if (Z_TYPE_P(return_value) == IS_OBJECT && Z_OBJ_P(return_value) == NULL) {
+       if (msgfmt_ctor(INTERNAL_FUNCTION_PARAM_PASSTHRU, 0) == FAILURE) {
+               zval_ptr_dtor(return_value);
                RETURN_NULL();
        }
 }
@@ -113,8 +113,7 @@ PHP_METHOD( MessageFormatter, __construct )
 
        zend_replace_error_handling(EH_THROW, IntlException_ce_ptr, &error_handling);
        return_value = getThis();
-       msgfmt_ctor(INTERNAL_FUNCTION_PARAM_PASSTHRU, 1);
-       if (Z_TYPE_P(return_value) == IS_OBJECT && Z_OBJ_P(return_value) == NULL) {
+       if (msgfmt_ctor(INTERNAL_FUNCTION_PARAM_PASSTHRU, 1) == FAILURE) {
                if (!EG(exception)) {
                        zend_throw_exception(IntlException_ce_ptr, "Constructor failed", 0);
                }
index a42556f746c9f35f858d4a6e7ecb1ae28ae5a463..d6b071a2494bcc37875cbde7cb70f8c33afda78d 100644 (file)
@@ -75,7 +75,7 @@ static zend_object *ResourceBundle_object_create( zend_class_entry *ce )
 /* }}} */
 
 /* {{{ ResourceBundle_ctor */
-static void resourcebundle_ctor(INTERNAL_FUNCTION_PARAMETERS, zend_bool is_constructor)
+static int resourcebundle_ctor(INTERNAL_FUNCTION_PARAMETERS, zend_bool is_constructor)
 {
        const char *bundlename;
        size_t          bundlename_len = 0;
@@ -94,11 +94,10 @@ static void resourcebundle_ctor(INTERNAL_FUNCTION_PARAMETERS, zend_bool is_const
        {
                intl_error_set( NULL, U_ILLEGAL_ARGUMENT_ERROR,
                        "resourcebundle_ctor: unable to parse input parameters", 0 );
-               Z_OBJ_P(return_value) = NULL;
-               return;
+               return FAILURE;
        }
 
-       INTL_CHECK_LOCALE_LEN_OBJ(locale_len, return_value);
+       INTL_CHECK_LOCALE_LEN_OR_FAILURE(locale_len);
 
        if (locale == NULL) {
                locale = intl_locale_get_default();
@@ -123,8 +122,10 @@ static void resourcebundle_ctor(INTERNAL_FUNCTION_PARAMETERS, zend_bool is_const
                                        rb->me, ULOC_ACTUAL_LOCALE, &INTL_DATA_ERROR_CODE(rb)));
                intl_errors_set_custom_msg(INTL_DATA_ERROR_P(rb), pbuf, 1);
                efree(pbuf);
-               Z_OBJ_P(return_value) = NULL;
+               return FAILURE;
        }
+
+       return SUCCESS;
 }
 /* }}} */
 
@@ -145,8 +146,7 @@ PHP_METHOD( ResourceBundle, __construct )
 
        zend_replace_error_handling(EH_THROW, IntlException_ce_ptr, &error_handling);
        return_value = getThis();
-       resourcebundle_ctor(INTERNAL_FUNCTION_PARAM_PASSTHRU, 1);
-       if (Z_TYPE_P(return_value) == IS_OBJECT && Z_OBJ_P(return_value) == NULL) {
+       if (resourcebundle_ctor(INTERNAL_FUNCTION_PARAM_PASSTHRU, 1) == FAILURE) {
                if (!EG(exception)) {
                        zend_throw_exception(IntlException_ce_ptr, "Constructor failed", 0);
                }
@@ -161,8 +161,8 @@ proto ResourceBundle resourcebundle_create( string $locale [, string $bundlename
 PHP_FUNCTION( resourcebundle_create )
 {
        object_init_ex( return_value, ResourceBundle_ce_ptr );
-       resourcebundle_ctor(INTERNAL_FUNCTION_PARAM_PASSTHRU, 0);
-       if (Z_TYPE_P(return_value) == IS_OBJECT && Z_OBJ_P(return_value) == NULL) {
+       if (resourcebundle_ctor(INTERNAL_FUNCTION_PARAM_PASSTHRU, 0) == FAILURE) {
+               zval_ptr_dtor(return_value);
                RETURN_NULL();
        }
 }
index 865f600dfc3e856ab46a1f61a1269b324b3a57b2..fbe7cbae1d175c098de54a13ef03e25dce2f37d5 100644 (file)
@@ -41,7 +41,7 @@ PHP_METHOD(Spoofchecker, __construct)
        SPOOFCHECKER_METHOD_FETCH_OBJECT_NO_CHECK;
 
        co->uspoof = uspoof_open(SPOOFCHECKER_ERROR_CODE_P(co));
-       INTL_CTOR_CHECK_STATUS(co, "spoofchecker: unable to open ICU Spoof Checker");
+       INTL_METHOD_CHECK_STATUS(co, "spoofchecker: unable to open ICU Spoof Checker");
 
        /* Single-script enforcement is on by default. This fails for languages
         like Japanese that legally use multiple scripts within a single word,
index 34f283b28179a6eb51d9ff39c8d1975735de2b22..ced84a0c608c53d8e963e5d17bb20decee16fb4a 100644 (file)
@@ -169,7 +169,7 @@ PHP_FUNCTION( transliterator_create_from_rules )
                str_rules, str_rules_len, TRANSLITERATOR_ERROR_CODE_P( to ) );
        /* (I'm not a big fan of non-obvious flow control macros ).
         * This one checks the error value, destroys object and returns false */
-       INTL_CTOR_CHECK_STATUS( to, "String conversion of rules to UTF-16 failed" );
+       INTL_METHOD_CHECK_STATUS_OR_NULL( to, "String conversion of rules to UTF-16 failed" );
 
        /* Open ICU Transliterator. */
        utrans = utrans_openU( id, ( sizeof( id ) - 1 ) / ( sizeof( *id ) ), (UTransDirection ) direction,
@@ -197,7 +197,7 @@ PHP_FUNCTION( transliterator_create_from_rules )
     }
        transliterator_object_construct( object, utrans, TRANSLITERATOR_ERROR_CODE_P( to ) );
        /* no need to close the transliterator manually on construction error */
-       INTL_CTOR_CHECK_STATUS( to, "transliterator_create_from_rules: internal constructor call failed" );
+       INTL_METHOD_CHECK_STATUS_OR_NULL( to, "transliterator_create_from_rules: internal constructor call failed" );
 }
 /* }}} */
 
@@ -227,11 +227,11 @@ PHP_FUNCTION( transliterator_create_inverse )
        TRANSLITERATOR_METHOD_FETCH_OBJECT_NO_CHECK; /* change "to" into new object (from "object" ) */
 
        utrans = utrans_openInverse( to_orig->utrans, TRANSLITERATOR_ERROR_CODE_P( to ) );
-       INTL_CTOR_CHECK_STATUS( to, "transliterator_create_inverse: could not create "
+       INTL_METHOD_CHECK_STATUS_OR_NULL( to, "transliterator_create_inverse: could not create "
                "inverse ICU transliterator" );
        transliterator_object_construct( object, utrans, TRANSLITERATOR_ERROR_CODE_P( to ) );
        /* no need to close the transliterator manually on construction error */
-       INTL_CTOR_CHECK_STATUS( to, "transliterator_create: internal constructor call failed" );
+       INTL_METHOD_CHECK_STATUS_OR_NULL( to, "transliterator_create: internal constructor call failed" );
 }
 /* }}} */