Skip to content

Ship HeaderAwareGetHttpRequestAction in the package - #64

Open
loevgaard wants to merge 1 commit into
2.xfrom
feat/header-aware-get-http-request
Open

Ship HeaderAwareGetHttpRequestAction in the package#64
loevgaard wants to merge 1 commit into
2.xfrom
feat/header-aware-get-http-request

Conversation

@loevgaard

Copy link
Copy Markdown
Member

Fixes gap G2 of #57.

Problem

payum/core's plain-PHP GetHttpRequest bridge never populates GetHttpRequest::$headers, and NotifyAction reads the QuickPay-Checksum-Sha256 header off exactly that property. So on a plain-PHP Payum every callback is rejected as unsigned (400) — silently, for every payment — unless the consumer found and copied the helper class hidden in examples/e2e/. That's a total failure of the notify path for a whole class of consumers, shipped as dev tooling.

Fix

  • The action now lives in the package proper: Setono\Payum\Quickpay\Bridge\PlainPhp\Action\HeaderAwareGetHttpRequestAction, with the registration snippet (addCoreGatewayFactoryConfig(['payum.action.get_http_request' => …])) in its docblock and in docs/UPGRADE-2.0.md. It is not auto-registered: the core gateway factory already defines that key, so a default could never take effect, and force-setting it would stomp the Symfony bridge for Symfony/Sylius consumers who don't need this.
  • Header values from getallheaders() (the SAPI's own or a polyfill — guzzle ships ralouphie/getallheaders, which passes $_SERVER values through untouched) and from the $_SERVER fallback are sanitized identically (string names, scalar values, everything else dropped), so the result is deterministic across SAPIs.
  • The e2e harness uses the shipped class; its local copy is deleted.
  • GetHttpRequest is declared a universalObjectCratesClass for PHPStan — headers is a dynamic property (payum marks the class #[AllowDynamicProperties]), and this is PHPStan's documented way to accept that contract.

The README section pointing plain-PHP consumers at this class comes in the follow-up docs PR, which is stacked on this one.

Tests

New HeaderAwareGetHttpRequestActionTest: headers populated alongside the parent's fields, the reconstructed checksum-header name is findable by NotifyAction's case-insensitive match, and non-header / non-scalar $_SERVER entries are dropped.

payum/core's plain-PHP GetHttpRequest bridge never populates the headers
property, and NotifyAction reads the QuickPay-Checksum-Sha256 header off
exactly that property — so on a plain-PHP Payum every callback was
rejected as unsigned with a 400, silently, for every payment, unless the
consumer found and copied the class hidden in examples/e2e/.

The action now lives in the package proper
(Setono\Payum\Quickpay\Bridge\PlainPhp\Action) with a registration
snippet in its docblock and the upgrade guide; the e2e harness uses the
shipped class and its local copy is gone. Headers from getallheaders()
(real or polyfilled — guzzle ships one that passes $_SERVER values
through untouched) and from the $_SERVER fallback are sanitized the
same way, so the result is deterministic across SAPIs.

GetHttpRequest is declared a universalObjectCratesClass for PHPStan:
headers is a dynamic property (payum marks the class
AllowDynamicProperties), and this is PHPStan's documented way to accept
that contract.
@codecov

codecov Bot commented Aug 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.66667% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.46%. Comparing base (6122f5f) to head (952142e).
⚠️ Report is 2 commits behind head on 2.x.

Files with missing lines Patch % Lines
...lainPhp/Action/HeaderAwareGetHttpRequestAction.php 66.66% 8 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##                2.x      #64      +/-   ##
============================================
- Coverage     97.43%   95.46%   -1.97%     
- Complexity      126      138      +12     
============================================
  Files            14       15       +1     
  Lines           351      375      +24     
============================================
+ Hits            342      358      +16     
- Misses            9       17       +8     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant