? Pending

User tests: Successful: Unsuccessful:

avatar Quy
Quy
5 Jul 2017

Pull Request for Issue #16970 .
Fix regression for Issue #15228 .

Summary of Changes

Revert to previous individual preg_match rather than combine them into 1.

Testing Instructions

Check your PHP error log.

or

Create the following PHP script and run it. 1st preg_match( (previous) outputs no error. 2nd preg_match( (current) outputs error.

<?php

$agent = "Mozilla/5.0 (iPad; CPU OS 10_3_2 like Mac OS X) AppleWebKit/603.1.30 (KHTML, like Gecko) CriOS/59.0.3071.102 Mobile/14F89 Safari/602.1";

echo 'v3.6.5 ';
preg_match('|CriOS[/ ]([0-9.]+)|', $agent, $version);
var_dump($version);
list ($majorVersion, $minorVersion) = explode('.', $version[1]);

echo 'v3.7.3 ';
preg_match('/Chrome[\/ ]([0-9.]+)|CrMo[\/ ]([0-9.]+)|CriOS[\/ ]([0-9.]+)/i', $agent, $version);
var_dump($version);
list ($majorVersion, $minorVersion) = explode('.', $version[1]);

?>

results (1st array is v3.6.5 and 2nd array is v3.7.3):

v3.6.5 array(2) {
  [0]=>
  string(19) "CriOS/59.0.3071.102"
  [1]=>
  string(13) "59.0.3071.102"
}
v3.7.3 array(4) {
  [0]=>
  string(19) "CriOS/59.0.3071.102"
  [1]=>
  string(0) ""
  [2]=>
  string(0) ""
  [3]=>
  string(13) "59.0.3071.102"
}
<br />
<b>Notice</b>:  Undefined offset: 1 in <b>C:\xampp\htdocs\agent.php</b> on line <b>14</b><br />

Expected result

No PHP Notice

Actual result

PHP message: PHP Notice: Undefined offset: 1 in /libraries/joomla/environment/browser.php on line 282\n

Documentation Changes Required

None

Votes

# of Users Experiencing Issue
1/1
Average Importance Score
3.00

avatar Quy Quy - open - 5 Jul 2017
avatar Quy Quy - change - 5 Jul 2017
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 5 Jul 2017
Category Libraries
avatar Quy Quy - change - 6 Jul 2017
The description was changed
avatar Quy Quy - edited - 6 Jul 2017
avatar Dmitry2003
Dmitry2003 - comment - 6 Jul 2017

omg code


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16989.

avatar Quy Quy - change - 10 Jul 2017
The description was changed
avatar Quy Quy - edited - 10 Jul 2017
avatar Quy Quy - change - 10 Jul 2017
Title
Fix undefined offset: 1
Regression: Fix undefined offset: 1
avatar Quy Quy - edited - 10 Jul 2017
avatar Quy
Quy - comment - 11 Jul 2017

@rdeutz Please consider for v3.7.4.
@photodude Please test/confirm.

Thanks.

avatar rdeutz
rdeutz - comment - 11 Jul 2017

needs to be RTC next week Tuesday

avatar photodude
photodude - comment - 11 Jul 2017

Confirmed via the provided user agent string
https://3v4l.org/KVdRU

Reverting to previous is an option

The current preg_match pattern is apparently returning on 0 and 3 with 1 and 2 being empty, whereas the old preg_match is returning on 0 and 1. This is apparently due to the position in the grouped pattern (since in the grouped pattern CriOS is the 3rd option it's version number is in the 3rd array position).

There is probably a way to resolve this version position using the grouped version, but likely simpler to revert the groupings where the version is expected at array position 1 (as is done in this PR)

avatar photodude
photodude - comment - 11 Jul 2017

Here is the correct correction to the existing single preg_match pattern
/Chrome|CrMo|CriOS[\/ ]([0-9.]+)/i

Correction example
https://3v4l.org/WSctl

avatar Quy Quy - change - 11 Jul 2017
Labels Added: ?
avatar photodude
photodude - comment - 11 Jul 2017

you are welcome @Quy Thanks for catching this oversite and making the corrections.

avatar Quy
Quy - comment - 15 Jul 2017

I couldn't resolve the conflict. Please test this PR #17139 instead. Thanks! @photodude

avatar Quy Quy - change - 15 Jul 2017
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2017-07-15 15:57:26
Closed_By Quy
avatar Quy Quy - close - 15 Jul 2017
avatar Brian5600
Brian5600 - comment - 29 Jul 2017

Hello
I tried to change browser.php today like PR #17139, and the error went away from Chrome browser on Ipad (Ios). I am on Joomla 3.7.4.
Hope it will be fixed in Jooomla.
Thanks @Brian5600.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16989.

avatar brianteeman
brianteeman - comment - 29 Jul 2017

@Brian5600 please report your test on #17139

Add a Comment

Login with GitHub to post a comment