From e1c27eb054a7c74f5e9a66fad48fc2477e484217 Mon Sep 17 00:00:00 2001 From: Artem Dragunov Date: Sat, 25 Jul 2026 04:38:52 +0300 Subject: [PATCH 01/25] ci: publish tagged releases to GitHub and Packagist Add a tag-triggered release workflow that cuts a GitHub release from the matching CHANGELOG section and asks Packagist to re-crawl the repository. The Packagist GitHub hook already publishes tags on its own, so the ping is a fallback for when it lags; workflow_dispatch runs it without cutting a release. Tag matching accepts both spellings in use (v1.1.2 and 3.0.0), and the "Latest" flag is decided by comparing the tag against every other one with the v prefix stripped, so a 2.x backport cut after 3.0.0 cannot steal the badge. Also run CI on maintenance branches named like 2.x, which is the form Composer reads as 2.x-dev. --- .github/workflows/ci.yml | 3 +- .github/workflows/release.yml | 108 ++++++++++++++++++++++++++++++++++ 2 files changed, 110 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/release.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 75868f4..455e45e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2,7 +2,8 @@ name: CI on: push: - branches: [ master ] + # Maintenance branches follow the 2.x naming Composer understands as 2.x-dev. + branches: [ master, '[0-9]+.x' ] pull_request: workflow_dispatch: diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml new file mode 100644 index 0000000..56b7575 --- /dev/null +++ b/.github/workflows/release.yml @@ -0,0 +1,108 @@ +name: Release + +# Tag push -> GitHub release with the CHANGELOG section + a Packagist re-crawl. +# The Packagist GitHub hook already publishes new tags on its own; this ping is +# the fallback for when it lags or breaks (e.g. after a repository rename). +# Run via workflow_dispatch to ping Packagist without cutting a release. + +on: + push: + tags: + # Historic tags use both spellings: v1.1.2 and 3.0.0. + - 'v[0-9]+.[0-9]+.[0-9]+*' + - '[0-9]+.[0-9]+.[0-9]+*' + workflow_dispatch: + +permissions: + contents: write + +concurrency: + group: release-${{ github.ref }} + cancel-in-progress: false + +jobs: + release: + name: GitHub release & Packagist + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + # Full history: the release step compares this tag against every other + # one to decide whether it is really the newest version. + fetch-depth: 0 + + - name: Extract the CHANGELOG section for this tag + if: startsWith(github.ref, 'refs/tags/') + run: | + set -euo pipefail + # CHANGELOG headings are always "### v3.0.0 — 2026-07-25"; tags are not + # always prefixed, so normalise 3.0.0 and v3.0.0 to the same lookup. + heading="### v${GITHUB_REF_NAME#v} " + awk -v h="$heading" 'index($0, h) == 1 { found = 1; next } found && /^### / { exit } found' \ + CHANGELOG.md > release-notes.md + if [ ! -s release-notes.md ]; then + echo "::warning::No CHANGELOG section matching '${heading}' — falling back to generated notes" + fi + + - name: Create the GitHub release + if: startsWith(github.ref, 'refs/tags/') + env: + GH_TOKEN: ${{ github.token }} + run: | + set -euo pipefail + if gh release view "$GITHUB_REF_NAME" >/dev/null 2>&1; then + echo "Release $GITHUB_REF_NAME already exists, leaving it untouched" + exit 0 + fi + # GitHub picks "Latest" by tag date, so a 2.x backport cut after 3.0.0 + # would steal the badge. Strip the optional v prefix before comparing, + # otherwise git sorts v1.1.2 above 3.0.0. + highest=$(git tag | sed 's/^v//' | sort -V | tail -1) + if [ "${GITHUB_REF_NAME#v}" = "$highest" ]; then latest=true; else latest=false; fi + echo "Highest tag in the repository: $highest (this release --latest=$latest)" + if [ -s release-notes.md ]; then + gh release create "$GITHUB_REF_NAME" --title "$GITHUB_REF_NAME" --latest="$latest" --notes-file release-notes.md + else + gh release create "$GITHUB_REF_NAME" --title "$GITHUB_REF_NAME" --latest="$latest" --generate-notes + fi + + - name: Ask Packagist to re-crawl the repository + env: + PACKAGIST_USERNAME: ${{ vars.PACKAGIST_USERNAME }} + PACKAGIST_TOKEN: ${{ secrets.PACKAGIST_TOKEN }} + run: | + set -euo pipefail + if [ -z "${PACKAGIST_USERNAME:-}" ] || [ -z "${PACKAGIST_TOKEN:-}" ]; then + echo "PACKAGIST_USERNAME (variable) or PACKAGIST_TOKEN (secret) is missing" >&2 + exit 1 + fi + # The safe token is enough here: update-package is the one write endpoint + # it may call. Bearer auth keeps the token out of the request URL. + response=$(curl -sS --fail-with-body \ + --retry 3 --retry-delay 5 --retry-all-errors \ + -X POST \ + -H 'Content-Type: application/json' \ + -H "Authorization: Bearer ${PACKAGIST_USERNAME}:${PACKAGIST_TOKEN}" \ + -A "unitpay-php-release (+https://github.com/${GITHUB_REPOSITORY})" \ + -d "{\"repository\":\"https://github.com/${GITHUB_REPOSITORY}\"}" \ + https://packagist.org/api/update-package) + echo "$response" + echo "$response" | jq -e '.status == "success"' >/dev/null + + - name: Wait for the version to appear on Packagist + if: startsWith(github.ref, 'refs/tags/') + run: | + set -euo pipefail + package=$(jq -r .name composer.json) + version="${GITHUB_REF_NAME#v}" + # Packagist queues the crawl, so this is a poll, not an immediate check. + for _ in $(seq 1 12); do + if curl -sS "https://repo.packagist.org/p2/${package}.json" \ + | jq -e --arg p "$package" --arg v "$version" \ + '[.packages[$p][].version] | any(. == $v or . == "v" + $v)' >/dev/null; then + echo "$package $version is live on Packagist" + exit 0 + fi + sleep 10 + done + echo "::warning::$package $version is not on Packagist yet — check https://packagist.org/packages/$package" From fb84f9ee57ab99ffd37900800120be5b0730080a Mon Sep 17 00:00:00 2001 From: Artem Dragunov Date: Sat, 25 Jul 2026 04:45:22 +0300 Subject: [PATCH 02/25] fix(api): bind fluent setter params to the calls that accept them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AbstractService::request() drained PendingParams on every method, so the params accumulated by setCashItems(), setCustomerEmail(), setCustomerPhone() and setBackUrl() bound to whichever call ran first. A lookup issued between the setters and the payment swallowed them: the receipt went out with getPartner, where it is meaningless, and the payment created right after it carried no 54-FZ receipt at all. Nothing reported the loss. Draining now belongs to the calls that accept those params — form(), initPayment() and offsetAdvance() — via an explicit withPending() at the call site. Every other service call neither reads nor clears them, so a receipt survives an intervening lookup and still reaches its payment. A consuming call clears the params even when the request fails, unchanged from before, so a retry must re-apply the setters. request() keeps its signature: AbstractService is extendable, and adding a parameter would break any subclass that overrides it. --- src/Api/AbstractService.php | 28 +++++++++++---- src/Api/PaymentService.php | 15 +++++--- src/Api/PendingParams.php | 8 +++-- tests/Api/AbstractServiceTest.php | 57 +++++++++++++++++++++++++++---- tests/Api/PaymentServiceTest.php | 30 ++++++++++++++++ tests/UnitpayFacadeTest.php | 11 +++--- tests/UnitpayFormTest.php | 20 +++++++++++ 7 files changed, 143 insertions(+), 26 deletions(-) diff --git a/src/Api/AbstractService.php b/src/Api/AbstractService.php index ea4cd8a..69ff1a8 100644 --- a/src/Api/AbstractService.php +++ b/src/Api/AbstractService.php @@ -38,11 +38,27 @@ public function __construct( } /** - * Runs a server-to-server call. Accumulated fluent params (cashItems, backUrl, ...) - * are drained and merged, with the explicit call params taking precedence. An - * explicit non-empty secretKey overrides the instance key so account-level methods - * (getPartner, payouts, ...) can use the account key. The params are cleared as part - * of drain(), symmetric with form(). + * Merges the accumulated fluent params (cashItems, backUrl, customerEmail, + * customerPhone) into a call's own params and clears them; explicit call params take + * precedence. Only initPayment() and offsetAdvance() accept them. + * + * Clearing happens before the request, so a failed call consumes them too and a retry + * must re-apply the setters — symmetric with form(). + * + * @param array $params + * @return array + */ + protected function withPending(array $params): array + { + return array_merge($this->pending->drain(), $params); + } + + /** + * Runs a server-to-server call. An explicit non-empty secretKey overrides the instance + * key so account-level methods (getPartner, payouts, ...) can use the account key. + * + * This does NOT touch the accumulated fluent params: a consuming method opts in + * explicitly by wrapping its params in withPending(). * * @param array $params * @throws UnitpayValidationException when the secret key is unset/empty @@ -50,8 +66,6 @@ public function __construct( */ protected function request(string $method, array $params): object { - $params = array_merge($this->pending->drain(), $params); - if (empty($params['secretKey'])) { $params['secretKey'] = $this->secretKey; } diff --git a/src/Api/PaymentService.php b/src/Api/PaymentService.php index 5554fd6..ca05cc9 100644 --- a/src/Api/PaymentService.php +++ b/src/Api/PaymentService.php @@ -9,18 +9,22 @@ final class PaymentService extends AbstractService { /** + * Creates a payment. Consuming call: the params accumulated by setCashItems(), + * setCustomerEmail(), setCustomerPhone() and setBackUrl() are folded in here and + * cleared. + * * @param int|float|string $sum * @param int|string $projectId * @param array $options extra params (e.g. desc, currency, account) */ public function initPayment(string $account, $sum, $projectId, string $paymentType, array $options = []): object { - return $this->request('initPayment', array_merge([ + return $this->request('initPayment', $this->withPending(array_merge([ 'account' => $account, 'sum' => $sum, 'projectId' => $projectId, 'paymentType' => $paymentType, - ], $options)); + ], $options))); } /** @@ -64,15 +68,16 @@ public function cancelPayment($paymentId, array $options = []): object /** * Advance-offset fiscal receipt. Account-level: pass the account key in - * $options['secretKey'] and optionally cashItems. + * $options['secretKey'] and optionally cashItems. Consuming call: a receipt set via + * setCashItems() is folded in here and cleared. * @param int|string $paymentId * @param array $options */ public function offsetAdvance(string $login, $paymentId, array $options = []): object { - return $this->request('offsetAdvance', array_merge([ + return $this->request('offsetAdvance', $this->withPending(array_merge([ 'login' => $login, 'paymentId' => $paymentId, - ], $options)); + ], $options))); } } diff --git a/src/Api/PendingParams.php b/src/Api/PendingParams.php index 24fc18e..1b76fea 100644 --- a/src/Api/PendingParams.php +++ b/src/Api/PendingParams.php @@ -4,9 +4,11 @@ /** * Mutable holder for the fluent-setter params (cashItems, customerEmail, backUrl, - * customerPhone) accumulated on the facade. Shared between the facade and its - * services so a setter chain reaches the next form()/service call, then drained so a - * reused instance never carries this call's params into the following one. + * customerPhone) accumulated on the facade. Shared between the facade and its services. + * + * Only form(), initPayment() and offsetAdvance() read and drain them; every other call + * leaves them untouched. A consuming call clears them, so a reused instance never carries + * one order's receipt into the next. */ final class PendingParams { diff --git a/tests/Api/AbstractServiceTest.php b/tests/Api/AbstractServiceTest.php index fde5a34..b456898 100644 --- a/tests/Api/AbstractServiceTest.php +++ b/tests/Api/AbstractServiceTest.php @@ -105,11 +105,12 @@ public function testFluentSetterParamsDoNotBleedIntoNextCall(): void } /** - * Fluent-setter params are cleared once the request has been attempted — on a transport - * failure too, not only on success — so a stale receipt cannot leak into an unrelated - * later call on a reused instance. A retry must re-apply the setters (symmetric with form()). + * A CONSUMING call clears the fluent-setter params once the request has been attempted — + * on a transport failure too, not only on success — so a stale receipt cannot leak into + * an unrelated later order on a reused instance. A retry must re-apply the setters + * (symmetric with form()). */ - public function testFluentSetterParamsAreClearedAfterFailedCall(): void + public function testFluentSetterParamsAreClearedAfterFailedConsumingCall(): void { // The first call simulates a transport failure (false), later ones succeed. $transport = new FakeTransport(false, '{"result":{}}'); @@ -117,19 +118,61 @@ public function testFluentSetterParamsAreClearedAfterFailedCall(): void $unitpay->setCashItems([new CashItem('Coffee', 1, 100.0)]); try { - $unitpay->payments()->getPayment(1); + $unitpay->payments()->initPayment('order-1', 100, 7, 'card'); $this->fail('expected a transport exception on the first call'); } catch (UnitpayTransportException $e) { // expected: the transport returned false } - $unitpay->payments()->getPayment(2); + $unitpay->payments()->initPayment('order-2', 100, 7, 'card'); - // The receipt was consumed by the failed call and did NOT leak into the next one. $this->assertStringContainsString('cashItems=', $transport->url(0)); $this->assertStringNotContainsString('cashItems=', $transport->url(1)); } + /** + * A lookup issued between setCashItems() and initPayment() must not swallow the + * receipt: only the calls that accept the fluent-setter params may drain them. + */ + public function testNonConsumingCallNeitherReceivesNorConsumesPendingParams(): void + { + $transport = new FakeTransport(); + $unitpay = new Unitpay('unitpay.test', 'my-secret', $transport); + $unitpay->setCashItems([new CashItem('Coffee', 1, 100.0)]) + ->setCustomerEmail('buyer@example.com'); + + $unitpay->reference()->getPartner('partner@example.com'); + $unitpay->payments()->initPayment('order-1', 100, 7, 'card'); + + $this->assertStringNotContainsString('cashItems=', $transport->url(0)); + $this->assertStringNotContainsString('customerEmail=', $transport->url(0)); + $this->assertStringContainsString('cashItems=', $transport->url(1)); + $this->assertStringContainsString('customerEmail=', $transport->url(1)); + } + + /** + * A failing non-consuming call must not eat the pending params either — the params + * belong to the payment that has not been sent yet. + */ + public function testFailedNonConsumingCallLeavesPendingParamsIntact(): void + { + $transport = new FakeTransport(false, '{"result":{}}'); + $unitpay = new Unitpay('unitpay.test', 'my-secret', $transport); + $unitpay->setCashItems([new CashItem('Coffee', 1, 100.0)]); + + try { + $unitpay->payments()->getPayment(1); + $this->fail('expected a transport exception on the first call'); + } catch (UnitpayTransportException $e) { + // expected: the transport returned false + } + + $unitpay->payments()->initPayment('order-1', 100, 7, 'card'); + + $this->assertStringNotContainsString('cashItems=', $transport->url(0)); + $this->assertStringContainsString('cashItems=', $transport->url(1)); + } + public function testNonObjectResponseIsReportedAsTemporaryServerError(): void { $unitpay = new Unitpay('unitpay.test', 'secret', new FakeTransport('this is not json')); diff --git a/tests/Api/PaymentServiceTest.php b/tests/Api/PaymentServiceTest.php index bd44725..7b34cf5 100644 --- a/tests/Api/PaymentServiceTest.php +++ b/tests/Api/PaymentServiceTest.php @@ -4,6 +4,7 @@ use PHPUnit\Framework\TestCase; use Tests\Support\FakeTransport; +use Unitpay\Model\CashItem; use Unitpay\Unitpay; final class PaymentServiceTest extends TestCase @@ -105,4 +106,33 @@ public function testOffsetAdvanceSendsLoginAndPaymentId(): void $this->assertSame('partner@example.com', $query['login']); $this->assertSame('555', $query['paymentId']); } + + /** + * offsetAdvance is a consuming call: the advance-offset receipt is optional, but when + * it is set via setCashItems() it must reach the request and be cleared afterwards. + */ + public function testOffsetAdvanceCarriesTheAccumulatedReceipt(): void + { + $transport = new FakeTransport(); + $unitpay = $this->unitpay($transport); + $unitpay->setCashItems([new CashItem('Coffee', 1, 100.0)]); + + $unitpay->payments()->offsetAdvance('partner@example.com', 555); + $unitpay->payments()->offsetAdvance('partner@example.com', 556); + + $this->assertStringContainsString('cashItems=', $transport->url(0)); + $this->assertStringNotContainsString('cashItems=', $transport->url(1)); + } + + /** getPayment does not accept a receipt, so it must never carry one. */ + public function testGetPaymentNeverCarriesTheAccumulatedReceipt(): void + { + $transport = new FakeTransport(); + $unitpay = $this->unitpay($transport); + $unitpay->setCashItems([new CashItem('Coffee', 1, 100.0)]); + + $unitpay->payments()->getPayment(555); + + $this->assertStringNotContainsString('cashItems=', $transport->url(0)); + } } diff --git a/tests/UnitpayFacadeTest.php b/tests/UnitpayFacadeTest.php index c35e923..67af010 100644 --- a/tests/UnitpayFacadeTest.php +++ b/tests/UnitpayFacadeTest.php @@ -72,18 +72,21 @@ public function testAllServicesShareTheInjectedTransport(): void } /** - * The fluent setters live on the facade but their params belong to whichever service - * is called next — the pending-params holder is shared, not per-service. + * The pending-params holder is shared across services rather than per-service, so a + * receipt set on the facade reaches PaymentService — but only the calls that accept it: + * a payout lookup in between neither receives nor consumes it. */ - public function testAccumulatedParamsReachAnyService(): void + public function testAccumulatedParamsReachTheConsumingServiceOnly(): void { $transport = new FakeTransport(); $unitpay = new Unitpay('unitpay.test', 'secret', $transport); $unitpay->setCashItems([new CashItem('Coffee', 1, 100.0)]); $unitpay->payouts()->massPaymentCommissions('partner@example.com'); + $unitpay->payments()->initPayment('order-1', 100, 7, 'card'); - $this->assertArrayHasKey('cashItems', $transport->query()); + $this->assertArrayNotHasKey('cashItems', $transport->query(0)); + $this->assertArrayHasKey('cashItems', $transport->query(1)); } /** The domain given to the constructor drives every endpoint the facade builds. */ diff --git a/tests/UnitpayFormTest.php b/tests/UnitpayFormTest.php index 08da885..601769e 100644 --- a/tests/UnitpayFormTest.php +++ b/tests/UnitpayFormTest.php @@ -3,6 +3,7 @@ namespace Tests; use PHPUnit\Framework\TestCase; +use Tests\Support\FakeTransport; use Unitpay\Exception\UnitpayValidationException; use Unitpay\Model\CashItem; use Unitpay\Signature\SignatureBuilder; @@ -116,6 +117,25 @@ public function testFormClearsAccumulatedParamsAfterCall(): void $this->assertArrayNotHasKey('customerEmail', $second); } + /** + * A service call issued between the setters and form() must not swallow them: the + * receipt and customer belong to the form, not to the lookup that ran first. + */ + public function testFormKeepsSettersAcrossAnInterveningServiceCall(): void + { + $transport = new FakeTransport(); + $unitpay = new Unitpay('unitpay.ru', self::SECRET, $transport); + $unitpay->setCustomerEmail('customer@example.com') + ->setCashItems([new CashItem('X', 1, 100.0)]); + + $unitpay->payments()->getPayment(555); + $query = $this->queryOf($unitpay->form('pk', 100, 'acc', 'desc')); + + $this->assertStringNotContainsString('cashItems=', $transport->url(0)); + $this->assertSame('customer@example.com', $query['customerEmail']); + $this->assertArrayHasKey('cashItems', $query); + } + /** The form signature must cover ONLY the four vital params, not the setter params. */ public function testFormSignatureExcludesSetterParams(): void { From 982bbeecb44a39c3c1bbbb9c96a21f3f3f1a8f79 Mon Sep 17 00:00:00 2001 From: Artem Dragunov Date: Sat, 25 Jul 2026 04:48:07 +0300 Subject: [PATCH 03/25] fix: validate IP allowlist entries and the facade domain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both settings used to fail silently when misconfigured. setAllowedIps() and addAllowedIps() accepted any string. A typo such as "31.186.100.4 9" or a /33 prefix produced an allowlist that matched nothing, so every webhook was refused with a bare "IP address Error" and no hint at the cause. They now reject malformed entries through the IpAllowlist entry validator the feed path already used. The whole call is rejected rather than the offending entry alone, so the allowlist is never left half-configured. The feed path keeps its fail-safe contract and still never throws. The constructor domain fed three URLs — the API endpoint, the hosted form and the webhook IP feed — with no checks, so a value carrying a scheme or a path produced malformed URLs whose failure surfaced much later as a transport error or an allowlist that quietly failed to refresh. It must now be a bare host, optionally with a port, and is checked at construction time. --- src/Unitpay.php | 24 +++++++++- src/Webhook/WebhookVerifier.php | 30 +++++++++++- tests/UnitpayFacadeTest.php | 57 +++++++++++++++++++++++ tests/Webhook/AllowedIpsTest.php | 79 ++++++++++++++++++++++++++++++++ 4 files changed, 188 insertions(+), 2 deletions(-) diff --git a/src/Unitpay.php b/src/Unitpay.php index 71db1a6..c972f47 100644 --- a/src/Unitpay.php +++ b/src/Unitpay.php @@ -43,7 +43,8 @@ final class Unitpay private ?ReferenceService $reference = null; /** - * @param string $domain host only, e.g. "unitpay.ru" — without scheme or path. + * @param string $domain host only, e.g. "unitpay.ru" — without scheme or path, + * optionally with a :port. * @param TransportInterface|null $transport outbound HTTP transport for api()/feed fetch. * Defaults to CurlTransport. Inject a fake to test without the network. * @param array|null $request inbound webhook array read by the webhook @@ -58,6 +59,7 @@ public function __construct( ?array $request = null, ?string $clientIp = null ) { + $this->assertValidDomain($domain); $this->secretKey = $secretKey; $this->apiUrl = "https://$domain/api"; $this->formUrl = "https://$domain/pay/"; @@ -218,6 +220,26 @@ private function makeService(string $class) ); } + /** + * Rejects anything that is not a bare host (optionally with a port). The domain is + * interpolated into the API, form and IP-feed URLs, so a scheme or path would produce + * malformed URLs. + * + * @throws UnitpayValidationException when $domain is not a bare host + */ + private function assertValidDomain(string $domain): void + { + // Labels of 1-63 alphanumerics/hyphens (not hyphen-edged), 253 chars total, optional :port. + $label = '[A-Za-z0-9](?:[A-Za-z0-9-]{0,61}[A-Za-z0-9])?'; + $host = '(?:' . $label . '\.)*' . $label; + if (preg_match('~^(?=.{1,253}(?::|$))' . $host . '(?::\d{1,5})?$~', $domain) !== 1) { + throw new UnitpayValidationException( + 'Domain must be a bare host such as "unitpay.ru", without scheme, path or query, got ' + . var_export($domain, true) + ); + } + } + /** * Machine-readable fingerprint token for the form() URL: php__. * major.minor so the exact PHP patch is not exposed in the buyer-visible payment form URL. diff --git a/src/Webhook/WebhookVerifier.php b/src/Webhook/WebhookVerifier.php index 36bc191..b8fea46 100644 --- a/src/Webhook/WebhookVerifier.php +++ b/src/Webhook/WebhookVerifier.php @@ -146,10 +146,13 @@ public function getHandlerParams(): ?array * * Passing an empty array with no addAllowedIps() entries leaves the allowlist empty, * so every webhook is rejected (fail-closed, not a no-op) — pass at least one IP/CIDR. - * @param string[] $ips + * + * @param string[] $ips exact IPs and/or CIDR ranges + * @throws UnitpayValidationException on a malformed entry */ public function setAllowedIps(array $ips): self { + $this->assertValidEntries($ips); $this->supportedUnitpayIp = $ips; $this->ipAllowlist = null; return $this; @@ -159,15 +162,40 @@ public function setAllowedIps(array $ips): self * Adds the merchant's own IP/CIDR ranges (e.g. your proxy/relay) on top of the * Unitpay list. Preserved across refreshAllowedIps()/setAllowedIps(). Duplicates * are removed. + * * @param string[] $ips exact IPs and/or CIDR ranges + * @throws UnitpayValidationException on a malformed entry */ public function addAllowedIps(array $ips): self { + $this->assertValidEntries($ips); $this->customIps = array_values(array_unique(array_merge($this->customIps, $ips))); $this->ipAllowlist = null; return $this; } + /** + * Rejects malformed allowlist entries up front. The whole call fails, so the allowlist + * is never left half-configured. + * + * The feed path does NOT go through here: refreshAllowedIps() stays fail-safe and drops + * bad entries instead of throwing. Only the two manual setters throw. + * + * @param string[] $ips + * @throws UnitpayValidationException + */ + private function assertValidEntries(array $ips): void + { + foreach ($ips as $entry) { + if (!IpAllowlist::isValidEntry($entry)) { + throw new UnitpayValidationException( + 'Invalid IP allowlist entry: ' . var_export($entry, true) + . '. Expected an IPv4/IPv6 address or a CIDR range like "77.75.153.0/25".' + ); + } + } + } + /** * Fetches Unitpay's current published webhook IPs and makes them the allowlist. * diff --git a/tests/UnitpayFacadeTest.php b/tests/UnitpayFacadeTest.php index 67af010..cfedda2 100644 --- a/tests/UnitpayFacadeTest.php +++ b/tests/UnitpayFacadeTest.php @@ -8,6 +8,7 @@ use Unitpay\Api\PayoutService; use Unitpay\Api\ReferenceService; use Unitpay\Api\SubscriptionService; +use Unitpay\Exception\UnitpayValidationException; use Unitpay\Model\CashItem; use Unitpay\Unitpay; use Unitpay\Webhook\WebhookVerifier; @@ -89,6 +90,62 @@ public function testAccumulatedParamsReachTheConsumingServiceOnly(): void $this->assertArrayHasKey('cashItems', $transport->query(1)); } + /** + * @return array + */ + public function validDomainProvider(): array + { + return [ + 'production' => ['unitpay.ru'], + 'test tld' => ['unitpay.test'], + 'subdomain' => ['sandbox.unitpay.ru'], + 'single label' => ['localhost'], + 'host with port' => ['localhost:8080'], + 'hyphenated' => ['my-shop.example.com'], + ]; + } + + /** + * @dataProvider validDomainProvider + */ + public function testConstructorAcceptsBareHost(string $domain): void + { + $this->assertInstanceOf(Unitpay::class, new Unitpay($domain, 'secret')); + } + + /** + * @return array + */ + public function invalidDomainProvider(): array + { + return [ + 'with scheme' => ['https://unitpay.ru'], + 'scheme only' => ['http://'], + 'with path' => ['unitpay.ru/api'], + 'with query' => ['unitpay.ru?x=1'], + 'with fragment' => ['unitpay.ru#frag'], + 'with userinfo' => ['user@unitpay.ru'], + 'leading space' => [' unitpay.ru'], + 'trailing space' => ['unitpay.ru '], + 'empty' => [''], + 'hyphen edged' => ['-unitpay.ru'], + 'double dot' => ['unitpay..ru'], + ]; + } + + /** + * A scheme or path used to sail through and surface later as a transport error or an + * allowlist that silently failed to refresh. + * + * @dataProvider invalidDomainProvider + */ + public function testConstructorRejectsAnythingButABareHost(string $domain): void + { + $this->expectException(UnitpayValidationException::class); + $this->expectExceptionMessage('Domain must be a bare host'); + new Unitpay($domain, 'secret'); + } + /** The domain given to the constructor drives every endpoint the facade builds. */ public function testDomainDrivesFormAndApiEndpoints(): void { diff --git a/tests/Webhook/AllowedIpsTest.php b/tests/Webhook/AllowedIpsTest.php index 51129eb..6a1f768 100644 --- a/tests/Webhook/AllowedIpsTest.php +++ b/tests/Webhook/AllowedIpsTest.php @@ -5,6 +5,7 @@ use PHPUnit\Framework\TestCase; use Tests\Support\FakeTransport; use Unitpay\Exception\UnitpayIpException; +use Unitpay\Exception\UnitpayValidationException; use Unitpay\Signature\SignatureBuilder; use Unitpay\Unitpay; use Unitpay\Webhook\WebhookVerifier; @@ -192,6 +193,84 @@ public function testEmptyAllowlistRejectsEveryWebhook(): void $webhook->checkHandlerRequest(); } + // --- manual entry validation ------------------------------------------ + + /** + * @return array + */ + public function malformedEntryProvider(): array + { + return [ + 'trailing junk' => ['31.186.100.4 9'], + 'prefix out of range' => ['31.186.100.0/33'], + 'not an address' => ['garbage'], + 'octet out of range' => ['999.999.999.999'], + 'empty string' => [''], + ]; + } + + /** + * A typo used to be silent: the entry matched nothing and every webhook was refused + * with a bare "IP address Error". + * + * @dataProvider malformedEntryProvider + */ + public function testSetAllowedIpsRejectsMalformedEntry(string $entry): void + { + $webhook = (new Unitpay('unitpay.ru', self::SECRET))->webhook(); + + $this->expectException(UnitpayValidationException::class); + $this->expectExceptionMessage('Invalid IP allowlist entry'); + $webhook->setAllowedIps([$entry]); + } + + /** + * @dataProvider malformedEntryProvider + */ + public function testAddAllowedIpsRejectsMalformedEntry(string $entry): void + { + $webhook = (new Unitpay('unitpay.ru', self::SECRET))->webhook(); + + $this->expectException(UnitpayValidationException::class); + $webhook->addAllowedIps([$entry]); + } + + /** The whole call is rejected, so the allowlist is never left half-configured. */ + public function testRejectedCallLeavesThePreviousAllowlistIntact(): void + { + $webhook = (new Unitpay('unitpay.ru', self::SECRET))->webhook(); + $webhook->setAllowedIps(['203.0.113.7']); + + try { + $webhook->setAllowedIps(['198.51.100.5', 'garbage']); + $this->fail('expected the malformed entry to be rejected'); + } catch (UnitpayValidationException $e) { + // expected + } + + $this->assertSame(['203.0.113.7'], $webhook->getAllowedIps()); + } + + public function testValidIpv4Ipv6AndCidrEntriesAreAccepted(): void + { + $webhook = (new Unitpay('unitpay.ru', self::SECRET))->webhook(); + + $webhook->setAllowedIps(['203.0.113.7', '203.0.113.0/24', '2001:db8::1', '2001:db8::/32']); + + $this->assertSame( + ['203.0.113.7', '203.0.113.0/24', '2001:db8::1', '2001:db8::/32'], + $webhook->getAllowedIps() + ); + } + + /** The feed path stays fail-safe: it drops bad entries instead of throwing. */ + public function testFeedRefreshStillDoesNotThrowOnMalformedEntries(): void + { + $webhook = $this->handler($this->feed(['garbage', '31.186.100.0/33']), self::DEFAULT_IP); + + $this->assertTrue($webhook->refreshAllowedIps()->checkHandlerRequest()); + } + // --- matcher cache reset --------------------------------------------- public function testAddAllowedIpsInvalidatesTheMatcherCache(): void From b43e594098330b7acc0ee68f6d85db5f2d2bbdea Mon Sep 17 00:00:00 2001 From: Artem Dragunov Date: Sat, 25 Jul 2026 04:52:26 +0300 Subject: [PATCH 04/25] chore(qa): restore PHPMD thresholds and raise PHPStan to level 8 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The PHPMD ruleset was still the one written for the pre-3.0 single-file god class: it lifted ExcessiveClassComplexity to 60 and NPath to 300, and excluded MissingImport, LongVariable, ExcessiveClassLength, TooManyMethods and TooManyPublicMethods. Measured against the current src/, none of those five rules fires and the class-complexity default of 50 is never reached, so the detector was green because the bar was low rather than because the code was clean. They are gone. What remains is what actually fires, each named in a comment: StaticAccess for four stateless helpers, ShortVariable for $ip and $ch, and Superglobals for the documented $_GET/$_SERVER webhook design. checkHandlerRequest() sits at CC 10 and NPath 256, so those two thresholds are set just above it rather than at the old 300/11 — restructuring the method that decides whether a webhook is trusted is not worth the risk, and the next guard clause added there doubles NPath to 512 and trips the rule. PHPStan goes from level 6 to 8. The three findings in src/ were real type smells: the transport result reached json_decode as string|false, curl_exec() was returned as bool|string against a string|false signature, and makeService() returned a four-way union that each service getter narrowed without proof. The first is now an explicit is_string() guard with unchanged behaviour, the second normalises the boolean case, the third is a generic. Test helpers declare what parse_str() really produces. No baseline, no suppressions. --- phpmd.xml | 70 ++++++++++++++----------------- phpstan.neon | 2 +- src/Api/AbstractService.php | 3 +- src/Http/CurlTransport.php | 3 +- src/Unitpay.php | 8 ++-- tests/Api/AbstractServiceTest.php | 5 ++- tests/Api/PaymentServiceTest.php | 5 ++- tests/FloatHandlingTest.php | 4 +- tests/Support/FakeTransport.php | 6 ++- tests/UnitpayCashItemsTest.php | 4 +- tests/UnitpayFormTest.php | 4 +- 11 files changed, 62 insertions(+), 52 deletions(-) diff --git a/phpmd.xml b/phpmd.xml index d6f0ad7..a23a707 100644 --- a/phpmd.xml +++ b/phpmd.xml @@ -6,46 +6,40 @@ xsi:noNamespaceSchemaLocation="http://pmd.sf.net/ruleset_xml_schema.xsd"> - Curated PHPMD ruleset for the single-file, global-namespace Unitpay SDK. - A few default rules are excluded or retuned because they conflict with the - SDK's deliberate design rather than pointing at real problems. + PHPMD ruleset for the layered Unitpay SDK (PSR-4, src/). + + Stock rulesets with the smallest possible set of deviations: every exclusion and + every retuned threshold below corresponds to a violation that actually fires + against the current src/ and is named explicitly. The generous thresholds this + file used to carry were granted to the pre-3.0 single-file god class; that class + is gone, so they were dropped rather than inherited — an over-tuned detector + would silently accept a slide back towards it. - + - - - - - - - + - - - - - - - - @@ -53,29 +47,29 @@ - - + - + - + - diff --git a/phpstan.neon b/phpstan.neon index 63aadba..1cd333b 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -1,5 +1,5 @@ parameters: - level: 6 + level: 8 paths: - src - tests diff --git a/src/Api/AbstractService.php b/src/Api/AbstractService.php index 69ff1a8..d39bc17 100644 --- a/src/Api/AbstractService.php +++ b/src/Api/AbstractService.php @@ -82,7 +82,8 @@ protected function request(string $method, array $params): object PHP_QUERY_RFC3986 ); - $response = json_decode($this->transport->send($requestUrl, $this->fingerprintHeaders())); + $body = $this->transport->send($requestUrl, $this->fingerprintHeaders()); + $response = is_string($body) ? json_decode($body) : null; if (!is_object($response)) { throw new UnitpayTransportException('Temporary server error. Please try again later.'); } diff --git a/src/Http/CurlTransport.php b/src/Http/CurlTransport.php index 7e8a84e..37d0b94 100644 --- a/src/Http/CurlTransport.php +++ b/src/Http/CurlTransport.php @@ -34,7 +34,8 @@ public function send(string $url, array $headers = []) if (\PHP_VERSION_ID < 80000) { curl_close($ch); } - return $body; + // curl_exec() returns true only when RETURNTRANSFER is off; it is always on here. + return is_string($body) ? $body : false; } $http = ['timeout' => 10]; diff --git a/src/Unitpay.php b/src/Unitpay.php index c972f47..09142d6 100644 --- a/src/Unitpay.php +++ b/src/Unitpay.php @@ -2,6 +2,7 @@ namespace Unitpay; +use Unitpay\Api\AbstractService; use Unitpay\Api\PaymentService; use Unitpay\Api\PayoutService; use Unitpay\Api\PendingParams; @@ -205,10 +206,11 @@ public function form(string $publicKey, $sum, string $account, string $desc, str } /** - * @param class-string $class - * @return PaymentService|SubscriptionService|PayoutService|ReferenceService + * @template T of AbstractService + * @param class-string $class + * @return T */ - private function makeService(string $class) + private function makeService(string $class): AbstractService { return new $class( $this->transport, diff --git a/tests/Api/AbstractServiceTest.php b/tests/Api/AbstractServiceTest.php index b456898..f4bd744 100644 --- a/tests/Api/AbstractServiceTest.php +++ b/tests/Api/AbstractServiceTest.php @@ -63,8 +63,9 @@ public function testCashItemsFromSetterAreSentByService(): void $this->assertStringContainsString('cashItems=', $url); $this->assertStringContainsString('customerEmail=', $url); - $query = $transport->query(); - $items = json_decode(base64_decode((string) $query['cashItems']), true); + // parse_str() types query values as array|string; cashItems is always scalar here. + $encoded = $transport->query()['cashItems'] ?? ''; + $items = json_decode(base64_decode(is_string($encoded) ? $encoded : ''), true); $this->assertSame('Coffee', $items[0]['name']); } diff --git a/tests/Api/PaymentServiceTest.php b/tests/Api/PaymentServiceTest.php index 7b34cf5..562b007 100644 --- a/tests/Api/PaymentServiceTest.php +++ b/tests/Api/PaymentServiceTest.php @@ -20,7 +20,10 @@ public function testInitPaymentReturnsDecodedResponseViaInjectedTransport(): voi $response = $unitpay->payments()->initPayment('1', 100, 7, 'card'); - $this->assertSame(42, $response->result->receiptId); + $this->assertSame( + ['result' => ['receiptId' => 42]], + json_decode((string) json_encode($response), true) + ); } public function testInitPaymentSendsItsRequiredParams(): void diff --git a/tests/FloatHandlingTest.php b/tests/FloatHandlingTest.php index f57d9fd..0360689 100644 --- a/tests/FloatHandlingTest.php +++ b/tests/FloatHandlingTest.php @@ -35,7 +35,9 @@ private function sign(array $params): string } /** - * @return array + * parse_str() yields string values plus arrays for bracketed keys, hence the union. + * + * @return array|string> */ private function queryOf(string $url): array { diff --git a/tests/Support/FakeTransport.php b/tests/Support/FakeTransport.php index 63baea1..1ae96aa 100644 --- a/tests/Support/FakeTransport.php +++ b/tests/Support/FakeTransport.php @@ -59,8 +59,10 @@ public function lastUrl(): string } /** - * Query string of the n-th call, parsed into an array. - * @return array + * Query string of the n-th call, parsed into an array. parse_str() yields string + * values plus arrays for bracketed keys, hence the union. + * + * @return array|string> */ public function query(int $index = 0): array { diff --git a/tests/UnitpayCashItemsTest.php b/tests/UnitpayCashItemsTest.php index 95a41f6..8678e04 100644 --- a/tests/UnitpayCashItemsTest.php +++ b/tests/UnitpayCashItemsTest.php @@ -23,8 +23,10 @@ private function serializedItems(Unitpay $unitpay): array { $url = $unitpay->form('pk', 1, 'acc', 'desc'); parse_str((string) parse_url($url, PHP_URL_QUERY), $query); + // parse_str() types query values as array|string; cashItems is always scalar here. + $encoded = $query['cashItems'] ?? ''; - return json_decode(base64_decode((string) $query['cashItems']), true); + return json_decode(base64_decode(is_string($encoded) ? $encoded : ''), true); } public function testRequiredFieldsAreAlwaysSerialized(): void diff --git a/tests/UnitpayFormTest.php b/tests/UnitpayFormTest.php index 601769e..eb031e0 100644 --- a/tests/UnitpayFormTest.php +++ b/tests/UnitpayFormTest.php @@ -14,7 +14,9 @@ final class UnitpayFormTest extends TestCase private const SECRET = 'secret'; /** - * @return array + * parse_str() yields string values plus arrays for bracketed keys, hence the union. + * + * @return array|string> */ private function queryOf(string $url): array { From c5b9c6814c5dee9f652ae69b8f0bec05b2e0dc50 Mon Sep 17 00:00:00 2001 From: Artem Dragunov Date: Sat, 25 Jul 2026 04:53:35 +0300 Subject: [PATCH 05/25] ci: add PHP 8.5 to the matrix and verify the release tag matches VERSION composer.json declares an open "php": ">=7.4" and development runs on 8.5, but the matrix stopped at 8.4, so the supported range was wider than the verified one. The audit step no longer swallows failures: with no runtime dependencies the surface is just the dev toolchain, where an advisory is rare enough to be worth stopping for, and continue-on-error meant nobody saw it. Unitpay::VERSION is maintained by hand and the telemetry tests assert against the constant itself, so a forgotten bump keeps the suite green and ships the wrong version in User-Agent, X-Unitpay-Client and the form's sdk parameter. The release workflow now compares the constant with the tag before anything is published, so a mismatch cuts no release and pings no Packagist. History carries both tag spellings; AGENTS.md records the unprefixed form used since 2.0.0 as canonical. --- .github/workflows/ci.yml | 3 +-- .github/workflows/release.yml | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 455e45e..bf9ebb8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -21,7 +21,7 @@ jobs: strategy: fail-fast: false matrix: - php: ['7.4', '8.0', '8.1', '8.2', '8.3', '8.4'] + php: ['7.4', '8.0', '8.1', '8.2', '8.3', '8.4', '8.5'] steps: - uses: actions/checkout@v4 @@ -74,4 +74,3 @@ jobs: - name: Security audit (composer) run: composer audit - continue-on-error: true diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 56b7575..2bc02ac 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -31,6 +31,25 @@ jobs: # one to decide whether it is really the newest version. fetch-depth: 0 + - name: Set up PHP + if: startsWith(github.ref, 'refs/tags/') + uses: shivammathur/setup-php@v2 + with: + php-version: '8.3' + coverage: none + + - name: Verify Unitpay::VERSION matches the tag + if: startsWith(github.ref, 'refs/tags/') + run: | + set -euo pipefail + declared=$(php -r 'require "src/Unitpay.php"; echo Unitpay\Unitpay::VERSION;') + expected="${GITHUB_REF_NAME#v}" + if [ "$declared" != "$expected" ]; then + echo "::error::Tag ${GITHUB_REF_NAME} declares version '${expected}' but Unitpay::VERSION is '${declared}'. Bump the constant and retag." + exit 1 + fi + echo "Unitpay::VERSION=${declared} matches tag ${GITHUB_REF_NAME}" + - name: Extract the CHANGELOG section for this tag if: startsWith(github.ref, 'refs/tags/') run: | From be07b8ba5f7988a5f43d581ec0d6ab24cbfff2ac Mon Sep 17 00:00:00 2001 From: Artem Dragunov Date: Sat, 25 Jul 2026 04:57:59 +0300 Subject: [PATCH 06/25] docs: document the 3.1.0 behavior changes and bump VERSION MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit api-methods.md carried the canonical statement of the old fluent-parameter contract ("merged into the next call — form() or any service method"), which the 3.1.0 fix inverts; it now lists the three consuming calls and shows what an intervening lookup does. receipts.md, getting-started.md, webhooks.md and examples/receipt.php follow, and getting-started.md documents the accepted domain format. The changelog leads with the three behavior changes rather than filing them as fixes: two of them turn a silent failure into a thrown exception, and the third changes which call a receipt attaches to. --- CHANGELOG.md | 12 ++++++++++++ docs/api-methods.md | 31 +++++++++++++++++++++++++++---- docs/getting-started.md | 8 +++++++- docs/receipts.md | 13 +++++++++++++ docs/webhooks.md | 6 ++++++ examples/receipt.php | 8 +++++--- src/Unitpay.php | 2 +- 7 files changed, 71 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f1189f..a143bdc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,17 @@ # Changelog +### v3.1.0 — 2026-07-25 + +**Behavior changes.** Three settings that used to fail silently now either bind correctly or say what is wrong. Nothing changed on the wire. + +* **Fluent setter params bind to the calls that accept them, not to whichever call runs first.** `AbstractService::request()` used to drain the accumulated params on every method, so `setCashItems()` followed by any lookup sent the 54-FZ receipt with that lookup — where it is meaningless — and the payment created right after it carried no receipt at all. Nothing reported the loss. The params are now read and cleared only by `form()`, `payments()->initPayment()` and `payments()->offsetAdvance()`; every other service call leaves them untouched, so a receipt survives an intervening lookup and still reaches its payment. A consuming call still clears them even when the request fails, so a retry must re-apply the setters. If your code relied on a receipt reaching a non-payment method, it was already being discarded before the payment — the fix makes it arrive instead +* **`setAllowedIps()` and `addAllowedIps()` reject malformed entries** with `UnitpayValidationException` instead of accepting them. A typo such as `31.186.100.4 9` or a `/33` prefix produced an allowlist that matched nothing, so every webhook was refused with a bare `IP address Error` and no hint at the cause. The whole call is rejected rather than the offending entry alone, so the allowlist is never left half-configured. `refreshAllowedIps()` is unchanged and still fail-safe: it drops bad feed entries and never throws +* **The constructor `$domain` must be a bare host** (optionally with a port) and is validated at construction time, throwing `UnitpayValidationException` otherwise. It feeds three URLs — the API endpoint, the hosted form and the webhook IP feed — so a value carrying a scheme or a path (`https://unitpay.ru`, `unitpay.ru/api`) used to produce malformed URLs whose failure surfaced much later as a transport error or an allowlist that quietly failed to refresh +* QA: PHPStan raised from level 6 to **level 8**, with no baseline and no suppressions. The three findings in `src/` were real type smells — the transport result reached `json_decode` as `string|false`, `curl_exec()` was returned as `bool|string` against a `string|false` signature, and `makeService()` returned a four-way union that each service getter narrowed without proof. Behavior is unchanged in all three +* QA: the PHPMD ruleset was still the one written for the pre-3.0 single-file class, lifting `ExcessiveClassComplexity` to 60 and `NPathComplexity` to 300 and excluding five further rules. Measured against the current `src/`, none of those five fires and the class-complexity default is never reached, so they were dropped. What remains is only what actually fires, each named in a comment; the two thresholds `checkHandlerRequest()` needs now sit just above its measured values +* CI: added PHP **8.5** to the test matrix, which now covers the whole declared `>=7.4` range, and `composer audit` no longer swallows failures +* CI: a tagged release verifies that `Unitpay::VERSION` matches the tag before publishing anything — the telemetry tests assert against the constant itself, so a forgotten bump used to stay green and ship the wrong version in `User-Agent`, `X-Unitpay-Client` and the form's `sdk` parameter. Release tags are unprefixed (`3.1.0`), the form used since 2.0.0 + ### v3.0.0 — 2026-07-25 **Breaking release.** The SDK is no longer a single file in the global namespace: it is now a PSR-4 package (`Unitpay\` → `src/`) split into layers — Http, Api services, Signature, Webhook, Model/Enum, Exception — behind a thin `Unitpay\Unitpay` facade. Nothing changed on the wire; only the PHP surface you call. Step-by-step renames are in [docs/migration-v3.md](docs/migration-v3.md). diff --git a/docs/api-methods.md b/docs/api-methods.md index bf3f0be..651ad8e 100644 --- a/docs/api-methods.md +++ b/docs/api-methods.md @@ -88,10 +88,33 @@ Note: `confirmPayment` and `cancelPayment` return a top-level `message` ## Fluent parameters Parameters accumulated by `setCashItems()`, `setCustomerEmail()`, `setCustomerPhone()` and -`setBackUrl()` are merged into the next call — `form()` or any service method — and cleared -afterwards, so a reused instance never carries one order's receipt into the next. Explicit -options take precedence over accumulated ones. The clearing happens even when the request -fails, so a retry must re-apply the setters. +`setBackUrl()` are merged into the calls that accept them, and cleared afterwards — so a +reused instance never carries one order's receipt into the next. Explicit options take +precedence over accumulated ones. The clearing happens even when the request fails, so a +retry must re-apply the setters. + +Only three calls consume them: + +| Call | Consumes | +| --- | --- | +| `form()` | `backUrl`, `customerEmail`, `customerPhone`, `cashItems` | +| `payments()->initPayment()` | `backUrl`, `customerEmail`, `customerPhone`, `cashItems` | +| `payments()->offsetAdvance()` | `cashItems` | + +Every other service method — `getPayment`, `refundPayment`, `confirmPayment`, +`cancelPayment` and everything on `subscriptions()`, `payouts()` and `reference()` — neither +receives nor clears them. A lookup issued between the setters and the payment therefore +leaves the receipt alone: + +```php +$unitpay->setCashItems([$item]); + +$unitpay->reference()->getPartner($login); // no receipt attached, nothing consumed +$unitpay->payments()->initPayment(...); // the receipt arrives here +``` + +> Before 3.1 every service call drained these params, so the lookup above sent the receipt +> with `getPartner` and the payment was created without one. ## See Also diff --git a/docs/getting-started.md b/docs/getting-started.md index 56e0bfa..013e023 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -56,11 +56,17 @@ a `TransportInterface`, the inbound request array, and the client IP: new Unitpay(string $domain, ?string $secretKey = null, ?TransportInterface $transport = null, ?array $request = null, ?string $clientIp = null) ``` +`$domain` is a **bare host** — `unitpay.ru`, or the address Unitpay support gave you — with +an optional `:port`. No scheme, path or query: it is interpolated into the API endpoint, the +hosted-form URL and the webhook IP feed, so `https://unitpay.ru` or `unitpay.ru/api` would +produce broken URLs. Anything else throws `UnitpayValidationException` from the constructor. + ## Create a payment (Unitpay hosted form) `form()` builds a signed redirect URL to Unitpay's hosted payment page. Fluent setters (`setBackUrl`, `setCustomerEmail`, `setCustomerPhone`, `setCashItems`) are optional and -apply to both `form()` and the service calls. +apply to `form()`, `payments()->initPayment()` and `payments()->offsetAdvance()` — see +[Fluent parameters](api-methods.md#fluent-parameters). ```php setMeasure(Measure::ITEM); $unitpay->setCashItems([$item]); ``` +The receipt waits on the instance until one of the three calls that accept it runs — +`form()`, `payments()->initPayment()` or `payments()->offsetAdvance()` — and is cleared +there. Other service calls ignore it, so a lookup in between is harmless: + +```php +$unitpay->setCashItems([$item]); +$unitpay->payments()->getPayment($someOtherId); // receipt untouched +$unitpay->payments()->initPayment(...); // receipt arrives here +``` + +A consuming call clears the receipt even if the request fails, so a retry must call +`setCashItems()` again. See [Fluent parameters](api-methods.md#fluent-parameters). + The dictionaries live in `Unitpay\Model\Enum` as const-classes: `Nds`, `PaymentObject`, `PaymentMethod`, `Measure` (and `PaymentType` for payment-method codes). They are plain classes with `public const`, not native enums, because the SDK supports PHP 7.4. diff --git a/docs/webhooks.md b/docs/webhooks.md index a8fccb7..f93c784 100644 --- a/docs/webhooks.md +++ b/docs/webhooks.md @@ -95,6 +95,12 @@ webhook verifier: * `$webhook->setAllowedIps([...])` replaces the Unitpay list outright. Passing an empty array is fail-closed, not a no-op: with no `addAllowedIps()` entries it rejects every webhook. +* Both setters accept exact IPv4/IPv6 addresses and CIDR ranges, and reject anything else + with `UnitpayValidationException`. A malformed entry (`'31.186.100.4 9'`, + `'31.186.100.0/33'`) would otherwise match nothing and every webhook would fail with a + bare `IP address Error`. The whole call is rejected, so the allowlist is never left + half-configured. `refreshAllowedIps()` keeps its fail-safe contract instead: it drops bad + entries from the feed and never throws. * Override `getIp()` if you run behind a proxy (the check uses `REMOTE_ADDR`, not the spoofable `X-Forwarded-For`). `getIp()` and `isAllowedIp()` are `protected`, so extend `WebhookVerifier` to change them. diff --git a/examples/receipt.php b/examples/receipt.php index 2c18225..ebb8bd2 100644 --- a/examples/receipt.php +++ b/examples/receipt.php @@ -4,9 +4,11 @@ /** * 54-FZ fiscal receipt: line items are described by CashItem objects and attached to - * the payment via setCashItems(). The receipt goes out with the next form()/service call - * and is cleared after a successful call. For the customer to receive the receipt, set - * their contact (email and/or phone) via setCustomerEmail()/setCustomerPhone(). + * the payment via setCashItems(). The receipt goes out with the next call that accepts + * one — form(), payments()->initPayment() or payments()->offsetAdvance() — and is cleared + * there; other service calls leave it alone, so a lookup in between is harmless. A retry + * after a failed call must re-apply the setters. For the customer to receive the receipt, + * set their contact (email and/or phone) via setCustomerEmail()/setCustomerPhone(). * * The dictionaries of VAT rates, payment objects, payment methods and units of measure * are const-classes under Unitpay\Model\Enum: Nds, PaymentObject, PaymentMethod, Measure. diff --git a/src/Unitpay.php b/src/Unitpay.php index 09142d6..539ddd0 100644 --- a/src/Unitpay.php +++ b/src/Unitpay.php @@ -26,7 +26,7 @@ final class Unitpay { /** SDK version; sent in the telemetry fingerprint. Keep in sync with the release git tag. */ - public const VERSION = '3.0.0'; + public const VERSION = '3.1.0'; /** Unitpay API surface this SDK targets; sent in the telemetry fingerprint. */ public const API_VERSION = 'v1'; From e31b92d5b493d39183eee19bbcb5094d9d1d879a Mon Sep 17 00:00:00 2001 From: Artem Dragunov Date: Sat, 25 Jul 2026 12:39:16 +0300 Subject: [PATCH 07/25] chore(release): consolidate the unreleased 3.1.0 batch into 4.0.0 3.1.0 was prepared on feature/tech-debt-bc-safe-3-1-0 but never merged or tagged. Its contents ship as part of 4.0.0 instead of as a separate release, so the CHANGELOG entry is relabelled rather than duplicated. The VERSION bump lands first on purpose: the release workflow compares the git tag against Unitpay::VERSION, so while the constant read 3.1.0 that release could still have been tagged and published by accident. --- CHANGELOG.md | 8 ++++++-- src/Unitpay.php | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a143bdc..7833615 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,8 +1,12 @@ # Changelog -### v3.1.0 — 2026-07-25 +### v4.0.0 — 2026-07-25 -**Behavior changes.** Three settings that used to fail silently now either bind correctly or say what is wrong. Nothing changed on the wire. +**Breaking release.** The transport contract changed: `Unitpay\Http\TransportInterface` now returns a `Response` object instead of `string|false`, and inbound webhooks older than the tolerance window are rejected. If you implement your own transport, see [docs/migration-v4.md](docs/migration-v4.md); if you only use the SDK's own transport, the upgrade is a version bump. + +> **3.1.0 was never published.** The BC-safe tech-debt batch prepared under that number ships here instead, so this entry covers both it and the 4.0 work. Nothing was dropped. + +**Behavior changes carried over from the unreleased 3.1.0.** Three settings that used to fail silently now either bind correctly or say what is wrong. Nothing changed on the wire. * **Fluent setter params bind to the calls that accept them, not to whichever call runs first.** `AbstractService::request()` used to drain the accumulated params on every method, so `setCashItems()` followed by any lookup sent the 54-FZ receipt with that lookup — where it is meaningless — and the payment created right after it carried no receipt at all. Nothing reported the loss. The params are now read and cleared only by `form()`, `payments()->initPayment()` and `payments()->offsetAdvance()`; every other service call leaves them untouched, so a receipt survives an intervening lookup and still reaches its payment. A consuming call still clears them even when the request fails, so a retry must re-apply the setters. If your code relied on a receipt reaching a non-payment method, it was already being discarded before the payment — the fix makes it arrive instead * **`setAllowedIps()` and `addAllowedIps()` reject malformed entries** with `UnitpayValidationException` instead of accepting them. A typo such as `31.186.100.4 9` or a `/33` prefix produced an allowlist that matched nothing, so every webhook was refused with a bare `IP address Error` and no hint at the cause. The whole call is rejected rather than the offending entry alone, so the allowlist is never left half-configured. `refreshAllowedIps()` is unchanged and still fail-safe: it drops bad feed entries and never throws diff --git a/src/Unitpay.php b/src/Unitpay.php index 539ddd0..20fa855 100644 --- a/src/Unitpay.php +++ b/src/Unitpay.php @@ -26,7 +26,7 @@ final class Unitpay { /** SDK version; sent in the telemetry fingerprint. Keep in sync with the release git tag. */ - public const VERSION = '3.1.0'; + public const VERSION = '4.0.0'; /** Unitpay API surface this SDK targets; sent in the telemetry fingerprint. */ public const API_VERSION = 'v1'; From 3b20aead25fe875b57a24124d6e33995b3322c87 Mon Sep 17 00:00:00 2001 From: Artem Dragunov Date: Sat, 25 Jul 2026 12:39:28 +0300 Subject: [PATCH 08/25] feat(http)!: replace the string|false transport contract with a Response object MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TransportInterface::send(): string|false threw away the HTTP status, the response headers and the cURL errno, so a connect failure, a read timeout, a 404, a 500 with an HTML body, a disabled allow_url_fopen and malformed JSON all reached the caller as the same UnitpayTransportException("Temporary server error. Please try again later.") — including the two that are permanent, not temporary. This is finding F008. TransportInterface::request() now returns Http\Response, carrying the status, body, headers, errno and a wasRequestSent() flag. AbstractService classifies it into UnitpayNetworkException (no response), UnitpayHttpException (non-2xx) or UnitpayResponseException (2xx with an unusable payload), all extending UnitpayTransportException so one catch still covers everything. wasRequestSent() is derived from CURLINFO_CONNECT_TIME, not from the errno: cURL reports CURLE_OPERATION_TIMEDOUT for a connect timeout and a read timeout alike, and the consequence differs completely — after a read timeout the server may already have created the payment, and the API accepts no idempotency key to make a repeat harmless. CURLINFO_PRETRANSFER_TIME looks like the right field and is not: measured on cURL 8.21.0 it is non-zero on connect-phase failures. The measurement table is kept next to the code. WebhookVerifier::fetchUnitpayIps() stays fail-safe and still throws nothing: a webhook handler must not start failing because a best-effort feed refresh could not reach the server. BREAKING CHANGE: custom TransportInterface implementations must replace send(): string|false with request(): Response. --- src/Api/AbstractService.php | 70 +++++++- src/Exception/UnitpayHttpException.php | 21 +++ src/Exception/UnitpayNetworkException.php | 21 +++ src/Exception/UnitpayResponseException.php | 20 +++ src/Exception/UnitpayTransportException.php | 41 ++++- src/Http/CurlTransport.php | 179 +++++++++++++++++--- src/Http/Response.php | 122 +++++++++++++ src/Http/TransportInterface.php | 10 +- src/Webhook/WebhookVerifier.php | 11 +- tests/Api/AbstractServiceTest.php | 127 ++++++++++++-- tests/Http/ResponseTest.php | 103 +++++++++++ tests/Support/FakeTransport.php | 46 +++-- 12 files changed, 716 insertions(+), 55 deletions(-) create mode 100644 src/Exception/UnitpayHttpException.php create mode 100644 src/Exception/UnitpayNetworkException.php create mode 100644 src/Exception/UnitpayResponseException.php create mode 100644 src/Http/Response.php create mode 100644 tests/Http/ResponseTest.php diff --git a/src/Api/AbstractService.php b/src/Api/AbstractService.php index d39bc17..747399d 100644 --- a/src/Api/AbstractService.php +++ b/src/Api/AbstractService.php @@ -2,8 +2,12 @@ namespace Unitpay\Api; +use Unitpay\Exception\UnitpayHttpException; +use Unitpay\Exception\UnitpayNetworkException; +use Unitpay\Exception\UnitpayResponseException; use Unitpay\Exception\UnitpayTransportException; use Unitpay\Exception\UnitpayValidationException; +use Unitpay\Http\Response; use Unitpay\Http\TransportInterface; use Unitpay\Signature\SignatureBuilder; @@ -82,13 +86,69 @@ protected function request(string $method, array $params): object PHP_QUERY_RFC3986 ); - $body = $this->transport->send($requestUrl, $this->fingerprintHeaders()); - $response = is_string($body) ? json_decode($body) : null; - if (!is_object($response)) { - throw new UnitpayTransportException('Temporary server error. Please try again later.'); + return $this->decode($this->transport->request($requestUrl, $this->fingerprintHeaders())); + } + + /** + * Turns a transport result into the decoded JSON envelope, or into the exception that + * describes what actually went wrong. Before 4.0 every branch below collapsed into one + * "Temporary server error" — including a disabled allow_url_fopen, which is permanent. + * + * @throws UnitpayNetworkException no response arrived + * @throws UnitpayHttpException a response arrived with a non-2xx status + * @throws UnitpayResponseException a 2xx arrived whose body is not a JSON object + */ + private function decode(Response $response): object + { + if ($response->getStatusCode() === 0) { + throw new UnitpayNetworkException( + $this->networkMessage($response), + $response->getErrno(), + $response->getTransportError() + ); + } + + if (!$response->isSuccessful()) { + throw new UnitpayHttpException( + sprintf('Unitpay API returned HTTP %d.', $response->getStatusCode()), + $response->getStatusCode(), + $response->getBody() + ); + } + + $decoded = json_decode($response->getBody()); + if (!is_object($decoded)) { + throw new UnitpayResponseException( + sprintf( + 'Unitpay API returned HTTP %d with a body that is not a JSON object (%s).', + $response->getStatusCode(), + json_last_error_msg() + ), + $response->getStatusCode(), + $response->getBody() + ); + } + + return $decoded; + } + + /** + * Whether the request reached Unitpay decides what the caller may safely do next, so + * the message says it outright. The API accepts no idempotency key, so a blind repeat + * of a delivered initPayment can create a second payment. + */ + private function networkMessage(Response $response): string + { + $detail = sprintf('%s (error %d)', $response->getTransportError(), $response->getErrno()); + + if ($response->wasRequestSent()) { + return 'No response from the Unitpay API: ' . $detail + . '. The request was sent, so it may already have been processed — check the' + . ' payment state before repeating it.'; } - return $response; + return 'Could not reach the Unitpay API: ' . $detail + . '. The request was not sent, so nothing was processed.'; } /** diff --git a/src/Exception/UnitpayHttpException.php b/src/Exception/UnitpayHttpException.php new file mode 100644 index 0000000..f30e4c9 --- /dev/null +++ b/src/Exception/UnitpayHttpException.php @@ -0,0 +1,21 @@ +statusCode = $statusCode; + $this->responseBody = $responseBody; + } +} diff --git a/src/Exception/UnitpayNetworkException.php b/src/Exception/UnitpayNetworkException.php new file mode 100644 index 0000000..344ef08 --- /dev/null +++ b/src/Exception/UnitpayNetworkException.php @@ -0,0 +1,21 @@ +errno = $errno; + $this->transportError = $transportError; + } +} diff --git a/src/Exception/UnitpayResponseException.php b/src/Exception/UnitpayResponseException.php new file mode 100644 index 0000000..1b54659 --- /dev/null +++ b/src/Exception/UnitpayResponseException.php @@ -0,0 +1,20 @@ +statusCode = $statusCode; + $this->responseBody = $responseBody; + } +} diff --git a/src/Exception/UnitpayTransportException.php b/src/Exception/UnitpayTransportException.php index 34f6919..10f3e94 100644 --- a/src/Exception/UnitpayTransportException.php +++ b/src/Exception/UnitpayTransportException.php @@ -2,7 +2,46 @@ namespace Unitpay\Exception; -/** api() could not obtain a usable response from Unitpay (network or response parsing). */ +/** + * A service call could not obtain a usable response from Unitpay. + * + * Base of the three concrete cases — UnitpayNetworkException (no response), + * UnitpayHttpException (non-2xx) and UnitpayResponseException (unusable payload) — so a + * caller that only wants "the request failed" still gets away with one catch. Every + * accessor is nullable because each subclass fills in only what its case actually knows. + * + * This class deliberately takes plain scalars rather than an Http\Response: Exception is + * a leaf layer and must not depend on Http (see .ai-factory/ARCHITECTURE.md). Turning a + * Response into one of these lives in the Api layer. + */ class UnitpayTransportException extends \InvalidArgumentException implements UnitpayExceptionInterface { + protected ?int $statusCode = null; + protected ?int $errno = null; + protected ?string $transportError = null; + protected ?string $responseBody = null; + + /** HTTP status, or null when no response arrived. */ + public function getStatusCode(): ?int + { + return $this->statusCode; + } + + /** cURL errno (or Http\Response::ERRNO_LOCAL), or null when a response did arrive. */ + public function getErrno(): ?int + { + return $this->errno; + } + + /** Transport-level error text, or null when a response did arrive. */ + public function getTransportError(): ?string + { + return $this->transportError; + } + + /** Raw response body, or null when no response arrived. */ + public function getResponseBody(): ?string + { + return $this->responseBody; + } } diff --git a/src/Http/CurlTransport.php b/src/Http/CurlTransport.php index 37d0b94..e98d1b2 100644 --- a/src/Http/CurlTransport.php +++ b/src/Http/CurlTransport.php @@ -10,46 +10,185 @@ * The file_get_contents fallback suppresses its transport warning via * set_error_handler (not the '@' operator, which QA rules forbid) — otherwise that * warning would log the URL together with the secret. + * + * Neither path throws: the outcome is described in a Response and classified by the + * calling layer. */ final class CurlTransport implements TransportInterface { + private int $connectTimeout = 5; + private int $timeout = 10; + /** * @param string[] $headers - * @return string|false */ - public function send(string $url, array $headers = []) + public function request(string $url, array $headers = []): Response { if (function_exists('curl_init')) { - $ch = curl_init($url); - $opts = [ - CURLOPT_RETURNTRANSFER => true, - CURLOPT_CONNECTTIMEOUT => 5, - CURLOPT_TIMEOUT => 10, - ]; - if ($headers !== []) { - $opts[CURLOPT_HTTPHEADER] = $headers; - } - curl_setopt_array($ch, $opts); - $body = curl_exec($ch); - if (\PHP_VERSION_ID < 80000) { - curl_close($ch); - } - // curl_exec() returns true only when RETURNTRANSFER is off; it is always on here. - return is_string($body) ? $body : false; + return $this->requestViaCurl($url, $headers); + } + + return $this->requestViaStream($url, $headers); + } + + /** + * @param string[] $headers + */ + private function requestViaCurl(string $url, array $headers): Response + { + $ch = curl_init($url); + $opts = [ + CURLOPT_RETURNTRANSFER => true, + CURLOPT_CONNECTTIMEOUT => $this->connectTimeout, + CURLOPT_TIMEOUT => $this->timeout, + // Prepends the header block to the body rather than taking a + // CURLOPT_HEADERFUNCTION callback, whose mandatory handle argument would go + // unused. Redirects are not followed, so there is exactly one block. + CURLOPT_HEADER => true, + ]; + if ($headers !== []) { + $opts[CURLOPT_HTTPHEADER] = $headers; + } + curl_setopt_array($ch, $opts); + + $raw = curl_exec($ch); + $errno = curl_errno($ch); + $error = curl_error($ch); + $status = (int) curl_getinfo($ch, CURLINFO_RESPONSE_CODE); + $headerSize = (int) curl_getinfo($ch, CURLINFO_HEADER_SIZE); + $connectTime = (float) curl_getinfo($ch, CURLINFO_CONNECT_TIME); + + if (\PHP_VERSION_ID < 80000) { + curl_close($ch); + } + + // curl_exec() returns true only when RETURNTRANSFER is off; it is always on here. + if ($errno !== 0 || !is_string($raw)) { + return Response::failed($errno, $error, self::wasRequestSent($connectTime)); + } + + return Response::received( + $status, + substr($raw, $headerSize), + self::splitHeaderBlock(substr($raw, 0, $headerSize)) + ); + } + + /** + * @param string[] $headers + */ + private function requestViaStream(string $url, array $headers): Response + { + if (!filter_var(ini_get('allow_url_fopen'), FILTER_VALIDATE_BOOLEAN)) { + return Response::failed( + Response::ERRNO_LOCAL, + 'ext-curl is not installed and allow_url_fopen is disabled in php.ini, so the SDK ' + . 'cannot make outbound requests. This is a permanent local configuration problem, ' + . 'not a temporary one: install ext-curl or enable allow_url_fopen.', + false + ); } - $http = ['timeout' => 10]; + $http = [ + 'timeout' => $this->timeout, + // Without this a 4xx/5xx yields false and the error body is lost — exactly the + // diagnostic this release exists to preserve. + 'ignore_errors' => true, + ]; if ($headers !== []) { $http['header'] = implode("\r\n", $headers); } $context = stream_context_create(['http' => $http]); + set_error_handler(static function () { return true; }); try { - return file_get_contents($url, false, $context); + // The stream wrapper writes $http_response_header into this scope, but only + // once a response actually arrives. Seeding it keeps the no-response case from + // reading an undefined variable. + $http_response_header = []; + $body = file_get_contents($url, false, $context); + $responseHeaders = $http_response_header; } finally { restore_error_handler(); } + + if (!is_string($body)) { + return Response::failed( + Response::ERRNO_LOCAL, + 'The request failed and the file_get_contents fallback cannot report why. ' + . 'Install ext-curl for a diagnosable transport.', + // This path exposes no connect/read phase signal, so the conservative + // answer is the only safe one — and it is why it is never retried. + false + ); + } + + /** @var string[] $responseHeaders */ + return Response::received(self::parseStatus($responseHeaders), $body, $responseHeaders); + } + + /** + * Whether the request had already been handed to the wire when the attempt failed — + * the flag the retry policy is built on, so getting it wrong risks a duplicate + * payment rather than a duplicate log line. + * + * cURL reports CURLE_OPERATION_TIMEDOUT (28) for both a connect timeout and a read + * timeout, so the errno cannot separate them. CURLINFO_CONNECT_TIME can: it is + * exactly 0.0 until a connection is established. Measured on cURL 8.21.0: + * + * case errno connect_time pretransfer_time + * HTTP 200 0 0.003443 0.144292 + * HTTP 404 0 0.002099 0.169619 + * DNS failure 6 0.000000 0.001893 + * connection refused 7 0.000000 0.000316 + * connect timeout 28 0.000000 3.003365 + * + * Note the third column: CURLINFO_PRETRANSFER_TIME is NOT zero on connect-phase + * failures on modern cURL, so it cannot be used for this — do not "simplify" back to + * it. + * + * A TLS handshake that fails after the TCP connect reports connect_time > 0 with the + * request still unsent, so this answers "sent" there. That is the deliberate + * direction of the error: claiming "sent" costs a missed retry, claiming "not sent" + * wrongly costs a second payment. + */ + private static function wasRequestSent(float $connectTime): bool + { + return $connectTime > 0.0; + } + + /** + * Splits the raw cURL header block into the same shape the stream wrapper produces, + * so both transports hand Response identical header lines. + * + * @return string[] + */ + private static function splitHeaderBlock(string $block): array + { + $lines = preg_split('/\r\n|\n/', trim($block)); + + return $lines === false ? [] : array_values(array_filter($lines, static function (string $line): bool { + return $line !== ''; + })); + } + + /** + * Reads the status out of the response header lines. Defaults to 200 when no status + * line is present: file_get_contents returning a string means the fetch itself + * succeeded, and the pre-4.0 contract treated any body as a success. + * + * @param string[] $headers + */ + private static function parseStatus(array $headers): int + { + foreach ($headers as $header) { + if (preg_match('#^HTTP/\d(?:\.\d)?\s+(\d{3})#', $header, $matches) === 1) { + return (int) $matches[1]; + } + } + + return 200; } } diff --git a/src/Http/Response.php b/src/Http/Response.php new file mode 100644 index 0000000..b1707af --- /dev/null +++ b/src/Http/Response.php @@ -0,0 +1,122 @@ +statusCode = $statusCode; + $this->body = $body; + $this->headers = $headers; + $this->errno = $errno; + $this->error = $error; + $this->requestSent = $requestSent; + } + + /** + * An HTTP response came back, whatever its status. Receiving one proves the request + * reached the server, so requestSent is always true here. + * + * @param string[] $headers raw response header lines, as received + */ + public static function received(int $statusCode, string $body, array $headers = []): self + { + return new self($statusCode, $body, $headers, 0, '', true); + } + + /** + * No HTTP response came back. + * + * @param int $errno cURL errno, or ERRNO_LOCAL for a local failure + * @param bool $requestSent whether the request had already been put on the wire — see + * wasRequestSent(); pass false when it cannot be determined + */ + public static function failed(int $errno, string $error, bool $requestSent): self + { + return new self(0, '', [], $errno, $error, $requestSent); + } + + /** HTTP status, or 0 when no response arrived. */ + public function getStatusCode(): int + { + return $this->statusCode; + } + + public function getBody(): string + { + return $this->body; + } + + /** @return string[] raw response header lines */ + public function getHeaders(): array + { + return $this->headers; + } + + /** cURL errno, ERRNO_LOCAL for a local failure, or 0 when the transport did not fail. */ + public function getErrno(): int + { + return $this->errno; + } + + /** Transport-level error text, or '' when the transport did not fail. */ + public function getTransportError(): string + { + return $this->error; + } + + /** + * Whether the request was already on the wire when the attempt failed. + * + * This is the retry-safety signal, and the only thing a retry decision may be based + * on. cURL reports CURLE_OPERATION_TIMEDOUT (28) for both a connect timeout and a read + * timeout, so the errno cannot tell the two apart — but the consequence differs + * completely: after a read timeout the server may already have created the payment, + * and the Unitpay API accepts no idempotency key to make a repeat harmless. + */ + public function wasRequestSent(): bool + { + return $this->requestSent; + } + + /** Whether an HTTP response arrived with a 2xx status. */ + public function isSuccessful(): bool + { + return $this->statusCode >= 200 && $this->statusCode < 300; + } +} diff --git a/src/Http/TransportInterface.php b/src/Http/TransportInterface.php index 4824466..1371087 100644 --- a/src/Http/TransportInterface.php +++ b/src/Http/TransportInterface.php @@ -4,15 +4,15 @@ /** * Outbound HTTP transport used by the API services and the webhook IP-feed fetch. - * Implementations perform an HTTP GET and return the raw response body, or false - * on a transport error. Inject a fake in tests to exercise the SDK without the - * network. + * Implementations perform an HTTP GET and describe the outcome in a Response — they + * report failures through it rather than throwing, so the calling layer owns the + * decision of which exception a given outcome deserves. Inject a fake in tests to + * exercise the SDK without the network. */ interface TransportInterface { /** * @param string[] $headers HTTP headers of the form "Name: value" - * @return string|false raw response body, or false on a transport error */ - public function send(string $url, array $headers = []); + public function request(string $url, array $headers = []): Response; } diff --git a/src/Webhook/WebhookVerifier.php b/src/Webhook/WebhookVerifier.php index b8fea46..f36dbd7 100644 --- a/src/Webhook/WebhookVerifier.php +++ b/src/Webhook/WebhookVerifier.php @@ -267,11 +267,18 @@ protected function isAllowedIp(string $ip): bool /** * Fetches and validates the published webhook IP feed. + * + * Deliberately throws nothing, unlike the API services which turn the same transport + * results into typed exceptions: a webhook handler must not start failing because a + * best-effort feed refresh could not reach the server. Any non-2xx, any transport + * error and any malformed payload yield null, and the caller keeps the current list. + * * @return string[]|null validated non-empty list, or null on any error */ private function fetchUnitpayIps(): ?array { - $body = $this->transport->send($this->ipsUrl); - return is_string($body) ? IpAllowlist::parseWebhooksFeed($body) : null; + $response = $this->transport->request($this->ipsUrl); + + return $response->isSuccessful() ? IpAllowlist::parseWebhooksFeed($response->getBody()) : null; } } diff --git a/tests/Api/AbstractServiceTest.php b/tests/Api/AbstractServiceTest.php index f4bd744..e54b287 100644 --- a/tests/Api/AbstractServiceTest.php +++ b/tests/Api/AbstractServiceTest.php @@ -5,7 +5,11 @@ use InvalidArgumentException; use PHPUnit\Framework\TestCase; use Tests\Support\FakeTransport; +use Unitpay\Exception\UnitpayHttpException; +use Unitpay\Exception\UnitpayNetworkException; +use Unitpay\Exception\UnitpayResponseException; use Unitpay\Exception\UnitpayTransportException; +use Unitpay\Http\Response; use Unitpay\Model\CashItem; use Unitpay\Unitpay; @@ -174,38 +178,137 @@ public function testFailedNonConsumingCallLeavesPendingParamsIntact(): void $this->assertStringContainsString('cashItems=', $transport->url(1)); } - public function testNonObjectResponseIsReportedAsTemporaryServerError(): void + public function testMissingSecretThrows(): void { - $unitpay = new Unitpay('unitpay.test', 'secret', new FakeTransport('this is not json')); + $unitpay = new Unitpay('unitpay.test', null, new FakeTransport()); $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('Temporary server error'); + $this->expectExceptionMessage('SecretKey is null'); $unitpay->payments()->getPayment(1); } - public function testMissingSecretThrows(): void + /** + * A 2xx whose body is not a JSON object is a protocol problem, not a network one: + * the server answered, it just did not answer with what the API promises. + */ + public function testNonJsonBodyThrowsResponseException(): void { - $unitpay = new Unitpay('unitpay.test', null, new FakeTransport()); + $unitpay = new Unitpay('unitpay.test', 'secret', new FakeTransport('this is not json')); - $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('SecretKey is null'); - $unitpay->payments()->getPayment(1); + try { + $unitpay->payments()->getPayment(1); + $this->fail('expected a response exception'); + } catch (UnitpayResponseException $e) { + $this->assertSame('this is not json', $e->getResponseBody()); + $this->assertInstanceOf(UnitpayTransportException::class, $e); + $this->assertInstanceOf(InvalidArgumentException::class, $e); + } } - /** A transport failure is a typed exception, still catchable as InvalidArgumentException. */ - public function testTransportFailureThrowsTypedTransportException(): void + /** A connect-phase failure never reached the server; the message must say so. */ + public function testConnectFailureThrowsNetworkException(): void { - $unitpay = new Unitpay('unitpay.test', 'secret', new FakeTransport(false)); + $transport = new FakeTransport(Response::failed(7, 'Failed to connect to unitpay.test port 443', false)); + $unitpay = new Unitpay('unitpay.test', 'secret', $transport); + + try { + $unitpay->payments()->getPayment(1); + $this->fail('expected a network exception'); + } catch (UnitpayNetworkException $e) { + $this->assertSame(7, $e->getErrno()); + $this->assertNotNull($e->getTransportError()); + $this->assertStringContainsString('Failed to connect', (string) $e->getTransportError()); + $this->assertStringContainsString('was not sent', $e->getMessage()); + } + } + + /** + * A read timeout carries the same cURL errno as a connect timeout but a completely + * different consequence: the server saw the request and may already have created the + * payment. The message has to say that out loud — it is the difference between "retry + * safely" and "check before retrying". + */ + public function testReadTimeoutSaysTheRequestMayHaveBeenProcessed(): void + { + $transport = new FakeTransport(Response::failed(28, 'Operation timed out after 10001 milliseconds', true)); + $unitpay = new Unitpay('unitpay.test', 'secret', $transport); + + try { + $unitpay->payments()->initPayment('order-1', 100, 7, 'card'); + $this->fail('expected a network exception'); + } catch (UnitpayNetworkException $e) { + $this->assertSame(28, $e->getErrno()); + $this->assertStringContainsString('may already have been processed', $e->getMessage()); + $this->assertStringNotContainsString('was not sent', $e->getMessage()); + } + } + + public function testHttp404ThrowsHttpExceptionCarryingTheStatus(): void + { + $transport = new FakeTransport(Response::received(404, '{"error":{"message":"Not found"}}')); + $unitpay = new Unitpay('unitpay.test', 'secret', $transport); + + try { + $unitpay->payments()->getPayment(1); + $this->fail('expected an http exception'); + } catch (UnitpayHttpException $e) { + $this->assertSame(404, $e->getStatusCode()); + $this->assertStringContainsString('404', $e->getMessage()); + } + } + + /** + * A 500 with an HTML error page used to be indistinguishable from a timeout. The body + * is what an integrator needs to quote in a support ticket, so it must survive. + */ + public function testHttp500CarriesTheResponseBody(): void + { + $html = '

502 Bad Gateway

'; + $transport = new FakeTransport(Response::received(500, $html)); + $unitpay = new Unitpay('unitpay.test', 'secret', $transport); + + try { + $unitpay->payments()->getPayment(1); + $this->fail('expected an http exception'); + } catch (UnitpayHttpException $e) { + $this->assertSame(500, $e->getStatusCode()); + $this->assertSame($html, $e->getResponseBody()); + } + } + + /** + * The five cases above are distinct classes now, but a caller that only wants "the + * request failed" must still get away with a single catch — and with the pre-4.0 + * InvalidArgumentException catch it may already have. + * + * @dataProvider transportFailures + * @param string|false|Response $result + */ + public function testEveryTransportFailureIsStillOneCatch($result): void + { + $unitpay = new Unitpay('unitpay.test', 'secret', new FakeTransport($result)); try { $unitpay->payments()->getPayment(1); $this->fail('expected a transport exception'); } catch (UnitpayTransportException $e) { $this->assertInstanceOf(InvalidArgumentException::class, $e); - $this->assertStringContainsString('Temporary server error', $e->getMessage()); + $this->assertStringNotContainsString('Temporary server error', $e->getMessage()); } } + /** @return array */ + public function transportFailures(): array + { + return [ + 'connect failure' => [Response::failed(7, 'Failed to connect', false)], + 'read timeout' => [Response::failed(28, 'Operation timed out', true)], + 'http 404' => [Response::received(404, '')], + 'http 500' => [Response::received(500, 'oops')], + 'non-json 200' => ['this is not json'], + ]; + } + /** Account-level methods can override the project key with the account key (secretKey). */ public function testExplicitSecretKeyOverridesInstanceKey(): void { diff --git a/tests/Http/ResponseTest.php b/tests/Http/ResponseTest.php new file mode 100644 index 0000000..fde975c --- /dev/null +++ b/tests/Http/ResponseTest.php @@ -0,0 +1,103 @@ +assertSame(200, $response->getStatusCode()); + $this->assertSame('{"result":{}}', $response->getBody()); + $this->assertSame(['Content-Type: application/json'], $response->getHeaders()); + $this->assertSame(0, $response->getErrno()); + $this->assertSame('', $response->getTransportError()); + } + + /** A response that came back proves the request reached the server, whatever its status. */ + public function testReceivedAlwaysCountsAsRequestSent(): void + { + $this->assertTrue(Response::received(200, 'ok')->wasRequestSent()); + $this->assertTrue(Response::received(500, 'boom')->wasRequestSent()); + } + + public function testFailedHasNoStatusAndCarriesTheTransportError(): void + { + $response = Response::failed(7, 'Failed to connect to unitpay.ru port 443', false); + + $this->assertSame(0, $response->getStatusCode()); + $this->assertSame('', $response->getBody()); + $this->assertSame(7, $response->getErrno()); + $this->assertSame('Failed to connect to unitpay.ru port 443', $response->getTransportError()); + } + + /** + * The retry-safety signal. cURL reports CURLE_OPERATION_TIMEDOUT (28) for both a + * connect timeout and a read timeout, so the errno alone cannot tell whether the + * server saw the request. Only wasRequestSent() may drive a retry decision — a + * read timeout means the payment may already have been created. + */ + public function testWasRequestSentDistinguishesConnectFromReadTimeout(): void + { + $connectTimeout = Response::failed(28, 'Connection timed out after 5001 milliseconds', false); + $readTimeout = Response::failed(28, 'Operation timed out after 10001 milliseconds', true); + + $this->assertSame($connectTimeout->getErrno(), $readTimeout->getErrno()); + $this->assertFalse($connectTimeout->wasRequestSent()); + $this->assertTrue($readTimeout->wasRequestSent()); + } + + /** + * @dataProvider successfulStatuses + */ + public function testIsSuccessfulAcceptsTheWhole2xxRange(int $status): void + { + $this->assertTrue(Response::received($status, '')->isSuccessful()); + } + + /** @return array */ + public function successfulStatuses(): array + { + return [ + 'status 200' => [200], + 'status 201' => [201], + 'status 204' => [204], + 'status 299' => [299], + ]; + } + + /** + * @dataProvider unsuccessfulStatuses + */ + public function testIsSuccessfulRejectsEverythingOutside2xx(int $status): void + { + $this->assertFalse(Response::received($status, '')->isSuccessful()); + } + + /** @return array */ + public function unsuccessfulStatuses(): array + { + return [ + 'status 199' => [199], + 'status 301' => [301], + 'status 404' => [404], + 'status 429' => [429], + 'status 500' => [500], + ]; + } + + public function testFailedIsNeverSuccessful(): void + { + $this->assertFalse(Response::failed(6, 'Could not resolve host', false)->isSuccessful()); + } +} diff --git a/tests/Support/FakeTransport.php b/tests/Support/FakeTransport.php index 1ae96aa..f6331ea 100644 --- a/tests/Support/FakeTransport.php +++ b/tests/Support/FakeTransport.php @@ -2,37 +2,46 @@ namespace Tests\Support; +use Unitpay\Http\Response; use Unitpay\Http\TransportInterface; /** * Test double for the outbound HTTP transport: records every call (URL + headers) and - * replays a queue of canned bodies, so the SDK can be exercised without the network. - * Replaces the constructor callable the pre-3.0 suite injected. + * replays a queue of canned results, so the SDK can be exercised without the network. + * + * A queued item may be: + * - a string → shorthand for HTTP 200 with that body (the common case); + * - false → shorthand for a connect-phase failure (no response, request not sent); + * - a Response → the full result, needed for status codes, headers, and the read-timeout + * case where the request DID reach the server. */ final class FakeTransport implements TransportInterface { /** @var array */ private array $calls = []; - /** @var array */ + /** @var array */ private array $responses; /** - * @param string|false ...$responses bodies returned by successive send() calls; the - * last one is reused once the queue runs out. Pass - * false to simulate a transport failure. + * @param string|false|Response ...$responses results returned by successive request() + * calls; the last one is reused once the + * queue runs out. */ public function __construct(...$responses) { - /** @var array $responses */ - $this->responses = $responses === [] ? ['{"result":{}}'] : $responses; + if ($responses === []) { + $responses = ['{"result":{}}']; + } + + /** @var array $responses */ + $this->responses = array_map([self::class, 'normalize'], $responses); } /** * @param string[] $headers - * @return string|false */ - public function send(string $url, array $headers = []) + public function request(string $url, array $headers = []): Response { $this->calls[] = ['url' => $url, 'headers' => $headers]; @@ -82,4 +91,21 @@ public function header(string $name, int $index = 0): ?string return null; } + + /** + * @param string|false|Response $response + */ + private static function normalize($response): Response + { + if ($response instanceof Response) { + return $response; + } + + if ($response === false) { + // errno 7 = CURLE_COULDNT_CONNECT: the historical meaning of a bare `false`. + return Response::failed(7, 'Simulated transport failure', false); + } + + return Response::received(200, $response); + } } From 28a4ea7ac7044941a337ad0aed64eef96b46eabf Mon Sep 17 00:00:00 2001 From: Artem Dragunov Date: Sat, 25 Jul 2026 12:44:41 +0300 Subject: [PATCH 09/25] feat(http): configurable timeouts and connect-phase retries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CurlTransport's 5s/10s timeouts were hardcoded, so a merchant on a slow link could not raise them and a latency-sensitive one could not lower them. They are now constructor arguments, defaulting to the previous values, and a non-positive value is rejected at construction — cURL reads 0 as "wait forever", which in a payment flow is a request that never returns. RetryingTransport repeats an attempt only when it provably never reached Unitpay. The API accepts no idempotency key, so the policy is much narrower than what other payment SDKs ship: a read timeout, a 5xx, a 409 and a 429 are all left alone because the server may already have created the payment. Only DNS failure, refused connection and connect-phase timeout are retried, with capped exponential backoff and jitter, and the resulting failure reports how many attempts were made. It is a decorator rather than a flag inside CurlTransport so the policy can be unit-tested without the network, and it sits below the service layer so a retry re-sends identical bytes instead of re-entering withPending() against an already-drained PendingParams. DefaultTransport names the default stack (cURL behind the retry policy) and keeps the facade from having to know how it is assembled; withoutRetries() is the documented off switch. --- src/Http/CurlTransport.php | 48 +++- src/Http/DefaultTransport.php | 29 +++ src/Http/RetryingTransport.php | 141 ++++++++++++ src/Unitpay.php | 12 +- tests/Http/CurlTransportTest.php | 76 +++++++ tests/Http/RetryingTransportTest.php | 217 +++++++++++++++++++ tests/Support/SleeplessRetryingTransport.php | 26 +++ 7 files changed, 543 insertions(+), 6 deletions(-) create mode 100644 src/Http/DefaultTransport.php create mode 100644 src/Http/RetryingTransport.php create mode 100644 tests/Http/CurlTransportTest.php create mode 100644 tests/Http/RetryingTransportTest.php create mode 100644 tests/Support/SleeplessRetryingTransport.php diff --git a/src/Http/CurlTransport.php b/src/Http/CurlTransport.php index e98d1b2..f1afa5d 100644 --- a/src/Http/CurlTransport.php +++ b/src/Http/CurlTransport.php @@ -2,6 +2,8 @@ namespace Unitpay\Http; +use Unitpay\Exception\UnitpayValidationException; + /** * Default transport: cURL (with connect/read timeouts and no dependency on * allow_url_fopen) when ext-curl is present, otherwise a file_get_contents @@ -16,8 +18,38 @@ */ final class CurlTransport implements TransportInterface { - private int $connectTimeout = 5; - private int $timeout = 10; + private int $connectTimeout; + private int $timeout; + + /** + * @param int $connectTimeout seconds allowed for establishing the connection. cURL + * only; the file_get_contents fallback has no separate + * connect timeout and is bounded by $timeout alone. + * @param int $timeout seconds allowed for the whole request + * + * @throws UnitpayValidationException when either timeout is not positive + */ + public function __construct(int $connectTimeout = 5, int $timeout = 10) + { + // Rejected here rather than at the first request, so a misconfiguration surfaces + // where it was made. cURL reads 0 as "wait forever", which in a payment flow means + // a request that never returns. + self::assertPositive($connectTimeout, 'Connect timeout'); + self::assertPositive($timeout, 'Timeout'); + + $this->connectTimeout = $connectTimeout; + $this->timeout = $timeout; + } + + public function getConnectTimeout(): int + { + return $this->connectTimeout; + } + + public function getTimeout(): int + { + return $this->timeout; + } /** * @param string[] $headers @@ -129,6 +161,18 @@ private function requestViaStream(string $url, array $headers): Response return Response::received(self::parseStatus($responseHeaders), $body, $responseHeaders); } + /** + * @throws UnitpayValidationException + */ + private static function assertPositive(int $seconds, string $label): void + { + if ($seconds <= 0) { + throw new UnitpayValidationException( + sprintf('%s must be a positive number of seconds, got %d.', $label, $seconds) + ); + } + } + /** * Whether the request had already been handed to the wire when the attempt failed — * the flag the retry policy is built on, so getting it wrong risks a duplicate diff --git a/src/Http/DefaultTransport.php b/src/Http/DefaultTransport.php new file mode 100644 index 0000000..4635bfc --- /dev/null +++ b/src/Http/DefaultTransport.php @@ -0,0 +1,29 @@ +inner = $inner; + $this->maxRetries = $maxRetries; + $this->baseDelay = $baseDelay; + $this->maxDelay = $maxDelay; + } + + /** The wrapped transport, for callers that need to reach it without the retry policy. */ + public function getInner(): TransportInterface + { + return $this->inner; + } + + /** + * @param string[] $headers + */ + public function request(string $url, array $headers = []): Response + { + $attempts = 0; + + do { + if ($attempts > 0) { + $this->sleep($this->backoff($attempts)); + } + $response = $this->inner->request($url, $headers); + ++$attempts; + } while ($this->shouldRetry($response) && $attempts <= $this->maxRetries); + + return $this->describeAttempts($response, $attempts); + } + + /** + * Pauses between attempts. Overridden in tests so the suite never actually waits. + */ + protected function sleep(float $seconds): void + { + usleep((int) round($seconds * 1000000)); + } + + /** + * The whole safety argument in one expression: a status means the server answered, and + * wasRequestSent() means it saw the request even though no status came back. Either + * way it may already have acted on it. Do not widen this to cover 5xx or 429 unless + * the API gains an idempotency key. + */ + private function shouldRetry(Response $response): bool + { + return $response->getStatusCode() === 0 && !$response->wasRequestSent(); + } + + /** + * Exponential backoff, capped, with jitter over the upper half of the interval so a + * fleet of clients failing at the same moment does not retry in lockstep. + */ + private function backoff(int $retry): float + { + $delay = min($this->baseDelay * (2 ** ($retry - 1)), $this->maxDelay); + $jittered = $delay * 0.5 * (1 + (mt_rand() / mt_getrandmax())); + + return max($this->baseDelay * 0.5, $jittered); + } + + /** + * Records the attempt count in the failure the caller receives. Without it a retried + * failure is indistinguishable from a single one, and the seconds spent retrying look + * like an unexplained stall. + */ + private function describeAttempts(Response $response, int $attempts): Response + { + if ($attempts < 2 || $response->getStatusCode() !== 0) { + return $response; + } + + return Response::failed( + $response->getErrno(), + sprintf('%s (after %d attempts)', $response->getTransportError(), $attempts), + $response->wasRequestSent() + ); + } +} diff --git a/src/Unitpay.php b/src/Unitpay.php index 20fa855..1f2e700 100644 --- a/src/Unitpay.php +++ b/src/Unitpay.php @@ -9,7 +9,7 @@ use Unitpay\Api\ReferenceService; use Unitpay\Api\SubscriptionService; use Unitpay\Exception\UnitpayValidationException; -use Unitpay\Http\CurlTransport; +use Unitpay\Http\DefaultTransport; use Unitpay\Http\TransportInterface; use Unitpay\Model\CashItem; use Unitpay\Signature\SignatureBuilder; @@ -46,8 +46,12 @@ final class Unitpay /** * @param string $domain host only, e.g. "unitpay.ru" — without scheme or path, * optionally with a :port. - * @param TransportInterface|null $transport outbound HTTP transport for api()/feed fetch. - * Defaults to CurlTransport. Inject a fake to test without the network. + * @param TransportInterface|null $transport outbound HTTP transport for the service calls + * and the IP-feed fetch. Defaults to a CurlTransport wrapped + * in a RetryingTransport, which repeats an attempt only when + * it provably never reached Unitpay. Pass + * DefaultTransport::withoutRetries() to switch retries off, + * or a fake to test without the network. * @param array|null $request inbound webhook array read by the webhook * verifier. Defaults to $_GET. * @param string|null $clientIp sender IP used by the webhook verifier. Defaults to @@ -65,7 +69,7 @@ public function __construct( $this->apiUrl = "https://$domain/api"; $this->formUrl = "https://$domain/pay/"; $ipsUrl = "https://$domain/ips/ips_webhooks.json"; - $this->transport = $transport ?? new CurlTransport(); + $this->transport = $transport ?? DefaultTransport::create(); $this->signature = new SignatureBuilder(); $this->pending = new PendingParams(); $this->webhookVerifier = new WebhookVerifier( diff --git a/tests/Http/CurlTransportTest.php b/tests/Http/CurlTransportTest.php new file mode 100644 index 0000000..d6fac8d --- /dev/null +++ b/tests/Http/CurlTransportTest.php @@ -0,0 +1,76 @@ +assertSame(5, $transport->getConnectTimeout()); + $this->assertSame(10, $transport->getTimeout()); + } + + public function testTimeoutsAreConfigurable(): void + { + $transport = new CurlTransport(2, 30); + + $this->assertSame(2, $transport->getConnectTimeout()); + $this->assertSame(30, $transport->getTimeout()); + } + + /** + * cURL reads 0 as "wait forever". In a payment flow that is a request which never + * comes back, so it is rejected rather than passed through. + * + * @dataProvider nonPositiveTimeouts + */ + public function testNonPositiveConnectTimeoutIsRejected(int $seconds): void + { + $this->expectException(UnitpayValidationException::class); + $this->expectExceptionMessage('Connect timeout must be a positive number of seconds'); + + new CurlTransport($seconds, 10); + } + + /** + * @dataProvider nonPositiveTimeouts + */ + public function testNonPositiveTimeoutIsRejected(int $seconds): void + { + $this->expectException(UnitpayValidationException::class); + $this->expectExceptionMessage('Timeout must be a positive number of seconds'); + + new CurlTransport(5, $seconds); + } + + /** @return array */ + public function nonPositiveTimeouts(): array + { + return [ + 'zero' => [0], + 'negative' => [-1], + ]; + } + + /** The message has to name the offending value, or the caller has to go looking. */ + public function testRejectionMessageNamesTheValue(): void + { + try { + new CurlTransport(5, -3); + $this->fail('expected a validation exception'); + } catch (UnitpayValidationException $e) { + $this->assertStringContainsString('-3', $e->getMessage()); + } + } +} diff --git a/tests/Http/RetryingTransportTest.php b/tests/Http/RetryingTransportTest.php new file mode 100644 index 0000000..8444108 --- /dev/null +++ b/tests/Http/RetryingTransportTest.php @@ -0,0 +1,217 @@ +request('https://unitpay.test/api?method=getPayment'); + + $this->assertSame(3, $inner->callCount()); + $this->assertSame(200, $response->getStatusCode()); + } + + /** + * The single most important case in this file. A read timeout carries the same cURL + * errno as a connect timeout, but the server saw the request — retrying it is how you + * charge a customer twice. + */ + public function testNeverRetriesAfterTheRequestReachedTheServer(): void + { + $inner = new FakeTransport( + Response::failed(28, 'Operation timed out after 10001 milliseconds', true), + Response::received(200, '{"result":{}}') + ); + $transport = new SleeplessRetryingTransport($inner, 3); + + $response = $transport->request('https://unitpay.test/api?method=initPayment'); + + $this->assertSame(1, $inner->callCount()); + $this->assertSame(0, $response->getStatusCode()); + $this->assertTrue($response->wasRequestSent()); + $this->assertSame([], $transport->sleeps()); + } + + /** + * A status means the server answered, so it processed the request far enough to + * decide. None of these may be repeated blindly — not even a 429, which other SDKs + * retry only because they can send an idempotency key with it. + * + * @dataProvider serverAnsweredStatuses + */ + public function testNeverRetriesAnyStatusTheServerReturned(int $status): void + { + $inner = new FakeTransport( + Response::received($status, 'body'), + Response::received(200, '{"result":{}}') + ); + $transport = new SleeplessRetryingTransport($inner, 3); + + $response = $transport->request('https://unitpay.test/api?method=initPayment'); + + $this->assertSame(1, $inner->callCount()); + $this->assertSame($status, $response->getStatusCode()); + } + + /** @return array */ + public function serverAnsweredStatuses(): array + { + return [ + 'status 409' => [409], + 'status 429' => [429], + 'status 500' => [500], + 'status 502' => [502], + 'status 503' => [503], + ]; + } + + public function testDoesNotRetryASuccess(): void + { + $inner = new FakeTransport('{"result":{}}'); + $transport = new SleeplessRetryingTransport($inner, 3); + + $transport->request('https://unitpay.test/api?method=getPayment'); + + $this->assertSame(1, $inner->callCount()); + } + + public function testStopsAtTheConfiguredLimitAndReturnsTheLastFailure(): void + { + $inner = new FakeTransport(Response::failed(6, 'Could not resolve host', false)); + $transport = new SleeplessRetryingTransport($inner, 2); + + $response = $transport->request('https://unitpay.test/api?method=getPayment'); + + // 1 initial attempt + 2 retries. + $this->assertSame(3, $inner->callCount()); + $this->assertSame(6, $response->getErrno()); + $this->assertCount(2, $transport->sleeps()); + } + + public function testZeroRetriesPerformsExactlyOneCall(): void + { + $inner = new FakeTransport(Response::failed(7, 'Connection refused', false)); + $transport = new SleeplessRetryingTransport($inner, 0); + + $transport->request('https://unitpay.test/api?method=getPayment'); + + $this->assertSame(1, $inner->callCount()); + $this->assertSame([], $transport->sleeps()); + } + + /** + * A retry must re-send the same bytes. If the URL or the headers were rebuilt between + * attempts, a signed request could be re-signed differently — or the fluent-setter + * params could be dropped, which is why retries live below the service layer at all. + */ + public function testEveryAttemptSendsIdenticalUrlAndHeaders(): void + { + $inner = new FakeTransport(Response::failed(7, 'Connection refused', false)); + $transport = new SleeplessRetryingTransport($inner, 2); + $url = 'https://unitpay.test/api?method=initPayment&sum=100&signature=abc'; + $headers = ['User-Agent: unitpay-php-sdk/4.0.0', 'X-Unitpay-Client: {"lang":"php"}']; + + $transport->request($url, $headers); + + $this->assertSame(3, $inner->callCount()); + $this->assertSame($url, $inner->url(0)); + $this->assertSame($url, $inner->url(1)); + $this->assertSame($url, $inner->url(2)); + foreach ([0, 1, 2] as $attempt) { + $this->assertSame('unitpay-php-sdk/4.0.0', $inner->header('User-Agent', $attempt)); + $this->assertSame('{"lang":"php"}', $inner->header('X-Unitpay-Client', $attempt)); + } + } + + /** Backoff grows and stays under the cap, so a retry storm cannot stall a request thread. */ + public function testBackoffIsExponentialAndCapped(): void + { + $inner = new FakeTransport(Response::failed(7, 'Connection refused', false)); + $transport = new SleeplessRetryingTransport($inner, 4, 0.5, 2.0); + + $transport->request('https://unitpay.test/api?method=getPayment'); + + $sleeps = $transport->sleeps(); + $this->assertCount(4, $sleeps); + foreach ($sleeps as $delay) { + $this->assertGreaterThanOrEqual(0.25, $delay); + $this->assertLessThanOrEqual(2.0, $delay); + } + } + + /** + * Retries must not be invisible: a caller staring at a network error needs to know the + * SDK already tried three times before giving up. + */ + public function testTheReturnedFailureReportsHowManyAttemptsWereMade(): void + { + $inner = new FakeTransport(Response::failed(7, 'Connection refused', false)); + $transport = new SleeplessRetryingTransport($inner, 2); + + $response = $transport->request('https://unitpay.test/api?method=getPayment'); + + $this->assertStringContainsString('Connection refused', $response->getTransportError()); + $this->assertStringContainsString('3 attempts', $response->getTransportError()); + } + + /** A single attempt is not "1 attempts", and it is not a retry story worth telling. */ + public function testASingleAttemptDoesNotClaimToHaveRetried(): void + { + $inner = new FakeTransport(Response::failed(7, 'Connection refused', false)); + $transport = new SleeplessRetryingTransport($inner, 0); + + $response = $transport->request('https://unitpay.test/api?method=getPayment'); + + $this->assertSame('Connection refused', $response->getTransportError()); + } + + /** + * @dataProvider invalidConfigurations + */ + public function testInvalidConfigurationIsRejectedAtConstruction( + int $maxRetries, + float $baseDelay, + float $maxDelay, + string $expectedMessage + ): void { + $this->expectException(UnitpayValidationException::class); + $this->expectExceptionMessage($expectedMessage); + + new RetryingTransport(new FakeTransport(), $maxRetries, $baseDelay, $maxDelay); + } + + /** @return array */ + public function invalidConfigurations(): array + { + return [ + 'negative retries' => [-1, 0.5, 2.0, 'Max retries must not be negative'], + 'negative base delay' => [2, -0.5, 2.0, 'Base delay must not be negative'], + 'max delay below base' => [2, 2.0, 0.5, 'Max delay must not be smaller than the base delay'], + ]; + } +} diff --git a/tests/Support/SleeplessRetryingTransport.php b/tests/Support/SleeplessRetryingTransport.php new file mode 100644 index 0000000..d134ee7 --- /dev/null +++ b/tests/Support/SleeplessRetryingTransport.php @@ -0,0 +1,26 @@ +sleeps; + } + + protected function sleep(float $seconds): void + { + $this->sleeps[] = $seconds; + } +} From 415f930b29380174ef39c2a60a992abcd2f820f8 Mon Sep 17 00:00:00 2001 From: Artem Dragunov Date: Sat, 25 Jul 2026 12:49:29 +0300 Subject: [PATCH 10/25] feat(webhook)!: reject replayed webhooks outside the tolerance window MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A signature proves a webhook was genuine once, not that it is genuine now: until this change a captured request could be replayed forever, and the SDK would accept it every time. checkHandlerRequest() now also verifies that params[date] is within 300s of the server clock, enabled by default and adjustable with setWebhookTolerance(0 disables). The check runs after the signature and before the IP allowlist, so an unsigned payload cannot be used to probe how far the clock is off. date is parsed with DateTimeImmutable::createFromFormat against a fixed +03:00 zone, never strtotime(): the API documents the format but not the timezone, and strtotime() would resolve it against the ambient date.timezone — making the same webhook pass on a Moscow server and fail on a UTC one, three hours being ten times the window. A regression test runs the same fixture under four timezones. An absent date is accepted rather than refused. It is documented for all four inbound methods, and it is inside the signed payload, so an attacker cannot strip it to skip the window — removing any param changes the hash and the signature check rejects the request first. Fail-closed would therefore prevent nothing while costing the merchant real notifications. A date that is present but unparseable or stale is still rejected. UnitpayReplayException extends UnitpaySignatureException, so a handler that already rejects on a bad signature keeps working unchanged. BREAKING CHANGE: webhooks whose date is more than 300s from the server clock are now rejected. Call setWebhookTolerance() to widen or disable the window. --- src/Exception/UnitpayReplayException.php | 15 ++ src/Webhook/WebhookVerifier.php | 130 +++++++++- tests/Support/FrozenClockWebhookVerifier.php | 27 ++ tests/Webhook/WebhookReplayTest.php | 255 +++++++++++++++++++ tests/Webhook/WebhookVerifierTest.php | 14 +- 5 files changed, 437 insertions(+), 4 deletions(-) create mode 100644 src/Exception/UnitpayReplayException.php create mode 100644 tests/Support/FrozenClockWebhookVerifier.php create mode 100644 tests/Webhook/WebhookReplayTest.php diff --git a/src/Exception/UnitpayReplayException.php b/src/Exception/UnitpayReplayException.php new file mode 100644 index 0000000..8b5e8ae --- /dev/null +++ b/src/Exception/UnitpayReplayException.php @@ -0,0 +1,15 @@ +|null */ private ?array $handlerParams = null; @@ -110,6 +130,10 @@ public function checkHandlerRequest(): bool throw new UnitpaySignatureException('Wrong signature'); } + // After the signature, never before it: an unsigned payload must not be usable to + // probe how far this server's clock is off. + $this->assertFresh($params); + if (!$this->isAllowedIp($ip)) { throw new UnitpayIpException('IP address Error'); } @@ -139,6 +163,28 @@ public function getHandlerParams(): ?array return $this->handlerParams; } + /** + * Sets how far the webhook's `date` may be from this server's clock, in seconds. + * Defaults to 300. Pass 0 to disable the check — a webhook then stays replayable for + * as long as its signature is valid, which is forever. + * + * Raise it if your handler sits behind a queue that can hold a webhook for minutes, + * or if the server clock is not synchronised. Both are better fixed than tolerated. + * + * @throws UnitpayValidationException on a negative value + */ + public function setWebhookTolerance(int $seconds): self + { + if ($seconds < 0) { + throw new UnitpayValidationException( + sprintf('Webhook tolerance must not be negative, got %d.', $seconds) + ); + } + $this->tolerance = $seconds; + + return $this; + } + /** * Overrides the list of Unitpay IPs allowed to call the handler. Fully replaces the * built-in default (or previously fetched) list, but does NOT touch the merchant IPs @@ -242,6 +288,84 @@ public function getErrorHandlerResponse(string $message): string return (string) json_encode(['error' => ['message' => $message]]); } + /** + * Current unix time. A seam so tests can move the clock without sleeping — the same + * role getIp() and isAllowedIp() play for the request. + */ + protected function now(): int + { + return time(); + } + + /** + * Rejects a webhook whose payment timestamp is too far from this server's clock. + * + * Safe to trust because `date` is part of the signed payload — SignatureBuilder hashes + * every param except the signature itself — so it cannot be back-dated, and it cannot + * be stripped either: removing it changes the hash and the signature check above fails + * first. That last point is why an absent `date` is allowed through rather than + * refused. It is documented for all four inbound methods, but if Unitpay ever omits it + * the absence is theirs, not an attacker's, and rejecting those webhooks would cost + * the merchant real notifications while preventing nothing. + * + * @param array $params + * @throws UnitpayReplayException when the timestamp is unreadable or outside the window + */ + private function assertFresh(array $params): void + { + if ($this->tolerance === 0 || !isset($params['date'])) { + return; + } + + $timestamp = self::parseWebhookDate($params['date']); + if ($timestamp === null) { + throw new UnitpayReplayException(sprintf( + 'Webhook timestamp "%s" is not a valid Y-m-d H:i:s date.', + is_scalar($params['date']) ? (string) $params['date'] : gettype($params['date']) + )); + } + + $drift = abs($this->now() - $timestamp); + if ($drift > $this->tolerance) { + throw new UnitpayReplayException(sprintf( + 'Webhook timestamp %s is outside the %ds tolerance window (off by %ds).', + (string) $params['date'], + $this->tolerance, + $drift + )); + } + } + + /** + * @param mixed $value + * @return int|null unix timestamp, or null when the value is not a well-formed date + */ + private static function parseWebhookDate($value): ?int + { + if (!is_string($value)) { + return null; + } + + $date = DateTimeImmutable::createFromFormat( + 'Y-m-d H:i:s', + $value, + new DateTimeZone(self::WEBHOOK_TIMEZONE) + ); + if ($date === false) { + return null; + } + + // createFromFormat accepts overflowing components ("2026-13-45") and reports them + // as warnings instead of failing, so the strict answer needs this second look. + // PHP >= 8.2 returns false here when there is nothing to report. + $errors = DateTimeImmutable::getLastErrors(); + if (is_array($errors) && ($errors['warning_count'] > 0 || $errors['error_count'] > 0)) { + return null; + } + + return $date->getTimestamp(); + } + /** * Sender IP of the inbound request (the overridden clientIp or $_SERVER['REMOTE_ADDR']). * Override for proxy-aware logic. diff --git a/tests/Support/FrozenClockWebhookVerifier.php b/tests/Support/FrozenClockWebhookVerifier.php new file mode 100644 index 0000000..d67123b --- /dev/null +++ b/tests/Support/FrozenClockWebhookVerifier.php @@ -0,0 +1,27 @@ +now = $timestamp; + + return $this; + } + + protected function now(): int + { + return $this->now; + } +} diff --git a/tests/Webhook/WebhookReplayTest.php b/tests/Webhook/WebhookReplayTest.php new file mode 100644 index 0000000..ad801c1 --- /dev/null +++ b/tests/Webhook/WebhookReplayTest.php @@ -0,0 +1,255 @@ + $overrides + * @return array{method: string, params: array} + */ + private function requestAt(int $timestamp, string $method = 'pay', array $overrides = []): array + { + $params = array_merge([ + 'account' => '42', + 'orderSum' => '100.00', + 'unitpayId' => '999', + 'date' => WebhookVerifierTest::dateAt($timestamp), + ], $overrides); + + $params['signature'] = (new SignatureBuilder())->build($params, self::SECRET, $method); + + return ['method' => $method, 'params' => $params]; + } + + /** + * @param array{method: string, params: array} $request + */ + private function verifier(array $request): FrozenClockWebhookVerifier + { + $verifier = new FrozenClockWebhookVerifier( + self::SECRET, + new SignatureBuilder(), + new FakeTransport(), + 'https://unitpay.ru/ips/ips_webhooks.json', + $request, + self::ALLOWED_IP + ); + + return $verifier->freezeAt(self::NOW); + } + + public function testAFreshWebhookPasses(): void + { + $this->assertTrue($this->verifier($this->requestAt(self::NOW))->checkHandlerRequest()); + } + + public function testAWebhookFromTenMinutesAgoIsRejected(): void + { + $this->expectException(UnitpayReplayException::class); + $this->expectExceptionMessage('outside the 300s tolerance window'); + + $this->verifier($this->requestAt(self::NOW - 600))->checkHandlerRequest(); + } + + /** Clock skew cuts both ways: a timestamp from the future is just as untrustworthy. */ + public function testAWebhookFromTenMinutesInTheFutureIsRejected(): void + { + $this->expectException(UnitpayReplayException::class); + + $this->verifier($this->requestAt(self::NOW + 600))->checkHandlerRequest(); + } + + /** + * @dataProvider boundaryOffsets + */ + public function testTheBoundaryIsInclusive(int $offset): void + { + $this->assertTrue($this->verifier($this->requestAt(self::NOW + $offset))->checkHandlerRequest()); + } + + /** @return array */ + public function boundaryOffsets(): array + { + return [ + 'exactly 300s old' => [-300], + 'exactly 300s ahead' => [300], + ]; + } + + /** + * @dataProvider justOutsideOffsets + */ + public function testOneSecondPastTheBoundaryIsRejected(int $offset): void + { + $this->expectException(UnitpayReplayException::class); + + $this->verifier($this->requestAt(self::NOW + $offset))->checkHandlerRequest(); + } + + /** @return array */ + public function justOutsideOffsets(): array + { + return [ + '301s old' => [-301], + '301s ahead' => [301], + ]; + } + + public function testToleranceZeroDisablesTheCheck(): void + { + $verifier = $this->verifier($this->requestAt(self::NOW - 86400))->setWebhookTolerance(0); + + $this->assertTrue($verifier->checkHandlerRequest()); + } + + public function testToleranceIsConfigurable(): void + { + $verifier = $this->verifier($this->requestAt(self::NOW - 600))->setWebhookTolerance(900); + + $this->assertTrue($verifier->checkHandlerRequest()); + } + + public function testNegativeToleranceIsRejected(): void + { + $this->expectException(UnitpayValidationException::class); + $this->expectExceptionMessage('Webhook tolerance must not be negative'); + + $this->verifier($this->requestAt(self::NOW))->setWebhookTolerance(-1); + } + + /** + * A date the SDK cannot parse is a failed freshness check, not something to wave + * through — otherwise garbage in the field would silently disable the window. + * + * @dataProvider malformedDates + */ + public function testAMalformedDateIsRejected(string $date): void + { + $this->expectException(UnitpayReplayException::class); + + $this->verifier($this->requestAt(self::NOW, 'pay', ['date' => $date]))->checkHandlerRequest(); + } + + /** @return array */ + public function malformedDates(): array + { + return [ + 'not a date' => ['not-a-date'], + 'impossible components' => ['2026-13-45 99:99:99'], + 'wrong format' => ['25/07/2026 12:00'], + 'empty' => [''], + ]; + } + + /** + * The regression this whole design exists to prevent. `strtotime()` would resolve the + * same string against the ambient date.timezone, so a webhook accepted on a Moscow + * server would be rejected on a UTC one — three hours is ten times the window. + * + * @dataProvider serverTimezones + */ + public function testTheVerdictDoesNotDependOnTheServerTimezone(string $timezone): void + { + $original = date_default_timezone_get(); + date_default_timezone_set($timezone); + + try { + $this->assertTrue($this->verifier($this->requestAt(self::NOW))->checkHandlerRequest()); + } finally { + date_default_timezone_set($original); + } + } + + /** @return array */ + public function serverTimezones(): array + { + return [ + 'UTC' => ['UTC'], + 'Europe/Moscow' => ['Europe/Moscow'], + 'America/New_York' => ['America/New_York'], + 'Asia/Tokyo' => ['Asia/Tokyo'], + ]; + } + + /** + * Ordering matters: the signature gate stays first, so an attacker cannot use an + * unsigned payload to probe how far the server clock is off. + */ + public function testABadSignatureIsReportedBeforeAStaleDate(): void + { + $request = $this->requestAt(self::NOW - 86400); + $request['params']['orderSum'] = '0.01'; // invalidates the signature + + $this->expectException(UnitpaySignatureException::class); + $this->expectExceptionMessage('Wrong signature'); + + $this->verifier($request)->checkHandlerRequest(); + } + + /** + * `date` is documented for all four inbound methods, but it is also part of the signed + * payload — an attacker cannot strip it, because removing any param changes the hash + * and the signature check fails first. So if it is ever genuinely absent, that is + * Unitpay's choice, not an attack, and rejecting every such webhook would cost the + * merchant real notifications for no security gain. + */ + public function testAWebhookWithoutADateIsAcceptedRatherThanBlocked(): void + { + $params = ['account' => '42', 'orderSum' => '100.00', 'unitpayId' => '999']; + $params['signature'] = (new SignatureBuilder())->build($params, self::SECRET, 'pay'); + + $this->assertTrue($this->verifier(['method' => 'pay', 'params' => $params])->checkHandlerRequest()); + } + + /** The window applies to every inbound method, not only to `pay`. */ + public function testTheWindowAppliesToEveryInboundMethod(): void + { + foreach (['check', 'pay', 'preauth', 'error'] as $method) { + $fresh = $this->verifier($this->requestAt(self::NOW, $method)); + $this->assertTrue($fresh->checkHandlerRequest(), $method . ' should pass when fresh'); + + try { + $this->verifier($this->requestAt(self::NOW - 3600, $method))->checkHandlerRequest(); + $this->fail($method . ' should be rejected when stale'); + } catch (UnitpayReplayException $e) { + $this->addToAssertionCount(1); + } + } + } + + /** Still catchable by handlers written against the pre-4.0 exception surface. */ + public function testReplayExceptionIsCaughtBySignatureExceptionHandlers(): void + { + try { + $this->verifier($this->requestAt(self::NOW - 600))->checkHandlerRequest(); + $this->fail('expected a replay exception'); + } catch (UnitpaySignatureException $e) { + $this->assertInstanceOf(UnitpayReplayException::class, $e); + $this->assertInstanceOf(WebhookVerifier::class, $this->verifier($this->requestAt(self::NOW))); + } + } +} diff --git a/tests/Webhook/WebhookVerifierTest.php b/tests/Webhook/WebhookVerifierTest.php index ba952fe..6f9e483 100644 --- a/tests/Webhook/WebhookVerifierTest.php +++ b/tests/Webhook/WebhookVerifierTest.php @@ -30,7 +30,9 @@ private function validRequest(string $method = 'pay', array $overrides = []): ar 'account' => '42', 'orderSum' => '100.00', 'orderCurrency' => 'RUB', - 'date' => '2026-07-20 12:00:00', + // Fresh by default: the replay window is on out of the box, so a fixed date + // would make every webhook test start failing the moment it went stale. + 'date' => self::dateAt(time()), 'payerSum' => '100.00', 'unitpayId' => '999', ], $overrides); @@ -40,6 +42,16 @@ private function validRequest(string $method = 'pay', array $overrides = []): ar return ['method' => $method, 'params' => $params]; } + /** + * Formats a unix timestamp the way Unitpay sends it: `Y-m-d H:i:s` wall clock in + * UTC+3. gmdate() rather than date() so the fixture does not shift with the ambient + * date.timezone — which is exactly what the timezone regression test below checks. + */ + public static function dateAt(int $timestamp): string + { + return gmdate('Y-m-d H:i:s', $timestamp + 3 * 3600); + } + /** * @param array $params */ From 936a40f200b9d4c516f2df476f17a9aca88c6056 Mon Sep 17 00:00:00 2001 From: Artem Dragunov Date: Sat, 25 Jul 2026 12:54:21 +0300 Subject: [PATCH 11/25] feat(telemetry): CMS/framework/module slots and an opt-out MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fingerprint was fixed, so it could say "PHP 8.1 on Darwin" but never which integration was actually talking. Most Unitpay integrations are CMS modules, which makes "unitpay-bitrix 3.1 on Bitrix 22" the part worth having and exactly the part that could not be expressed. setCms(), setFramework() and setModule() fill named slots that ride along in both User-Agent and X-Unitpay-Client; an unset slot is omitted rather than sent empty, and a half-filled one is rejected. disableTelemetry() drops X-Unitpay-Client entirely while the User-Agent keeps naming the SDK and its version, since a request that identifies nothing is harder to support. Telemetry\ClientInfo is a new leaf layer holding the slots and building the headers, which AbstractService previously did inline. It is shared by reference like Api\PendingParams: the facade caches service objects on first use, so a slot set after the first payments() call still has to reach the service that already exists. There is a test for exactly that. The default payload is unchanged, so the existing telemetry assertions hold without edits. PHPMD: the facade reached a CouplingBetweenObjects of 13, the stock limit, by gaining ClientInfo. Measured across src/ the limit is raised to 14 with the real numbers recorded — the loosening also covers WebhookVerifier at 12 and AbstractService at 11, which the comment says out loud rather than implying the exemption is targeted. --- phpmd.xml | 28 +++++++- src/Api/AbstractService.php | 33 ++-------- src/Telemetry/ClientInfo.php | 121 +++++++++++++++++++++++++++++++++++ src/Unitpay.php | 55 +++++++++++++++- tests/Api/TelemetryTest.php | 93 +++++++++++++++++++++++++++ 5 files changed, 299 insertions(+), 31 deletions(-) create mode 100644 src/Telemetry/ClientInfo.php diff --git a/phpmd.xml b/phpmd.xml index a23a707..b9226a4 100644 --- a/phpmd.xml +++ b/phpmd.xml @@ -62,7 +62,33 @@ - + + + + + + + + + + +