Issue #2505579 by dpi, NivethaSubramaniyan, smustgrave, ian.mahoney, amwhalen,...

Issue #2505579 by dpi, NivethaSubramaniyan, smustgrave, ian.mahoney, amwhalen, Medha Kumari, Chi: Improve handling of invalid input in time zone abbreviation to TZID route

(cherry picked from commit 6cdc5614)

core/modules/system/src/Controller/TimezoneController.php

@@ -5,33 +5,48 @@
 use Symfony\Component\HttpFoundation\JsonResponse;
 
 /**
- * Provides a callback for finding out a timezone name.
+ * Provides a callback for finding a time zone identifier.
  */
 class TimezoneController {
 
   /**
-   * Retrieve a JSON object containing a time zone name given a timezone
-   * abbreviation.
+   * Returns a time zone identifier given a time zone abbreviation.
    *
    * @param string $abbreviation
    *   Time zone abbreviation.
    * @param int $offset
    *   Offset from GMT in seconds. Defaults to -1 which means that first found
-   *   time zone corresponding to abbr is returned. Otherwise exact offset is
-   *   searched and only if not found then the first time zone with any offset
-   *   is returned.
-   * @param null|bool $is_daylight_saving_time
-   *   Daylight saving time indicator. If abbr does not exist then the time
-   *   zone is searched solely by offset and isdst.
+   *   time zone corresponding to abbreviation is returned. Otherwise exact
+   *   offset is searched and only if not found then the first time zone with
+   *   any offset is returned.
+   * @param null|int $is_daylight_saving_time
+   *   Daylight saving time indicator. If abbreviation does not exist then the
+   *   time zone is searched solely by offset and is DST.
    *
    * @return \Symfony\Component\HttpFoundation\JsonResponse
-   *   The timezone name in JsonResponse object.
+   *   The time zone identifier or 'false' in JsonResponse object.
    */
   public function getTimezone($abbreviation = '', $offset = -1, $is_daylight_saving_time = NULL) {
+    $offset = intval($offset);
+    // Out of bounds check for offset. Offset +/- UTC is typically no
+    // smaller/larger than -12/+14.
+    if ($offset < -60000 || $offset > 60000) {
+      return new JsonResponse(FALSE);
+    }
+
+    if (isset($is_daylight_saving_time)) {
+      $original = intval($is_daylight_saving_time);
+      $is_daylight_saving_time = min(1, max(-1, intval($is_daylight_saving_time)));
+      // Catch if out of boundary.
+      if ($original !== $is_daylight_saving_time) {
+        return new JsonResponse(FALSE);
+      }
+    }
+
     // An abbreviation of "0" passed in the callback arguments should be
     // interpreted as the empty string.
     $abbreviation = $abbreviation ? $abbreviation : '';
-    $timezone = timezone_name_from_abbr($abbreviation, intval($offset), $is_daylight_saving_time);
+    $timezone = timezone_name_from_abbr($abbreviation, $offset, $is_daylight_saving_time);
     return new JsonResponse($timezone);
   }
 

core/modules/system/system.routing.yml

@@ -455,6 +455,9 @@ system.timezone:
     is_daylight_saving_time: NULL
   requirements:
     _access: 'TRUE'
+    abbreviation: '0|([A-Z]{3,5})'
+    offset: '\-?\d+'
+    is_daylight_saving_time: '\-1|0|1'
 
 system.admin_config:
   path: '/admin/config'

core/modules/system/tests/src/Functional/Datetime/DrupalDateTimeTest.php

@@ -31,20 +31,6 @@ protected function setUp(): void {
 
   }
 
-  /**
-   * Tests that the AJAX Timezone Callback can deal with various formats.
-   */
-  public function testSystemTimezone() {
-    $options = [
-      'query' => [
-        'date' => 'Tue+Sep+17+2013+21%3A35%3A31+GMT%2B0100+(BST)#',
-      ],
-    ];
-    // Query the AJAX Timezone Callback with a long-format date.
-    $response = $this->drupalGet('system/timezone/BST/3600/1', $options);
-    $this->assertEquals('"Europe\\/London"', $response, 'Timezone AJAX callback successfully identifies and responds to a long-format date.');
-  }
-
   /**
    * Tests that DrupalDateTime can detect the right timezone to use.
    *

core/modules/system/tests/src/Functional/Datetime/TimeZoneAbbreviationRouteTest.php

+<?php
+
+namespace Drupal\Tests\system\Functional\Datetime;
+
+use Drupal\Tests\BrowserTestBase;
+
+// cspell:ignore ABCDEFGHIJK
+
+/**
+ * Tests converting JavaScript time zone abbreviations to time zone identifiers.
+ *
+ * @group Datetime
+ */
+class TimeZoneAbbreviationRouteTest extends BrowserTestBase {
+
+  /**
+   * {@inheritdoc}
+   */
+  protected static $modules = ['system'];
+
+  /**
+   * {@inheritdoc}
+   */
+  protected $defaultTheme = 'stark';
+
+  /**
+   * Test that the AJAX Timezone Callback can deal with various formats.
+   */
+  public function testSystemTimezone() {
+    $options = [
+      'query' => [
+        'date' => 'Tue+Sep+17+2013+21%3A35%3A31+GMT%2B0100+(BST)#',
+      ],
+    ];
+    // Query the AJAX Timezone Callback with a long-format date.
+    $response = $this->drupalGet('system/timezone/BST/3600/1', $options);
+    $this->assertEquals($response, '"Europe\/London"');
+  }
+
+  /**
+   * Test the AJAX Timezone Callback with invalid inputs.
+   *
+   * @param string $path
+   *   Path to call.
+   * @param string|null $expectedResponse
+   *   Expected response, or NULL if expecting error.
+   * @param bool $expectInvalidRequest
+   *   Whether to expect the request is invalid.
+   *
+   * @dataProvider providerAbbreviationConversion
+   */
+  public function testAbbreviationConversion($path, $expectedResponse = NULL, $expectInvalidRequest = FALSE) {
+    $response = $this->drupalGet('system/timezone/' . $path);
+    if (isset($expectedResponse)) {
+      $this->assertEquals($response, $expectedResponse);
+    }
+    $this->assertSession()->statusCodeEquals($expectInvalidRequest ? 404 : 200);
+  }
+
+  /**
+   * Provides test data for testGet().
+   *
+   * @return array
+   *   Test scenarios.
+   */
+  public function providerAbbreviationConversion() {
+    return [
+      'valid, default offset' => [
+        'CST/0/0',
+        '"America\/Chicago"',
+      ],
+      // This should be the same TZID as default value.
+      'valid, default, explicit' => [
+        'CST/-1/0',
+        '"America\/Chicago"',
+      ],
+      // Same abbreviation but different offset.
+      'valid, default, alternative offset' => [
+        'CST/28800/0',
+        '"Asia\/Chongqing"',
+      ],
+      // Using '0' as offset will get the best matching time zone for an offset.
+      'valid, no abbreviation, offset, no DST' => [
+        '0/3600/0',
+        '"Europe\/Paris"',
+      ],
+      'valid, no abbreviation, offset, with DST' => [
+        '0/3600/1',
+        '"Europe\/London"',
+      ],
+      'invalid, unknown abbreviation' => [
+        'foo/0/0',
+        NULL,
+        FALSE,
+      ],
+      'invalid abbreviation, out of range (short)' => [
+        'A',
+        NULL,
+        TRUE,
+      ],
+      'invalid abbreviation, out of range (long)' => [
+        'ABCDEFGHIJK',
+        NULL,
+        TRUE,
+      ],
+      'invalid offset, non integer' => [
+        'CST/foo',
+        NULL,
+        TRUE,
+      ],
+      'invalid offset, out of range (lower)' => [
+        'CST/-100000',
+        'false',
+      ],
+      'invalid offset, out of range (higher)' => [
+        'CST/100000',
+        'false',
+      ],
+      'invalid DST value' => [
+        'CST/3600/blah',
+        NULL,
+        TRUE,
+      ],
+      'invalid DST value, out of range (lower)' => [
+        'CST/3600/-2',
+        NULL,
+        TRUE,
+      ],
+      'invalid DST value, out of range (higher)' => [
+        'CST/3600/2',
+        NULL,
+        TRUE,
+      ],
+    ];
+  }
+
+}