diff --git a/docs/UPGRADE-2.0.md b/docs/UPGRADE-2.0.md index b5dda3d..375fc49 100644 --- a/docs/UPGRADE-2.0.md +++ b/docs/UPGRADE-2.0.md @@ -66,7 +66,15 @@ response. Ensure: - your gateway is configured with the correct `private_key`, and - your Payum HTTP-request bridge exposes request headers. The Symfony bridge (`Payum\Core\Bridge\Symfony\Action\GetHttpRequestAction`, used by Sylius/Symfony) does; the plain-PHP - bridge does not, so a pure plain-PHP setup must supply a header-capable `GetHttpRequest` action. + bridge does not, so a pure plain-PHP setup must register the + `Setono\Payum\Quickpay\Bridge\PlainPhp\Action\HeaderAwareGetHttpRequestAction` this package ships: + + ```php + (new PayumBuilder()) + ->addCoreGatewayFactoryConfig([ + 'payum.action.get_http_request' => new HeaderAwareGetHttpRequestAction(), + ]) + ``` ## Quickpay sends callbacks to two different places diff --git a/examples/e2e/HeaderAwareGetHttpRequestAction.php b/examples/e2e/HeaderAwareGetHttpRequestAction.php deleted file mode 100644 index 48c3fc6..0000000 --- a/examples/e2e/HeaderAwareGetHttpRequestAction.php +++ /dev/null @@ -1,65 +0,0 @@ -headers = self::headers(); - } - - /** - * @return array - */ - private static function headers(): array - { - if (function_exists('getallheaders')) { - /** @var array|false $headers */ - $headers = getallheaders(); - - if (is_array($headers)) { - return $headers; - } - } - - // Fallback for SAPIs without getallheaders(): rebuild from $_SERVER. NotifyAction matches the - // header name case-insensitively, so the exact casing produced here does not matter. - $headers = []; - - foreach ($_SERVER as $name => $value) { - if (!is_string($name) || !str_starts_with($name, 'HTTP_') || !is_scalar($value)) { - continue; - } - - $name = str_replace(' ', '-', ucwords(strtolower(str_replace('_', ' ', substr($name, 5))))); - $headers[$name] = (string) $value; - } - - return $headers; - } -} diff --git a/examples/e2e/bootstrap.php b/examples/e2e/bootstrap.php index 242d85d..4b07d6e 100644 --- a/examples/e2e/bootstrap.php +++ b/examples/e2e/bootstrap.php @@ -19,11 +19,10 @@ use Payum\Core\Registry\StorageRegistryInterface; use Payum\Core\Storage\FilesystemStorage; use Payum\Core\Storage\StorageInterface; -use Setono\Payum\Quickpay\Examples\E2E\HeaderAwareGetHttpRequestAction; +use Setono\Payum\Quickpay\Bridge\PlainPhp\Action\HeaderAwareGetHttpRequestAction; use Setono\Payum\Quickpay\QuickpayGatewayFactory; require __DIR__ . '/../../vendor/autoload.php'; -require __DIR__ . '/HeaderAwareGetHttpRequestAction.php'; e2e_load_dotenv(); diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 1ff472c..1fe88d7 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -2,3 +2,9 @@ parameters: level: 8 paths: - src + + # payum/core marks GetHttpRequest #[AllowDynamicProperties]: `headers` is a dynamic property only + # some bridges set (the Symfony one, and our HeaderAwareGetHttpRequestAction). This is PHPStan's + # documented way to say "dynamic properties are part of this class's contract". + universalObjectCratesClasses: + - Payum\Core\Request\GetHttpRequest diff --git a/src/Bridge/PlainPhp/Action/HeaderAwareGetHttpRequestAction.php b/src/Bridge/PlainPhp/Action/HeaderAwareGetHttpRequestAction.php new file mode 100644 index 0000000..996464d --- /dev/null +++ b/src/Bridge/PlainPhp/Action/HeaderAwareGetHttpRequestAction.php @@ -0,0 +1,81 @@ +addCoreGatewayFactoryConfig([ + * 'payum.action.get_http_request' => new HeaderAwareGetHttpRequestAction(), + * ]) + * + * Symfony (and therefore Sylius) consumers need none of this — payum's Symfony bridge populates the + * headers already. + * + * The headers are rebuilt from `$_SERVER` rather than read via `getallheaders()`: that function + * exists only on some SAPIs, and the polyfills that fill the gap are old and pass `$_SERVER` values + * through unsanitized. `$_SERVER` is available everywhere and carries every request header under + * the CGI `HTTP_*` convention, so it is the one source that behaves the same on every SAPI. + */ +final class HeaderAwareGetHttpRequestAction extends GetHttpRequestAction +{ + /** + * @param mixed|GetHttpRequest $request + */ + public function execute($request): void + { + if (!$request instanceof GetHttpRequest) { + throw RequestNotSupportedException::createActionNotSupported($this, $request); + } + + // The parent fills method/query/request/clientIp/uri/userAgent and — importantly for the HMAC — + // reads the raw body into `content` straight from php://input, un-re-encoded. + parent::execute($request); + + $request->headers = self::headers(); + } + + /** + * Rebuilds the request headers from `$_SERVER`. Every request header arrives as `HTTP_*` under + * the CGI convention; the two entity headers the CGI spec strips the prefix from + * (`CONTENT_TYPE`, `CONTENT_LENGTH`) are mapped too so the result reads like a full header set. + * Non-scalar values are dropped. NotifyAction matches the header name case-insensitively, so the + * exact casing produced here does not matter. + * + * @return array + */ + private static function headers(): array + { + $headers = []; + + foreach ($_SERVER as $name => $value) { + if (!is_string($name) || !is_scalar($value)) { + continue; + } + + if (str_starts_with($name, 'HTTP_')) { + $name = substr($name, 5); + } elseif ('CONTENT_TYPE' !== $name && 'CONTENT_LENGTH' !== $name) { + continue; + } + + $headers[str_replace(' ', '-', ucwords(strtolower(str_replace('_', ' ', $name))))] = (string) $value; + } + + return $headers; + } +} diff --git a/tests/Bridge/PlainPhp/Action/HeaderAwareGetHttpRequestActionTest.php b/tests/Bridge/PlainPhp/Action/HeaderAwareGetHttpRequestActionTest.php new file mode 100644 index 0000000..12f1eb2 --- /dev/null +++ b/tests/Bridge/PlainPhp/Action/HeaderAwareGetHttpRequestActionTest.php @@ -0,0 +1,147 @@ + */ + private array $originalServer; + + protected function setUp(): void + { + $this->originalServer = $_SERVER; + } + + protected function tearDown(): void + { + $_SERVER = $this->originalServer; + } + + /** + * @test + */ + public function shouldSupportGetHttpRequestOnly(): void + { + $action = new HeaderAwareGetHttpRequestAction(); + + self::assertTrue($action->supports(new GetHttpRequest())); + self::assertFalse($action->supports(new stdClass())); + self::assertFalse($action->supports('foo')); + } + + /** + * @test + */ + public function shouldThrowWhenExecutedWithAnUnsupportedRequest(): void + { + $this->expectException(RequestNotSupportedException::class); + + (new HeaderAwareGetHttpRequestAction())->execute(new stdClass()); + } + + /** + * @test + */ + public function shouldPopulateTheHeadersAlongsideTheParentFields(): void + { + $_SERVER['REQUEST_METHOD'] = 'POST'; + $_SERVER['HTTP_QUICKPAY_CHECKSUM_SHA256'] = 'the-checksum'; + $_SERVER['HTTP_USER_AGENT'] = 'the-agent'; + + $request = new GetHttpRequest(); + (new HeaderAwareGetHttpRequestAction())->execute($request); + + // The parent's own population must be preserved… + self::assertSame('POST', $request->method); + + // …and the headers — the property the plain-PHP parent never sets — must be there. + /** @var array $headers */ + $headers = $request->headers; + + self::assertSame('the-checksum', $headers['Quickpay-Checksum-Sha256']); + self::assertSame('the-agent', $headers['User-Agent']); + } + + /** + * The reconstruction produces `Quickpay-Checksum-Sha256` while Quickpay sends + * `QuickPay-Checksum-Sha256` (capital P). NotifyAction matches the header name + * case-insensitively, and this pins that the two really do meet. + * + * @test + */ + public function shouldProduceAHeaderNameNotifyActionMatches(): void + { + $_SERVER['HTTP_QUICKPAY_CHECKSUM_SHA256'] = 'the-checksum'; + + $request = new GetHttpRequest(); + (new HeaderAwareGetHttpRequestAction())->execute($request); + + /** @var array $headers */ + $headers = $request->headers; + + $found = false; + foreach (array_keys($headers) as $name) { + if (0 === strcasecmp($name, CallbackValidator::CHECKSUM_HEADER)) { + $found = true; + } + } + + self::assertTrue($found, 'The checksum header must be findable case-insensitively'); + } + + /** + * The CGI convention strips the HTTP_ prefix from the two entity headers, so they need their + * own mapping to show up in the result at all. + * + * @test + */ + public function shouldMapTheEntityHeadersTheCgiConventionLeavesUnprefixed(): void + { + $_SERVER['CONTENT_TYPE'] = 'application/json'; + $_SERVER['CONTENT_LENGTH'] = '42'; + + $request = new GetHttpRequest(); + (new HeaderAwareGetHttpRequestAction())->execute($request); + + /** @var array $headers */ + $headers = $request->headers; + + self::assertSame('application/json', $headers['Content-Type']); + self::assertSame('42', $headers['Content-Length']); + } + + /** + * @test + */ + public function shouldSkipNonHeaderAndNonScalarServerEntries(): void + { + $_SERVER['SOME_VAR'] = 'not-a-header'; + $_SERVER['HTTP_WEIRD_ARRAY'] = ['not', 'scalar']; + $_SERVER['HTTP_X_INT'] = 42; + + $request = new GetHttpRequest(); + (new HeaderAwareGetHttpRequestAction())->execute($request); + + /** @var array $headers */ + $headers = $request->headers; + + self::assertArrayNotHasKey('Some-Var', $headers); + self::assertArrayNotHasKey('SOME_VAR', $headers); + self::assertArrayNotHasKey('Weird-Array', $headers); + // Scalars are stringified, so a numeric server value comes out as a header string. + self::assertSame('42', $headers['X-Int']); + } +}