From 928311557eaf88ec638e8ebce57a5f2f74332b04 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Mon, 30 Aug 2021 06:56:52 +0200 Subject: [PATCH] FsockopenTest: add regression test for HTTP string PR 339 made a change to safeguard that the HTTP string would always contain a decimal point, never a comma. The original bug was related to the current locale influencing the results of the call to `sprintf()`. At the time, no test was added to safeguard the fix against regressions. This PR now adds this regression test. I've verified that without the fix as merged in 339, this test would fail. Includes adjusting the GH Actions workflows to install extra locales as otherwise the test would fail on the locale being changed to in the test not being available. --- .github/workflows/quicktest.yml | 9 +++++++++ .github/workflows/test.yml | 9 +++++++++ tests/Transport/FsockopenTest.php | 29 +++++++++++++++++++++++++++++ 3 files changed, 47 insertions(+) diff --git a/.github/workflows/quicktest.yml b/.github/workflows/quicktest.yml index b0bd62620..1d78a0795 100644 --- a/.github/workflows/quicktest.yml +++ b/.github/workflows/quicktest.yml @@ -29,6 +29,15 @@ jobs: php-version: ${{ matrix.php }} coverage: none + # At least one test needs a non-en_US locale to be available, so make sure it is. + - name: Install locales + run: | + sudo apt-get update + sudo apt-get install locales-all + + - name: Show available locales + run: locale -a + # Install dependencies and handle caching in one go. # @link https://github.com/marketplace/actions/install-composer-dependencies - name: Install Composer dependencies - normal diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 773f8cd66..62dfe38a1 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -61,6 +61,15 @@ jobs: coverage: ${{ steps.set_cov.outputs.COV }} tools: cs2pr + # At least one test needs a non-en_US locale to be available, so make sure it is. + - name: Install locales + run: | + sudo apt-get update + sudo apt-get install locales-all + + - name: Show available locales + run: locale -a + # Install dependencies and handle caching in one go. # @link https://github.com/marketplace/actions/install-composer-dependencies - name: Install Composer dependencies - normal diff --git a/tests/Transport/FsockopenTest.php b/tests/Transport/FsockopenTest.php index 5af8e7639..ba0f03be2 100644 --- a/tests/Transport/FsockopenTest.php +++ b/tests/Transport/FsockopenTest.php @@ -53,4 +53,33 @@ public function testContentLengthHeader() { public function checkContentLengthHeader($headers) { $this->assertStringContainsString('Content-Length: 0', $headers); } + + /** + * Issue #335/#339. + */ + public function testHTTPVersionHeader() { + // Remember the original locale. + $locale = setlocale(LC_NUMERIC, 0); + + // Set the locale to one using commas for the decimal point. + setlocale(LC_NUMERIC, 'de_DE@euro', 'de_DE.utf8', 'de_DE', 'de', 'ge'); + + // Make sure the locale was changed. + $this->assertNotSame($locale, setlocale(LC_NUMERIC, 0), 'Changing the locale failed'); + + $hooks = new Requests_Hooks(); + $hooks->register('fsockopen.after_headers', array($this, 'checkHTTPVersionHeader')); + + Requests::post(httpbin('/post'), array(), array(), $this->getOptions(array('hooks' => $hooks))); + + // Reset the locale. + setlocale(LC_NUMERIC, $locale); + } + + /** + * Issue #335/#339. + */ + public function checkHTTPVersionHeader($headers) { + $this->assertStringContainsString('HTTP/1.1', $headers, 'HTTP header is influenced by the system locale'); + } }