fix Android tablet detection in VERSION_TRUNCATION_MAJOR mode

matomo-org · May 29, 2023 · 49ca643 · 49ca643
1 parent cc4eb16
commit 49ca643
Show file tree

Hide file tree

Showing 6 changed files with 84 additions and 14 deletions.
diff --git a/DeviceDetector.php b/DeviceDetector.php
@@ -980,10 +980,10 @@ protected function parseDevice(): void
          * Devices running Android 3.X are tablets. Device type of Android 2.X and 4.X+ are unknown
          */
         if (null === $this->device && 'Android' === $osName && '' !== $osVersion) {
-            if (-1 === \version_compare($osVersion, '2.0')) {
+            if (-1 === \version_compare($osVersion, '2')) {
                 $this->device = AbstractDeviceParser::DEVICE_TYPE_SMARTPHONE;
-            } elseif (\version_compare($osVersion, '3.0') >= 0
-                && -1 === \version_compare($osVersion, '4.0')
+            } elseif (\version_compare($osVersion, '3') >= 0
+                && -1 === \version_compare($osVersion, '4')
             ) {
                 $this->device = AbstractDeviceParser::DEVICE_TYPE_TABLET;
             }

diff --git a/Parser/AbstractParser.php b/Parser/AbstractParser.php
@@ -356,6 +356,18 @@ protected function buildVersion(string $versionString, array $matches): string
         $versionString = $this->buildByMatch($versionString, $matches);
         $versionString = \str_replace('_', '.', $versionString);
 
+        return \trim($versionString, ' .');
+    }
+
+    /**
+     * Truncate the version based on the current truncation rule
+     *
+     * @param string $versionString
+     *
+     * @return string
+     */
+    protected function truncateVersion(string $versionString): string
+    {
         if (self::VERSION_TRUNCATION_NONE !== static::$maxMinorParts
             && \substr_count($versionString, '.') > static::$maxMinorParts
         ) {

diff --git a/Parser/Client/AbstractClientParser.php b/Parser/Client/AbstractClientParser.php
@@ -53,7 +53,7 @@ public function parse(): ?array
                     $result = [
                         'type'    => $this->parserName,
                         'name'    => $this->buildByMatch($regex['name'], $matches),
-                        'version' => $this->buildVersion((string) $regex['version'], $matches),
+                        'version' => $this->truncateVersion($this->buildVersion((string) $regex['version'], $matches)),
                     ];
 
                     break;

diff --git a/Parser/Client/Browser.php b/Parser/Client/Browser.php
@@ -917,7 +917,7 @@ protected function parseBrowserFromClientHints(): array
         return [
             'name'       => $name,
             'short_name' => $short,
-            'version'    => $this->buildVersion($version, []),
+            'version'    => $this->truncateVersion($this->buildVersion($version, [])),
         ];
     }
 
@@ -952,8 +952,9 @@ protected function parseBrowserFromUserAgent(): array
         $browserShort = self::getBrowserShortName($name);
 
         if (null !== $browserShort) {
-            $version       = $this->buildVersion((string) $regex['version'], $matches);
-            $engine        = $this->buildEngine($regex['engine'] ?? [], $version);
+            $fullVersion   = $this->buildVersion((string) $regex['version'], $matches);
+            $version       = $this->truncateVersion($fullVersion);
+            $engine        = $this->buildEngine($regex['engine'] ?? [], $fullVersion);
             $engineVersion = $this->buildEngineVersion($engine);
 
             return [

diff --git a/Parser/OperatingSystem.php b/Parser/OperatingSystem.php
@@ -450,7 +450,7 @@ protected function parseOsFromClientHints(): array
         return [
             'name'       => $name,
             'short_name' => $short,
-            'version'    => $this->buildVersion($version, []),
+            'version'    => $this->truncateVersion($this->buildVersion($version, [])),
         ];
     }
 
@@ -479,7 +479,7 @@ protected function parseOsFromUserAgent(): array
             ['name' => $name, 'short' => $short] = self::getShortOsData($name);
 
             $version = \array_key_exists('version', $osRegex)
-                ? $this->buildVersion((string) $osRegex['version'], $matches)
+                ? $this->truncateVersion($this->buildVersion((string) $osRegex['version'], $matches))
                 : '';
 
             foreach ($osRegex['versions'] ?? [] as $regex) {
@@ -495,7 +495,7 @@ protected function parseOsFromUserAgent(): array
                 }
 
                 if (\array_key_exists('version', $regex)) {
-                    $version = $this->buildVersion((string) $regex['version'], $matches);
+                    $version = $this->truncateVersion($this->buildVersion((string) $regex['version'], $matches));
                 }
 
                 break;

diff --git a/Tests/DeviceDetectorTest.php b/Tests/DeviceDetectorTest.php
@@ -230,7 +230,7 @@ public function testParse(array $fixtureData): void
         }
 
         $errorMessage = \sprintf(
-            "UserAgent: %s\nHeaders: %s",
+            "UserAgent: %s\nHeaders: %s\nVersion truncation: none",
             $ua,
             \print_r($fixtureData['headers'] ?? null, true)
         );
@@ -240,6 +240,48 @@ public function testParse(array $fixtureData): void
         $this->assertEquals($fixtureData, $uaInfo, $errorMessage);
     }
 
+    /**
+     * @dataProvider getFixtures
+     */
+    public function testParseWithVersionTruncationMajor(array $fixtureData): void
+    {
+        $ua          = $fixtureData['user_agent'];
+        $clientHints = !empty($fixtureData['headers']) ? ClientHints::factory($fixtureData['headers']) : null;
+
+        AbstractDeviceParser::setVersionTruncation(AbstractDeviceParser::VERSION_TRUNCATION_MAJOR);
+
+        try {
+            $uaInfo = DeviceDetector::getInfoFromUserAgent($ua, $clientHints);
+        } catch (\Exception $exception) {
+            throw new \Exception(
+                \sprintf('Error: %s from useragent %s', $exception->getMessage(), $ua),
+                $exception->getCode(),
+                $exception
+            );
+        }
+
+        $errorMessage = \sprintf(
+            "UserAgent: %s\nHeaders: %s\nVersion truncation: major",
+            $ua,
+            \print_r($fixtureData['headers'] ?? null, true)
+        );
+
+        unset($fixtureData['headers']); // ignore headers in result
+
+        // truncate versions
+        if (\array_key_exists('version', $fixtureData['os'] ?? [])) {
+            $fixtureData['os']['version'] = self::truncateVersion($fixtureData['os']['version']);
+        }
+
+        if (\array_key_exists('version', $fixtureData['client'] ?? [])) {
+            $fixtureData['client']['version'] = self::truncateVersion($fixtureData['client']['version']);
+        }
+
+        $this->assertEquals($fixtureData, $uaInfo, $errorMessage);
+
+        AbstractDeviceParser::setVersionTruncation(AbstractDeviceParser::VERSION_TRUNCATION_NONE);
+    }
+
     public function getFixtures(): array
     {
         $fixtures     = [];
@@ -613,7 +655,7 @@ public function testSetYamlParserInvalid(): void
     public function testSetYamlParser(): void
     {
         $reader = function & ($object, $property) {
-            $value = & Closure::bind(function & () use ($property) {
+            $value = &Closure::bind(function & () use ($property) {
                 return $this->$property;
             }, $object, $object)->__invoke();
 
@@ -630,15 +672,15 @@ public function testSetYamlParser(): void
 
         foreach ($dd->getClientParsers() as $parser) {
             if ($parser instanceof MobileApp) {
-                $appHints = & $reader($parser, 'appHints');
+                $appHints = &$reader($parser, 'appHints');
                 $this->assertInstanceOf(Symfony::class, $appHints->getYamlParser());
             }
 
             if (!($parser instanceof Browser)) {
                 continue;
             }
 
-            $browserHints = & $reader($parser, 'browserHints');
+            $browserHints = &$reader($parser, 'browserHints');
             $this->assertInstanceOf(Symfony::class, $browserHints->getYamlParser());
         }
     }
@@ -741,4 +783,19 @@ private function getDeviceDetector(): DeviceDetector
 
         return $dd;
     }
+
+    private static function truncateVersion(?string $versionString): ?string
+    {
+        if (null === $versionString) {
+            return null;
+        }
+
+        if (\substr_count($versionString, '.') > 0) {
+            $versionParts  = \explode('.', $versionString);
+            $versionParts  = \array_slice($versionParts, 0, 1);
+            $versionString = \implode('.', $versionParts);
+        }
+
+        return \trim($versionString, ' .');
+    }
 }