Skip to content

Commit

Permalink
feat(curl): Add connect timeout option (#10)
Browse files Browse the repository at this point in the history
* style(*): Pass through coding standards

* feat(curl): Add api_connect_timeout option

* test(php8.3): Add tests for php 8.3

* test(*): Fix ddev install
  • Loading branch information
julienloizelet authored Dec 7, 2023
1 parent 2b46a22 commit 616df38
Show file tree
Hide file tree
Showing 12 changed files with 55 additions and 29 deletions.
4 changes: 2 additions & 2 deletions .githooks/commit-msg
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ if [ -z "$1" ]; then
exit 1
fi

commitTitle="$(cat $1 | head -n1)"
commitTitle="$(head -n 1 < "$1")"

# ignore merge
if echo "$commitTitle" | grep -qE "^Merge"; then
Expand All @@ -15,7 +15,7 @@ fi

# check commit message
REGEX='^(feat|fix|docs|style|refactor|ci|test|chore|comment)\(.*\)\:.*'
if ! echo "$commitTitle" | grep -qE ${REGEX}; then
if ! echo "$commitTitle" | grep -qE "${REGEX}"; then
echo "Your commit title '$commitTitle' did not follow conventional commit message rules:"
echo "Please comply with the regex ${REGEX}"
exit 1
Expand Down
9 changes: 7 additions & 2 deletions .github/workflows/coding-standards.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ jobs:
strategy:
fail-fast: false
matrix:
php-version: ['7.4', '8.0', '8.1', '8.2']
php-version: ['7.4', '8.0', '8.1', '8.2', '8.3']

name: Coding standards test
runs-on: ubuntu-latest
Expand All @@ -38,14 +38,19 @@ jobs:
sudo apt-get -q update
sudo apt-get -q -y install libnss3-tools ddev
mkcert -install
ddev config global --instrumentation-opt-in=false --omit-containers=dba,ddev-ssh-agent
ddev config global --instrumentation-opt-in=false --omit-containers=ddev-ssh-agent
- name: Create empty PHP DDEV project
run: ddev config --project-type=php --project-name=crowdsec-php-common --php-version=${{ matrix.php-version }}

- name: Add-ons install
run: ddev get julienloizelet/ddev-tools

- name: Add Redis, Memcached and X-Debug
if: ${{ matrix.php-version == '8.3' }}
run: |
cp .ddev/okaeli-add-on/common/custom_files/config.php83missing.yaml .ddev/config.php83missing.yaml
- name: Start DDEV with PHP ${{ matrix.php-version }}
run: ddev start

Expand Down
9 changes: 7 additions & 2 deletions .github/workflows/unit-and-integration-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ jobs:
strategy:
fail-fast: false
matrix:
php-version: ['7.2','7.3','7.4','8.0','8.1', '8.2']
php-version: ['7.2','7.3','7.4','8.0','8.1', '8.2', '8.3']

name: Unit and integration test
runs-on: ubuntu-latest
Expand All @@ -41,14 +41,19 @@ jobs:
sudo apt-get -q update
sudo apt-get -q -y install libnss3-tools ddev
mkcert -install
ddev config global --instrumentation-opt-in=false --omit-containers=dba,ddev-ssh-agent
ddev config global --instrumentation-opt-in=false --omit-containers=ddev-ssh-agent
- name: Create empty PHP DDEV project
run: ddev config --project-type=php --project-name=crowdsec-php-common --php-version=${{ matrix.php-version }}

- name: Add-ons install
run: ddev get julienloizelet/ddev-tools

- name: Add Redis, Memcached and X-Debug
if: ${{ matrix.php-version == '8.3' }}
run: |
cp .ddev/okaeli-add-on/common/custom_files/config.php83missing.yaml .ddev/config.php83missing.yaml
- name: Start DDEV with PHP ${{ matrix.php-version }}
run: ddev start

Expand Down
11 changes: 11 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
The [public API](https://semver.org/spec/v2.0.0.html#spec-item-1) of this library consists of all public or protected methods, properties and constants belonging to
the `src` folder.

---

## [2.2.0](https://github.com/crowdsecurity/php-common/releases/tag/v2.2.0) - 2023-12-07
[_Compare with previous release_](https://github.com/crowdsecurity/php-common/compare/v2.1.1...v2.2.0)


### Added

- Add `api_connect_timeout` configuration for `Curl` request handler


---


Expand Down
2 changes: 0 additions & 2 deletions src/Client/AbstractClient.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,6 @@ public function __construct(

/**
* Retrieve a config value by name.
*
* @return mixed
*/
public function getConfig(string $name)
{
Expand Down
2 changes: 0 additions & 2 deletions src/Client/RequestHandler/AbstractRequestHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@ public function __construct(array $configs = [])

/**
* Retrieve a config value by name.
*
* @return mixed
*/
public function getConfig(string $name)
{
Expand Down
26 changes: 15 additions & 11 deletions src/Client/RequestHandler/Curl.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,6 @@
*/
class Curl extends AbstractRequestHandler
{
/**
* {@inheritdoc}
*/
public function handle(Request $request): Response
{
$handle = curl_init();
Expand All @@ -50,8 +47,6 @@ public function handle(Request $request): Response
/**
* @codeCoverageIgnore
*
* @param mixed $handle
*
* @return bool|string
*/
protected function exec($handle)
Expand All @@ -61,10 +56,6 @@ protected function exec($handle)

/**
* @codeCoverageIgnore
*
* @param mixed $handle
*
* @return mixed
*/
protected function getResponseHttpCode($handle)
{
Expand All @@ -78,7 +69,7 @@ private function handleConfigs(): array
if ($authType && Constants::AUTH_TLS === $authType) {
$verifyPeer = $this->getConfig('tls_verify_peer') ?? true;
$result[\CURLOPT_SSL_VERIFYPEER] = $verifyPeer;
// The --cert option
// The --cert option
$result[\CURLOPT_SSLCERT] = $this->getConfig('tls_cert_path') ?? '';
// The --key option
$result[\CURLOPT_SSLKEY] = $this->getConfig('tls_key_path') ?? '';
Expand All @@ -88,10 +79,23 @@ private function handleConfigs(): array
}
}
$timeout = $this->getConfig('api_timeout') ?? Constants::API_TIMEOUT;
// To obtain an unlimited timeout, we don't pass the option (as it is the default behavior)
/**
* To obtain an unlimited timeout, we don't pass the option (as it is the default behavior).
*
* @see https://curl.se/libcurl/c/CURLOPT_TIMEOUT.html
*/
if ($timeout > 0) {
$result[\CURLOPT_TIMEOUT] = $timeout;
}
$connectTimeout = $this->getConfig('api_connect_timeout') ?? Constants::API_CONNECT_TIMEOUT;
if ($connectTimeout >= 0) {
/**
* 0 means infinite timeout (@see https://www.php.net/manual/en/function.curl-setopt.php.
*
* @see https://curl.se/libcurl/c/CURLOPT_CONNECTTIMEOUT.html
*/
$result[\CURLOPT_CONNECTTIMEOUT] = $connectTimeout;
}

return $result;
}
Expand Down
3 changes: 0 additions & 3 deletions src/Client/RequestHandler/FileGetContents.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,6 @@
*/
class FileGetContents extends AbstractRequestHandler
{
/**
* {@inheritdoc}
*/
public function handle(Request $request): Response
{
$config = $this->createContextConfig($request);
Expand Down
8 changes: 6 additions & 2 deletions src/Constants.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
class Constants
{
/**
* @var int the default timeout (in seconds) when calling CrowdSec
* @var int the default timeout (total time of transfer operation in seconds) when calling a CrowdSec API
*/
public const API_TIMEOUT = 120;
/**
Expand All @@ -28,6 +28,10 @@ class Constants
* @var string The TLS auth type
*/
public const AUTH_TLS = 'tls';
/**
* @var int the default connection timeout (time of connection phase in seconds) when calling a CrowdSec API
*/
public const API_CONNECT_TIMEOUT = 300;
/**
* @var string The date format for CrowdSec data
*/
Expand Down Expand Up @@ -69,7 +73,7 @@ class Constants
*/
public const SCENARIO_REGEX = '#^[A-Za-z0-9]{0,16}\/[A-Za-z0-9_-]{0,64}$#';
/**
* @var string The CrowdSec country scope for decisions
* @var string The CrowdSec country scope for decisions
*/
public const SCOPE_COUNTRY = 'country';
/**
Expand Down
4 changes: 2 additions & 2 deletions tests/PHPUnitUtil.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ public static function getPHPUnitVersion(): string
*
* @see https://stackoverflow.com/a/37667018/7497291
*
* @param $object - instance in which protected value is being modified
* @param $object - instance in which protected value is being modified
* @param $property - property on instance being modified
* @param $value - new value of the property being modified
* @param $value - new value of the property being modified
*
* @return void
*/
Expand Down
1 change: 0 additions & 1 deletion tests/Unit/AbstractClientTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@
* @uses \CrowdSec\Common\Client\HttpMessage\Request::__construct
* @uses \CrowdSec\Common\Logger\AbstractLog::__construct
* @uses \CrowdSec\Common\Logger\FileLog::buildFileHandler
*/
final class AbstractClientTest extends TestAbstractClient
{
Expand Down
5 changes: 5 additions & 0 deletions tests/Unit/CurlTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
use CrowdSec\Common\Client\HttpMessage\Request;
use CrowdSec\Common\Client\HttpMessage\Response;
use CrowdSec\Common\Client\RequestHandler\Curl;
use CrowdSec\Common\Constants;
use CrowdSec\Common\Tests\Constants as TestConstants;
use CrowdSec\Common\Tests\MockedData;
use CrowdSec\Common\Tests\PHPUnitUtil;
Expand Down Expand Up @@ -142,6 +143,7 @@ public function testOptions()
\CURLOPT_URL => $url,
\CURLOPT_CUSTOMREQUEST => $method,
\CURLOPT_TIMEOUT => TestConstants::API_TIMEOUT,
\CURLOPT_CONNECTTIMEOUT => Constants::API_CONNECT_TIMEOUT,
\CURLOPT_SSL_VERIFYPEER => false,
\CURLOPT_ENCODING => '',
];
Expand Down Expand Up @@ -179,6 +181,7 @@ public function testOptions()
\CURLOPT_URL => $url . '?foo=bar&crowd=sec',
\CURLOPT_CUSTOMREQUEST => $method,
\CURLOPT_TIMEOUT => TestConstants::API_TIMEOUT,
\CURLOPT_CONNECTTIMEOUT => Constants::API_CONNECT_TIMEOUT,
\CURLOPT_SSL_VERIFYPEER => false,
\CURLOPT_ENCODING => '',
];
Expand Down Expand Up @@ -215,6 +218,7 @@ public function testOptions()
\CURLOPT_URL => $url,
\CURLOPT_CUSTOMREQUEST => $method,
\CURLOPT_TIMEOUT => TestConstants::API_TIMEOUT,
\CURLOPT_CONNECTTIMEOUT => Constants::API_CONNECT_TIMEOUT,
\CURLOPT_SSL_VERIFYPEER => true,
\CURLOPT_ENCODING => '',
\CURLOPT_SSLCERT => 'tls_cert_path_test',
Expand Down Expand Up @@ -250,6 +254,7 @@ public function testOptions()
\CURLOPT_URL => $url,
\CURLOPT_CUSTOMREQUEST => $method,
\CURLOPT_TIMEOUT => TestConstants::API_TIMEOUT,
\CURLOPT_CONNECTTIMEOUT => Constants::API_CONNECT_TIMEOUT,
\CURLOPT_SSL_VERIFYPEER => true,
\CURLOPT_SSLCERT => 'tls_cert_path_test',
\CURLOPT_SSLKEY => 'tls_key_path_test',
Expand Down

0 comments on commit 616df38

Please sign in to comment.