php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Doc Bug #73913 broken strrpos with negative offset
Submitted: 2017-01-11 17:16 UTC Modified: 2017-01-13 15:46 UTC
Votes:2
Avg. Score:3.0 ± 0.0
Reproduced:1 of 2 (50.0%)
Same Version:1 (100.0%)
Same OS:0 (0.0%)
From: proartex at mail dot ru Assigned: heiglandreas (profile)
Status: Assigned Package: Strings related
PHP Version: 5.6.29 OS: macOS Sierra 10.12.1
Private report: No CVE-ID: None
Have you experienced this issue?
Rate the importance of this bug to you:

 [2017-01-11 17:16 UTC] proartex at mail dot ru
Description:
------------
It is seems broken at least in the following cases:
 - if negative offset equals to haystack length;
 - if negative offset less than needle length;
 - if needle length equals to (haystack length + negative offset)

Negative offset should not move pointer but reduce search scope on the value of offset. In that case character placed at 0 position must not be found if search scope is reduced to empty string. Same should be applied in boundary cases: character placed next to reduced search scope must not be found (last case).

Test script:
---------------
<?php

var_dump(strrpos("aaaa", 'aa', -2)); //ER: 0, AR: 2
var_dump(strrpos("aaaa", 'a', -1)); //ER: 2, AR: 3
var_dump(strrpos("/documents/show/5474", '/', -20)); //ER: false, AR: 0
var_dump(strrpos("works_like_a_charm", 'charm', -3)); //ER: false, AR: 13
var_dump(strrpos("works_like_a_charm", 'charm', -4)); //ER: false, AR: 13

Expected result:
----------------
0
2
false
false
false

Actual result:
--------------
2
3
0
13
13

Patches

Add a Patch

Pull Requests

Pull requests:

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2017-01-11 17:19 UTC] proartex at mail dot ru
last case must be var_dump(strrpos("works_like_a_charm", 'charm', -5)); //ER: false, AR: 13
 [2017-01-11 17:32 UTC] heiglandreas@php.net
-Status: Open +Status: Not a bug
 [2017-01-11 17:32 UTC] heiglandreas@php.net
when offset is negative the search *starts* that many characters *from the end*. The search is not excluding the last n characters. 

strrpos("aaaa", 'aa', -2) == strrpos("aaaa", 'aa', 2)
strrpos("aaaa", 'a', -1) == strrpos("aaaa", 'a', 3)
strrpos("/documents/show/5474", '/', -20) == strrpos("/documents/show/5474", '/', 0)
strrpos("works_like_a_charm", 'charm', -3) == strrpos("works_like_a_charm", 'charm', 15)
strrpos("works_like_a_charm", 'charm', -4) == strrpos("works_like_a_charm", 'charm', 14)


So they work as expected. 

What you want to do would be achieved by this:

strrpos(substr("aaaa", -2), "aa");

When needle is longer than the remaining string, the offset seems to be expanded to at least include the needle.

Note also the comment on http://php.net/manual/de/function.strrpos.php#76447
 [2017-01-11 18:03 UTC] proartex at mail dot ru
1. Does strrpos("works_like_a_charm", 'charm', 15) really equals to strrpos("works_like_a_charm", 'charm', -3)? false != 13. It is also applicable for 3 last cases.
 [2017-01-11 21:57 UTC] heiglandreas@php.net
-Status: Not a bug +Status: Open
 [2017-01-11 21:57 UTC] heiglandreas@php.net
Looks like you're right! I dug deeper into it and it seems there is an issue when the negative offset is smaller or equals the size of the needle.

I'm digging deeper into it.
 [2017-01-12 07:47 UTC] heiglandreas@php.net
-Assigned To: +Assigned To: heiglandreas
 [2017-01-12 10:33 UTC] dima at virtuman dot com
Looks like the problem is related to https://github.com/php/php-src/blob/PHP-5.6.29/ext/standard/string.c#L1987

replace this one:
e = haystack + haystack_len + offset;

with this one:
e = haystack + haystack_len + offset - needle_len;

will solve the problem.
 [2017-01-12 11:30 UTC] dima at virtuman dot com
my previous comment is not absolutely right. I'm not sure why those conditions are needed:

if (needle_len > -offset) {
	e = haystack + haystack_len - needle_len;
} else {
	e = haystack + haystack_len + offset;
}

In all cases we need to subtract absolute value of offset from e pointer and also subtract value of needle length. There is no reason to check needle_len > -offset
 [2017-01-13 15:46 UTC] heiglandreas@php.net
-Status: Assigned +Status: Open -Type: Bug +Type: Documentation Problem
 [2017-01-13 15:46 UTC] heiglandreas@php.net
The current implementation is mathematically correct and is the same as f.e. the implementation in Java and other programming languages. The docs are misleading here.

The negative offset specifies the last offset in the string the needle can start. Therefore the results are correct.

Though the documentation of that feature needs some work to explicitly state that and make that more clear.
 [2017-01-13 16:28 UTC] dima at virtuman dot com
Java implementation has no negative fromIndex for "public int lastIndexOf(String str, int fromIndex)". Java implementation returns "-1" for any negative fromIndex.
There is an examples from python:
"
>>> 'aaaa'.rfind('a',0, -2)
1
>>> 'aaaa'.rfind('a', 0, -1)
2
>>> '/documents/show/5474'.rfind('/', 0, -20)
-1
>>> 'works_like_a_charm'.rfind('charm', 0, -3)
-1
>>> 'works_like_a_charm'.rfind('charm', 0, -5)
-1
"

It works the way we expect
 
PHP Copyright © 2001-2018 The PHP Group
All rights reserved.
Last updated: Tue Jul 17 11:01:54 2018 UTC