Fix case-sensitivity in discovery #134
Conversation
Auth/OpenID/Parse.php
Outdated
| return true; | ||
| } | ||
|
|
||
| if ( !is_callable('mb_ereg_search_init')) { |
There was a problem hiding this comment.
There's no need for this block now. There was never a need for it since preg_match replaced mb_ereg_search a while ago. Just delete this block and replace it with return false;
There was a problem hiding this comment.
Also, since you're basically short circuiting the whole method, you can probably just kill lines 229 - 235. If preg_match works as expected.
| if (!preg_match($regexp, $text, $match)) { | ||
| return false; | ||
| } | ||
| $match = $match[0]; |
There was a problem hiding this comment.
I'm concerned with removing this. preg_match returns an array the first element is the whole line, the second element is the text capture from $regexp. The purpose of this method is to just return the first line.
There was a problem hiding this comment.
Did my comment #134 (comment) address your concern ? You can check by yourself that it is expected that the match function returns an array, at least from the function used to extract links (openid rel) from the page.
|
I *needed* this change to support my OpenID address
I gladyly simplified the code to drop the mb_ereg_search_init part
so now it's straightforward (see a3d0943).
About match array. I needed that change because otherwise match[0],
used by the caller here:
```
if (!preg_match_all($this->_link_find, $head_match[0],
$link_matches)) {
ini_set( 'pcre.backtrack_limit', $old_btlimit );
return array();
}
```
Was getting a single character.
I guess it wasn't spotted before becuase basically preg_match was never
used, and the mb_ereg_search return code was different...
|
|
@marcoceppi did I not address your concern ? I'm using this change in production with PHP-5.6 in case it helps accepting it (btw: no CI keeping an eye on the code?) |
|
For the record: this change was also proposed downstream to GNUSocial (after successfully running it in production for some time now): https://git.gnu.io/gnu/gnu-social/merge_requests/140 |
|
@marcoceppi Wordpress plugin is also affected (surely using your library): diso/wordpress-openid#48 |
Closes #60 Equivalent change was proposed upstream: openid/php-openid#134
|
@marcoceppi hello ? |
|
thanks |
Closes #60 Equivalent change was proposed upstream: openid/php-openid#134
Fixes #133