Skip to content

Added ELASTIC_CLIENT_URL_PLUS_AS_SPACE env variable for URL encode #1303

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Apr 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .ci/test-matrix.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ STACK_VERSION:
- 7.17-SNAPSHOT

PHP_VERSION:
- 8.2-cli
- 8.1-cli
- 8.0-cli
- 7.4-cli
Expand Down
25 changes: 10 additions & 15 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,43 +9,38 @@ jobs:

strategy:
matrix:
php-version: [7.3, 7.4, 8.0, 8.1]
php-version: [7.3, 7.4, 8.0, 8.1, 8.2]
os: [ubuntu-latest]
es-version: [7.17-SNAPSHOT]

steps:
- name: Checkout
uses: actions/checkout@v2
uses: actions/checkout@v3

- name: Use PHP ${{ matrix.php-version }}
uses: shivammathur/setup-php@v2
with:
php-version: ${{ matrix.php-version }}
extensions: yaml
extensions: yaml, zip, curl
coverage: none
env:
COMPOSER_TOKEN: ${{ secrets.GITHUB_TOKEN }}

- name: Get composer cache directory
id: composercache
run: echo "::set-output name=dir::$(composer config cache-files-dir)"
run: echo "dir=$(composer config cache-files-dir)" >> $GITHUB_OUTPUT

- name: Cache dependencies
uses: actions/cache@v2
uses: actions/cache@v3
with:
path: ${{ steps.composercache.outputs.dir }}
key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.json') }}
restore-keys: ${{ runner.os }}-composer-
key: ${{ runner.os }}-php-${{ matrix.php-version }}-${{ hashFiles('**/composer.json') }}
restore-keys: ${{ runner.os }}-php-${{ matrix.php-version }}-

- name: Install dependencies
if: ${{ matrix.php-version != '8.0' }}
run: |
composer install --prefer-dist

- name: ignore ignore-platform-reqs if it is using php 8
if: ${{ matrix.php-version == '8.0' }}
run: |
composer install --prefer-dist --ignore-platform-reqs

- name: PHP Coding Standards
run: |
composer run-script phpcs
Expand All @@ -68,7 +63,7 @@ jobs:
sudo sysctl -w vm.max_map_count=262144

- name: Runs Elasticsearch ${{ matrix.es-version }}
uses: elastic/elastic-github-actions/elasticsearch@master
uses: elastic/elastic-github-actions/elasticsearch@trial-license
with:
stack-version: ${{ matrix.es-version }}

Expand Down
7 changes: 5 additions & 2 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
"ext-yaml": "*",
"ext-zip": "*",
"mockery/mockery": "^1.2",
"phpstan/phpstan": "^0.12",
"phpstan/phpstan": "^1.10",
"phpunit/phpunit": "^9.3",
"squizlabs/php_codesniffer": "^3.4",
"symfony/finder": "~4.0"
Expand All @@ -50,7 +50,10 @@
}
},
"config": {
"sort-packages": true
"sort-packages": true,
"allow-plugins": {
"php-http/discovery": true
}
},
"scripts": {
"phpcs": [
Expand Down
1 change: 1 addition & 0 deletions phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ parameters:
- '#Constant JSON_THROW_ON_ERROR not found#'
- '#Caught class JsonException not found#'
- '#Call to method getCode\(\) on an unknown class JsonException#'
- '#Variable \$\w+ in isset\(\) always exists and is not nullable#'
2 changes: 1 addition & 1 deletion src/Elasticsearch/Connections/Connection.php
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ function ($value) {
$uri = $this->path . $uri;
}

return $uri ?? '';
return $uri;
}

public function getHeaders(): array
Expand Down
10 changes: 4 additions & 6 deletions src/Elasticsearch/Endpoints/AbstractEndpoint.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,7 @@

use Elasticsearch\Common\Exceptions\UnexpectedValueException;
use Elasticsearch\Serializers\SerializerInterface;
use Elasticsearch\Transport;
use Exception;
use GuzzleHttp\Ring\Future\FutureArrayInterface;
use Elasticsearch\Utility;

abstract class AbstractEndpoint
{
Expand Down Expand Up @@ -127,7 +125,7 @@ public function setIndex($index)
$index = implode(",", $index);
}

$this->index = urlencode($index);
$this->index = Utility::urlencode($index);

return $this;
}
Expand Down Expand Up @@ -155,7 +153,7 @@ public function setType(?string $type)
$type = implode(",", $type);
}

$this->type = urlencode($type);
$this->type = Utility::urlencode($type);

return $this;
}
Expand All @@ -175,7 +173,7 @@ public function setId($docID)
$docID = (string) $docID;
}

$this->id = urlencode($docID);
$this->id = Utility::urlencode($docID);

return $this;
}
Expand Down
4 changes: 2 additions & 2 deletions src/Elasticsearch/Serializers/SmartSerializer.php
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,10 @@ private function decode(?string $data): array
$result = json_decode($data, true, 512, JSON_THROW_ON_ERROR);
return $result;
} catch (JsonException $e) {
throw new JsonErrorException($e->getCode(), $data, $result ?? []);
throw new JsonErrorException($e->getCode(), $data, []);
}
}
throw new JsonErrorException($e->getCode(), $data, $result ?? []);
throw new JsonErrorException($e->getCode(), $data, []);
}
}

Expand Down
47 changes: 47 additions & 0 deletions src/Elasticsearch/Utility.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<?php
/**
* Elasticsearch PHP client
*
* @link https://github.com/elastic/elasticsearch-php/
* @copyright Copyright (c) Elasticsearch B.V (https://www.elastic.co)
* @license http://www.apache.org/licenses/LICENSE-2.0 Apache License, Version 2.0
* @license https://www.gnu.org/licenses/lgpl-2.1.html GNU Lesser General Public License, Version 2.1
*
* Licensed to Elasticsearch B.V under one or more agreements.
* Elasticsearch B.V licenses this file to you under the Apache 2.0 License or
* the GNU Lesser General Public License, Version 2.1, at your option.
* See the LICENSE file in the project root for more information.
*/
declare(strict_types = 1);

namespace Elasticsearch;

class Utility
{
const ENV_URL_PLUS_AS_SPACE = 'ELASTIC_CLIENT_URL_PLUS_AS_SPACE';

/**
* Get the ENV variable with a thread safe fallback criteria
* @see https://github.com/elastic/elasticsearch-php/issues/1237
*
* @return string | false
*/
public static function getEnv(string $env)
{
return $_SERVER[$env] ?? $_ENV[$env] ?? getenv($env);
}

/**
* Encode a string in URL using urlencode() or rawurlencode()
* according to env variable ES_URL_PLUS_AS_SPACE.
* If ES_URL_PLUS_AS_SPACE is true use urlencode(), otherwise rawurlencode()
*
* @see https://github.com/elastic/elasticsearch-php/issues/1278
*/
public static function urlencode(string $url): string
{
return self::getEnv(self::ENV_URL_PLUS_AS_SPACE) === 'true'
? rawurlencode($url)
: urlencode($url);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
use Elasticsearch\Connections\ConnectionInterface;

/**
* Class SnifferTest
* Class RoundRobinSelectorTest
*
* @subpackage Tests\ConnectionPool\RoundRobinSelectorTest
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@
*/
class StickyRoundRobinSelectorTest extends \PHPUnit\Framework\TestCase
{
/**
* @var Elasticsearch\ConnectionPool\Selectors\StickyRoundRobinSelector
*/
protected $roundRobin;

public function setUp(): void
{
$this->roundRobin = new Elasticsearch\ConnectionPool\Selectors\StickyRoundRobinSelector();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@

use Elasticsearch\Common\Exceptions\Serializer\JsonErrorException;
use Elasticsearch\Serializers\SmartSerializer;
use Mockery as m;
use PHPUnit\Framework\TestCase;

/**
Expand All @@ -29,6 +28,11 @@
*/
class SmartSerializerTest extends TestCase
{
/**
* @var SmartSerializer
*/
protected $serializer;

public function setUp(): void
{
$this->serializer = new SmartSerializer();
Expand Down
21 changes: 21 additions & 0 deletions tests/Elasticsearch/Tests/TransportTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,27 @@

class TransportTest extends TestCase
{
/**
* @var LoggerInterface
*/
protected $logger;
/**
* @var LoggerInterface
*/
protected $trace;
/**
* @var SerializerInterface
*/
protected $serializer;
/**
* @var AbstractConnectionPool
*/
protected $connectionPool;
/**
* @var Connection
*/
protected $connection;

public function setUp(): void
{
$this->logger = $this->createMock(LoggerInterface::class);
Expand Down
77 changes: 77 additions & 0 deletions tests/Elasticsearch/Tests/UtilityTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
<?php
/**
* Elasticsearch PHP client
*
* @link https://github.com/elastic/elasticsearch-php/
* @copyright Copyright (c) Elasticsearch B.V (https://www.elastic.co)
* @license http://www.apache.org/licenses/LICENSE-2.0 Apache License, Version 2.0
* @license https://www.gnu.org/licenses/lgpl-2.1.html GNU Lesser General Public License, Version 2.1
*
* Licensed to Elasticsearch B.V under one or more agreements.
* Elasticsearch B.V licenses this file to you under the Apache 2.0 License or
* the GNU Lesser General Public License, Version 2.1, at your option.
* See the LICENSE file in the project root for more information.
*/


declare(strict_types = 1);

namespace Elasticsearch\Tests;

use Elasticsearch\Utility;
use PHPUnit\Framework\TestCase;

class UtilityTest extends TestCase
{
public function tearDown(): void
{
unset($_SERVER[Utility::ENV_URL_PLUS_AS_SPACE]);
unset($_ENV[Utility::ENV_URL_PLUS_AS_SPACE]);
putenv(Utility::ENV_URL_PLUS_AS_SPACE);
}

public function testGetEnvWithDollarServer()
{
$_SERVER[Utility::ENV_URL_PLUS_AS_SPACE] = 'true';
$this->assertEquals('true', Utility::getEnv(Utility::ENV_URL_PLUS_AS_SPACE));
}

public function testGetEnvWithDollarEnv()
{
$_ENV[Utility::ENV_URL_PLUS_AS_SPACE] = 'true';
$this->assertEquals('true', Utility::getEnv(Utility::ENV_URL_PLUS_AS_SPACE));
}

public function testGetEnvWithPutEnv()
{
putenv(Utility::ENV_URL_PLUS_AS_SPACE . '=true');
$this->assertEquals('true', Utility::getEnv(Utility::ENV_URL_PLUS_AS_SPACE));
}

public function testUrlencodeWithDefault()
{
$url = Utility::urlencode('bar baz');
$this->assertEquals('bar+baz', $url);
}

public function testUrlencodeWithDollarServer()
{
$_SERVER[Utility::ENV_URL_PLUS_AS_SPACE] = 'true';
$url = Utility::urlencode('bar baz');
$this->assertEquals('bar%20baz', $url);
}

public function testUrlencodeWithDollarEnv()
{
$_ENV[Utility::ENV_URL_PLUS_AS_SPACE] = 'true';
$url = Utility::urlencode('bar baz');
$this->assertEquals('bar%20baz', $url);
}

public function testUrlencodeWithPutEnv()
{
putenv(Utility::ENV_URL_PLUS_AS_SPACE . '=true');
$url = Utility::urlencode('bar baz');
$this->assertEquals('bar%20baz', $url);
}
}