Consistently handle out of bounds offsets in iconv_strpos()

Same as in all other strpos() style functions, throw ValueError on
out of bounds offset.

diff --git a/ext/iconv/iconv.c b/ext/iconv/iconv.c
index 89f2ffb37fdfe4f73254a42aa79663b5c69d7160..84f24baabbde7986820a8ba4c3ab20d5ff850be8 100644 (file)
--- a/ext/iconv/iconv.c
+++ b/ext/iconv/iconv.c
@@ -884,6 +884,10 @@ static php_iconv_err_t _php_iconv_strpos(size_t *pretval,
 
        iconv_close(cd);
 
+       if (err == PHP_ICONV_ERR_SUCCESS && offset > cnt) {
+               return PHP_ICONV_ERR_OUT_BY_BOUNDS;
+       }
+
        return err;
 }
 /* }}} */
@@ -1764,6 +1768,10 @@ static void _php_iconv_show_error(php_iconv_err_t err, const char *out_charset,
                        php_error_docref(NULL, E_WARNING, "Malformed string");
                        break;
 
+               case PHP_ICONV_ERR_OUT_BY_BOUNDS:
+                       zend_argument_value_error(3, "must be contained in argument #1 ($haystack)");
+                       break;
+
                default:
                        /* other error */
                        php_error_docref(NULL, E_NOTICE, "Unknown error (%d)", errno);
@@ -1881,12 +1889,13 @@ PHP_FUNCTION(iconv_strpos)
                }
                offset += haystk_len;
                if (offset < 0) { /* If offset before start */
-                       php_error_docref(NULL, E_WARNING, "Offset not contained in string.");
-                       RETURN_FALSE;
+                       zend_argument_value_error(3, "must be contained in argument #1 ($haystack)");
+                       RETURN_THROWS();
                }
        }
 
        if (ZSTR_LEN(ndl) < 1) {
+               // TODO: Support empty needles!
                RETURN_FALSE;
        }
 

diff --git a/ext/iconv/php_iconv.h b/ext/iconv/php_iconv.h
index 89506fab530b291beb7cfec2b0b1d381e6bf9f95..901ba440fd683cd74ed86b5ca7b8ec696172fc13 100644 (file)
--- a/ext/iconv/php_iconv.h
+++ b/ext/iconv/php_iconv.h
@@ -87,7 +87,8 @@ typedef enum _php_iconv_err_t {
        PHP_ICONV_ERR_ILLEGAL_CHAR      = 5,
        PHP_ICONV_ERR_UNKNOWN           = 6,
        PHP_ICONV_ERR_MALFORMED         = 7,
-       PHP_ICONV_ERR_ALLOC             = 8
+       PHP_ICONV_ERR_ALLOC             = 8,
+       PHP_ICONV_ERR_OUT_BY_BOUNDS     = 9
 } php_iconv_err_t;
 /* }}} */
 

diff --git a/ext/iconv/tests/iconv_strpos.phpt b/ext/iconv/tests/iconv_strpos.phpt
index 1dafd6c06b482ca396ec3c2f3ec1431cda5f3be5..df9427d2c621b185e203127f264738a9b39ea5b5 100644 (file)
--- a/ext/iconv/tests/iconv_strpos.phpt
+++ b/ext/iconv/tests/iconv_strpos.phpt
@@ -16,14 +16,19 @@ function foo($haystk, $needle, $offset, $to_charset = false, $from_charset = fal
     } catch (ValueError $exception) {
         echo $exception->getMessage() . "\n";
     }
-    if ($to_charset !== false) {
-        var_dump(iconv_strpos($haystk, $needle, $offset, $to_charset));
-    } else {
-        var_dump(iconv_strpos($haystk, $needle, $offset));
+    try {
+        if ($to_charset !== false) {
+            var_dump(iconv_strpos($haystk, $needle, $offset, $to_charset));
+        } else {
+            var_dump(iconv_strpos($haystk, $needle, $offset));
+        }
+    } catch (ValueError $exception) {
+        echo $exception->getMessage() . "\n";
     }
 }
 foo("abecdbcdabef", "bcd", -1);
 foo("abecdbcdabef", "bcd", -7);
+foo("abecdbcdabef", "bcd", 12);
 foo("abecdbcdabef", "bcd", 100000);
 foo("abcabcabcdabcababcdabc", "bcd", 0);
 foo("abcabcabcdabcababcdabc", "bcd", 10);
@@ -41,8 +46,10 @@ bool(false)
 bool(false)
 int(5)
 int(5)
-strpos(): Argument #3 ($offset) must be contained in argument #1 ($haystack)
 bool(false)
+bool(false)
+strpos(): Argument #3 ($offset) must be contained in argument #1 ($haystack)
+iconv_strpos(): Argument #3 ($offset) must be contained in argument #1 ($haystack)
 int(7)
 int(7)
 int(16)

diff --git a/ext/iconv/tests/iconv_strpos_variation5.phpt b/ext/iconv/tests/iconv_strpos_variation5.phpt
index 4feef545a4299dfc772571cb9f3f40b134c8cdce..625c44efa7e50ff883651b157c4c1f60d4c5a5e5 100644 (file)
--- a/ext/iconv/tests/iconv_strpos_variation5.phpt
+++ b/ext/iconv/tests/iconv_strpos_variation5.phpt
@@ -32,25 +32,29 @@ $needle_mb = base64_decode('44CC');
 for ($i = -30; $i <= 60; $i += 10) {
     echo "\n**-- Offset is: $i --**\n";
     echo "-- ASCII String --\n";
-    var_dump(iconv_strpos($string_ascii, $needle_ascii, $i));
+    try {
+        var_dump(iconv_strpos($string_ascii, $needle_ascii, $i));
+    } catch (ValueError $e) {
+        echo $e->getMessage(), "\n";
+    }
     echo "--Multibyte String --\n";
-    var_dump(iconv_strpos($string_mb, $needle_mb, $i, 'UTF-8'));
+    try {
+        var_dump(iconv_strpos($string_mb, $needle_mb, $i, 'UTF-8'));
+    } catch (ValueError $e) {
+        echo $e->getMessage(), "\n";
+    }
 }
 
 echo "Done";
 ?>
---EXPECTF--
+--EXPECT--
 *** Testing iconv_strpos() : usage variations ***
 
 **-- Offset is: -30 --**
 -- ASCII String --
-
-Warning: iconv_strpos(): Offset not contained in string. in %s on line %d
-bool(false)
+iconv_strpos(): Argument #3 ($offset) must be contained in argument #1 ($haystack)
 --Multibyte String --
-
-Warning: iconv_strpos(): Offset not contained in string. in %s on line %d
-bool(false)
+iconv_strpos(): Argument #3 ($offset) must be contained in argument #1 ($haystack)
 
 **-- Offset is: -20 --**
 -- ASCII String --
@@ -84,25 +88,25 @@ int(20)
 
 **-- Offset is: 30 --**
 -- ASCII String --
-bool(false)
+iconv_strpos(): Argument #3 ($offset) must be contained in argument #1 ($haystack)
 --Multibyte String --
-bool(false)
+iconv_strpos(): Argument #3 ($offset) must be contained in argument #1 ($haystack)
 
 **-- Offset is: 40 --**
 -- ASCII String --
-bool(false)
+iconv_strpos(): Argument #3 ($offset) must be contained in argument #1 ($haystack)
 --Multibyte String --
-bool(false)
+iconv_strpos(): Argument #3 ($offset) must be contained in argument #1 ($haystack)
 
 **-- Offset is: 50 --**
 -- ASCII String --
-bool(false)
+iconv_strpos(): Argument #3 ($offset) must be contained in argument #1 ($haystack)
 --Multibyte String --
-bool(false)
+iconv_strpos(): Argument #3 ($offset) must be contained in argument #1 ($haystack)
 
 **-- Offset is: 60 --**
 -- ASCII String --
-bool(false)
+iconv_strpos(): Argument #3 ($offset) must be contained in argument #1 ($haystack)
 --Multibyte String --
-bool(false)
+iconv_strpos(): Argument #3 ($offset) must be contained in argument #1 ($haystack)
 Done