Ship HeaderAwareGetHttpRequestAction in the package

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
 

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();
 

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

src/Bridge/PlainPhp/Action/HeaderAwareGetHttpRequestAction.php

@@ -0,0 +1,81 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Setono\Payum\Quickpay\Bridge\PlainPhp\Action;
+
+use Payum\Core\Bridge\PlainPhp\Action\GetHttpRequestAction;
+use Payum\Core\Exception\RequestNotSupportedException;
+use Payum\Core\Request\GetHttpRequest;
+
+/**
+ * A `GetHttpRequest` action for plain-PHP (non-Symfony) setups that also populates `headers`.
+ *
+ * This exists because of a real constraint, not for convenience:
+ * {@see \Setono\Payum\Quickpay\Action\NotifyAction} reads the `QuickPay-Checksum-Sha256` header off
+ * `GetHttpRequest::$headers` to verify the callback signature, and among payum/core's bridges only
+ * the Symfony one sets that property. The plain-PHP bridge leaves it unset — so on a plain-PHP Payum
+ * every callback is rejected as unsigned with a `400`, silently, for every payment. Register this
+ * action in its place:
+ *
+ *     (new PayumBuilder())
+ *         ->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<string, string>
+     */
+    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;
+    }
+}

tests/Bridge/PlainPhp/Action/HeaderAwareGetHttpRequestActionTest.php

@@ -0,0 +1,147 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Setono\Payum\Quickpay\Tests\Bridge\PlainPhp\Action;
+
+use Payum\Core\Exception\RequestNotSupportedException;
+use Payum\Core\Request\GetHttpRequest;
+use PHPUnit\Framework\TestCase;
+use Setono\Payum\Quickpay\Bridge\PlainPhp\Action\HeaderAwareGetHttpRequestAction;
+use Setono\Quickpay\Callback\CallbackValidator;
+use stdClass;
+
+/**
+ * The action reads `$_SERVER` and nothing else, so each test sets the entries it needs and the
+ * fixture is restored afterwards. No SAPI function and no polyfill is involved.
+ */
+final class HeaderAwareGetHttpRequestActionTest extends TestCase
+{
+    /** @var array<string, mixed> */
+    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<string, string> $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<string, string> $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<string, string> $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<string, string> $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']);
+    }
+}