Skip to content

Commit

Permalink
LRN-43664 Remove newer code unavailable in all supported PHP versions (…
Browse files Browse the repository at this point in the history 
…#74)

* Cover packages for all Debian versions so the Docker build handles older PHP versions

* Remove PHP 8 DNF parameter type declarations LRN-43664

* Remove incorrect argument to json_encode() and add unit test
  • Loading branch information
@david-scarratt-lrn
david-scarratt-lrn authored May 8, 2024
1 parent 2c05853 commit ad913f9
Show file tree
Hide file tree
Showing 4 changed files with 125 additions and 93 deletions.
2 changes: 1 addition & 1 deletion Dockerfile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ FROM php:${PHP_VERSION}-cli-${DEBIAN_VERSION}
COPY --from=composer /usr/bin/composer /usr/local/bin/composer

RUN apt-get update && apt-get install -y --no-install-recommends \
git=1:2.* libzip-dev=1.7.* unzip=6.0* zip=3.0* \
git=1:2.* libzip-dev=1.* unzip=6.0* zip=3.0* \
&& docker-php-ext-install zip \
&& apt-get clean \
&& rm -rf /var/lib/apt/lists/*
7 changes: 6 additions & 1 deletion src/Services/PreHashStrings/LegacyPreHashString.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ public function getPreHashString(
if (in_array($this->service, static::SERVICES_REQUIRING_SIGNED_REQUEST)) {
$requestJson = $request;
if (is_array($request)) {
$requestJson = json_encode($request, true);
$requestJson = json_encode($request);
}
$signatureArray[] = $requestJson;
}
Expand All @@ -162,6 +162,11 @@ public static function supportsService(string $service): bool
return in_array($service, static::SUPPORTED_SERVICES);
}

/**
* Validate and normalise the security and request packets. Currently
* nothing is done with the request packet, but future pre-hash schemes
* may require it.
*/
public function validate(array $security, array $request): array
{
foreach (array_keys($security) as $key) {
Expand Down
155 changes: 73 additions & 82 deletions tests/Request/InitTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public function testGenerateWithMeta(
string $service,
array $security,
string $secret,
array|string $request,
/* array|string */ $request,
?string $action
) {
// This test verifies the correctness of the added telemetry data
Expand Down Expand Up @@ -85,7 +85,7 @@ public function testGenerateSignature(
string $service,
array $security,
string $secret,
array|string $request,
/* array|string */ $request,
?string $action
) {
// We disable telemetry to be able to reliably test signature generation. Added telemetry
Expand All @@ -100,11 +100,11 @@ public function testGenerateSignature(
* @dataProvider generateProvider
*/
public function testGenerate(
array|string $expectedInitOptions,
/* array|string */ $expectedInitOptions,
string $service,
array $security,
string $secret,
array|string $request,
/* array|string */ $request,
?string $action
) {
// We disable telemetry to be able to reliably test signature generation. Added telemetry
Expand Down Expand Up @@ -282,84 +282,75 @@ public function constructorProvider(): array
'The service provided (wrongservice) is not valid'
],
'empty-security' => [

$service,
'',
$secret,
$request,
$action,
null,
ValidationException::class,
'The security packet must be an array or a valid JSON string'
],
'empty-security' => [

$service,
'',
$secret,
$request,
$action,
null,
ValidationException::class,
'The security packet must be an array or a valid JSON string'
],
'null-security' => [

$service,
null,
$secret,
$request,
$action,
null,
ValidationException::class,
'The security packet must be an array or a valid JSON string'
],
'empty-secret' => [

$service,
$security,
'',
$request,
$action,
null,
ValidationException::class,
'The `secret` argument must be a valid string'
],
'incorrect-security' => [

$service,
$wrongSecurity,
$secret,
$request,
$action,
null,
ValidationException::class,
'Invalid key found in the security packet: wrongParam'
],
'missing-questions_user_id' =>
[

'questions',
$security,
$secret,
$request,
$action,
null,
ValidationException::class,
'Questions API requires a `user_id` in the security packet'
],
'invalid-request' =>
[

$service,
$security,
$secret,
25,
$action,
null,
ValidationException::class,
'The request packet must be an array or a valid JSON string'
],
$service,
'',
$secret,
$request,
$action,
null,
ValidationException::class,
'The security packet must be an array or a valid JSON string'
],
'empty-security' => [
$service,
'',
$secret,
$request,
$action,
null,
ValidationException::class,
'The security packet must be an array or a valid JSON string'
],
'null-security' => [
$service,
null,
$secret,
$request,
$action,
null,
ValidationException::class,
'The security packet must be an array or a valid JSON string'
],
'empty-secret' => [
$service,
$security,
'',
$request,
$action,
null,
ValidationException::class,
'The `secret` argument must be a valid string'
],
'incorrect-security' => [
$service,
$wrongSecurity,
$secret,
$request,
$action,
null,
ValidationException::class,
'Invalid key found in the security packet: wrongParam'
],
'missing-questions_user_id' => [
'questions',
$security,
$secret,
$request,
$action,
null,
ValidationException::class,
'Questions API requires a `user_id` in the security packet'
],
'invalid-request' => [
$service,
$security,
$secret,
25,
$action,
null,
ValidationException::class,
'The request packet must be an array or a valid JSON string'
],
];
}

Expand Down
54 changes: 45 additions & 9 deletions tests/Services/PreHashStrings/LegacyPreHashStringTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,29 +13,25 @@ public function testPreHashString(
string $service,
array $security,
string $secret,
array|string $request,
?array $request,
?string $action,
bool $v1Compat,
string $expected
) {
$preHashString = new LegacyPreHashString($service, $v1Compat);
if (is_string($request)) {
$request = json_decode($request, true);
$this->assertTrue($request !== false, 'Cannot decode JSON from string request');
}
$result = $preHashString->getPreHashString($security, $request, $action, $v1Compat ? $secret : null);
$this->assertEquals($expected, $result);
}

/** @returns array <
/** @returns array<string, array<
* string $service
* array $security
* string $secret
* array|string $request
* array $request
* ?string $action
* bool $v1Compat
* string $expected
* > */
* >> */
public function preHashStringProvider()
{
$testCases = [];
Expand Down Expand Up @@ -86,6 +82,46 @@ public function preHashStringProvider()
}
}

return $testCases;
return array_merge($testCases, $this->additionalTests());
}

/**
* Additional test cases that detect bugs found or anticipated.
* @returns array <string $name, array <
* string $service
* array $security
* string $secret
* array $request
* ?string $action
* bool $v1Compat
* string $expected
* >>
*/
public function additionalTests(): array
{
return [
'api-items-json_encode-bug' => $this->jsonEncodeBugParams(),
];
}

/**
* Test case for a bug found when the request contains escapable characters.
* @returns array <
* string $service
* array $security
* string $secret
* array $request
* ?string $action
* bool $v1Compat
* string $expected
* >
*/
public function jsonEncodeBugParams(): array
{
$params = ParamsFixture::getWorkingItemsApiParams(true);
$params['request']['name'] = str_replace('-', '<&\'>', $params['request']['name']);
$params['v1Compat'] = false;
$params['expected'] = 'yis0TYCu7U9V4o7M_localhost_20140626-0528_$ANONYMIZED_USER_ID_{"user_id":"$ANONYMIZED_USER_ID","rendering_type":"assess","name":"' . $params['request']['name'] . '","state":"initial","activity_id":"items_assess_demo","session_id":"demo_session_uuid","type":"submit_practice","config":{"configuration":{"responsive_regions":true},"navigation":{"scrolling_indicator":true},"regions":"main","time":{"show_pause":true,"max_time":300},"title":"ItemsAPI Assess Isolation Demo","subtitle":"Testing Subtitle Text"},"items":["Demo3"]}';
return $params;
}
}

0 comments on commit ad913f9

Please sign in to comment.