Release/4.0.0 - #27
Merged
Merged
Conversation
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.
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.
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.
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.
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.
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.
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.
…nse object
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.
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.
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.
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.
docs/migration-v4.md leads with the only change most integrations have to act on — the TransportInterface rewrite — and is explicit that the third argument to Response::failed() is a safety decision rather than a detail: pass false when you cannot tell whether the request went out, because the retry policy reads it and the API has no idempotency key. api-methods.md gains the exception table with a "safe to repeat?" column, since that is the question the class is there to answer. webhooks.md documents the replay window, the UTC+3 assumption and why an absent date is accepted; its existing allowlist section now also states the worst-case wall clock that retries add to refreshAllowedIps(). telemetry.md documents the slots and the opt-out, and repeats that the values are merchant-supplied product names. examples/paymentInfo.php catches the three subclasses separately, which is the only way the difference between them is visible to someone reading the samples rather than the docs. TECH_DEBT_AUDIT.md moves F008 to the closed register, leaving three open findings. The note recommending F008 and F015 be done together is corrected rather than deleted: they were separated on purpose, because one is about the transport and the other about the payload.
Retries being on by default is stated in the CHANGELOG and in docs/getting-started.md, and it lives in one line of the facade: `$transport ?? DefaultTransport::create()`. Every existing test injected a FakeTransport, so that line never ran under test — replacing it with a bare `new CurlTransport()` left all 260 tests green while retries silently vanished from every default installation. DefaultTransportTest covers the stack through the public API; the facade test covers the wiring itself via reflection, since which transport was composed is an implementation detail that should not enter the API just to be observable. Both are needed: breaking DefaultTransport::create() fails both, while making the facade bypass DefaultTransport fails only the second. Also corrects the docblock on testFacadeIsUsableWithoutAnInjectedTransport, which still claimed the fallback was CurlTransport — untrue since 4.0.0.
On a host without ext-curl the file_get_contents fallback was retried, so a failure while reading the response to initPayment could create a second payment. The API accepts no idempotency key, so nothing outside the SDK would have absorbed the duplicate. Measured: 3 attempts where the documentation guaranteed 1. Response::wasRequestSent() was carrying two different claims as one boolean — "provably not sent" (cURL reports CURLINFO_CONNECT_TIME of 0.0) and "unknown" (the fallback sees no connect/read phase). shouldRetry() read the absence of true as positive knowledge that a repeat was safe, which turned the strictest possible rule into its opposite on that one path. ERRNO_LOCAL now carries "unknown" instead, and shouldRetry() requires all three exclusions: no status, not sent, and classifiable. wasRequestSent() keeps its single honest meaning and a custom transport gains a correct way to say it cannot tell. docs/migration-v4.md told custom transport authors the inverse — "if you cannot tell, pass false … the SDK never retries through your transport" — which is corrected here, since false alone is the claim strong enough to license a retry.
The Unitpay API takes secretKey as a query parameter, so the key travels through the SDK as an ordinary argument and lands in the arguments of the stack frames. Measured: with zend.exception_ignore_args=0, $e->getTrace() returns the key in full (it sits inside the $params array), and getTraceAsString()/(string) $e expose its first 15 characters wherever the key was a direct string argument — Unitpay::__construct(), SignatureBuilder::build(). Under PHP's default of 1 every channel is clean, and getMessage() is clean either way. Not a 4.0.0 regression: this is as old as the API contract and present in 3.x. What 4.0.0 changes is that there is now something better to log — the typed exceptions and their accessors — so the guidance belongs with them. Error trackers are the realistic exposure: Sentry, Bugsnag and Monolog's IntrospectionProcessor all serialize frame arguments when PHP hands them any. Documented in api-methods.md with the per-channel table, referenced from the migration guide next to the logging example, and recorded in TECH_DEBT_AUDIT.md as the second leak channel of F022 — both channels close at once if the backend ever accepts POST for these methods, so they are tracked as one finding rather than two.
IpAllowlist has always called ctype_digit() unconditionally, and unlike ext-json — compiled in unconditionally since PHP 8.0 — ctype is a bundled extension a build can still drop with --disable-ctype, so the dependency was real and undeclared. It lands in a major because it is a stricter install constraint. Still no Composer packages at runtime, and ext-curl stays in suggest. ComposerRequirementsTest holds the manifest to the code: it scans src/ with the tokenizer for calls into extensions a build can disable, asserts each one is declared in require (or in suggest, which is ext-curl and only ext-curl), and asserts require carries nothing but php and ext-*. examples/ is the declared source of truth for usage samples but sat outside phpstan.neon, and composer lint only syntax-checks it, so a renamed method or a changed argument count stayed invisible while the whole gate was green. phpstan-examples.neon runs at level 2, because method existence is checked on expressions only from that level up and the examples call everything through a variable, never through $this. The single variable.undefined identifier is suppressed — every example pulls its variables in through require of config.php / order.php, which PHPStan does not follow — and nothing else is. Both PHPStan scripts pin memory_limit=512M: PHP's 128M default aborts the parallel worker, which used to make a bare composer check fail for an environment reason that read as a code finding.
Two wire-format changes that have to land before the 4.0.0 tag, because a published
install keeps sending whatever shape it shipped with for years.
X-Unitpay-Client becomes Unitpay-Client. RFC 6648 retired the X- convention in 2012, and
four more SDKs (Java, Node, .NET, Python) are planned against this same header — fixing
the name after five implementations ship would mean a permanently two-named protocol. The
old name went out in 2.1.0 and 2.x/3.x installs will keep sending it regardless, so a
consumer has to accept both either way; emitting both from 4.0.0 would double the header
weight forever without removing that obligation, and is not done.
Each filled slot is now an object rather than a joined string:
"cms": {"name": "Bitrix", "version": "22.0"}
The join forced a consumer to split on "/", which is not a reliable delimiter — a name may
contain it, and the most natural thing an integrator passes as a module name is the
Composer package name, so setModule('unitpay/woocommerce', '2.1') produced
"unitpay/woocommerce/2.1": three segments, one separator, no way to tell where the name
ended. The two halves are now stored apart and joined in exactly one place, the
User-Agent, which cannot carry structure anyway.
json_encode gains JSON_UNESCAPED_SLASHES so a slash in a name reads normally. Not
JSON_UNESCAPED_UNICODE: the \uXXXX escaping is what keeps the header value pure ASCII.
The fixed fields (sdk_version, api_version, lang, lang_version, platform, publisher) keep
their flat shape — they shipped in 2.1.0 and restructuring them would be a second break
for no gain.
Three defects in the slot handling added by 936a40f, none of which a merchant could have worked around, plus the sink-side guard for the third. Telemetry no longer raises on merchant input. setSlot() rejected a blank name or version with UnitpayValidationException, and these setters run in an integration's bootstrap — a CMS that stops exposing its version string, an ordinary outcome of any CMS update, turned a cosmetic concern into a failed checkout. An unusable slot is now dropped and the request goes out without it. The unknown-slot exception stays: the facade passes only the constants, so it guards internal misuse rather than merchant input. Slot values are sanitized where they come in: control characters are stripped, and the halves are capped at 64 and 32 bytes with any incomplete UTF-8 sequence left by the cut dropped, since ext-mbstring is not a dependency. trim() only cleaned the edges, so setModule("evil\r\nX-Injected: 1", "1.0") used to reach the transport, which joins header lines with "\r\n" on the stream path and duly sent the injected line. Slot values commonly come from a module's own settings screen. An invalidly encoded value can no longer blank the payload. json_encode returns false on invalid UTF-8 and the (string) cast turned that into an empty header, so a single legacy windows-1251 CMS name cost sdk_version, lang_version and everything else. JSON_INVALID_UTF8_SUBSTITUTE keeps the payload; if encoding somehow still fails the header is omitted rather than sent blank. A slot joins the User-Agent only when both halves are printable ASCII — that field cannot carry raw UTF-8, and the JSON header has it either way, so an absent token beats a mangled one. CurlTransport::sanitizeHeaders() drops any header line carrying a CR or LF. Nothing the SDK builds can contain one now, which is why this is the second line of defence; it drops the line rather than raising, because a transport must not abort a payment over a diagnostic header.
docs/telemetry.md carries the new header name, the object-shaped slot payload, and a new section stating what a slot can and cannot do to a request: an unusable value is ignored rather than rejected, control characters are stripped, each half is capped, and a non-ASCII name rides in the JSON header only. docs/migration-v4.md gains the rename as a third breaking change. It affects an integrator only if something on their side reads the SDK's own outgoing headers, and it notes that 2.x and 3.x installs keep sending the old name, so a consumer has to accept both. The v4.0.0 changelog entry gains the rename and the input-handling guarantees. The v2.1.0 entry is left alone: 2.1.0 really did ship X-Unitpay-Client, and rewriting it would make the changelog lie. ARCHITECTURE.md and DESCRIPTION.md name the header in the folder map, the request-pipeline description and the feature list; the benchmark reference describes what 4.0.0 closed and is synced too, minus its accurate statement about what v3 sent. docs/superpowers/** is deliberately untouched — those are design records of a pre-3.0 iteration, not documentation of the shipped SDK.
"the slot is simply not sent" only described the first call. Ignoring a blank name or version means leaving the slot as it was, so an already-reported value survives a later setter that comes back empty — which is the behavior worth having, since a CMS update that breaks the version lookup should cost the update rather than the value already known. The wording now says so, the reasoning is recorded next to the early return, and a test pins it so a refactor cannot quietly turn "ignore" into "clear".
EXCISE, GAMBLING_BET, GAMBLING_PRIZE, LOTTERY_PRIZE and COMPOSITE are gone from Unitpay\Model\Enum\PaymentObject. The public API rejects all five, so no receipt has ever fiscalized with one. They were announced for removal in 2.1.0 (for 3.0), kept once to avoid stacking a dictionary change on top of the namespace break, and re-slated for 4.0 by their own @deprecated docblocks. Shipping 4.0.0 with them still present would have made those docblocks a lie and forfeited the break window until 5.0. BREAKING CHANGE: code naming one of the five constants fails to load with Error: Undefined constant. Code passing the raw string ('excise' and friends) is unaffected — CashItem does not validate $type against a whitelist, so the value travels to the backend and is rejected there exactly as before. PaymentObjectTest pins both halves of the dictionary through reflection: the five names must stay absent, and the surviving 25 must match value for value. Reflection rather than direct references because naming a removed constant in test code would be a PHPStan failure and then a fatal Error. The whole-map assertion also makes an undocumented backend addition fail until it is recorded in CHANGELOG.md.
CHANGELOG gets a breaking-change bullet in the v4.0.0 entry. The v3.0.0 bullet that recorded keeping the values is left alone — it was true when written and stays as the historical record. migration-v4.md gets section 5, which splits the impact by how the value is named rather than by which value it is: passing the constant breaks at load time, passing the raw string changes nothing. Includes the grep to find the first case. The intro said "Three things break" and the table listed three; both now count four. receipts.md loses the paragraph advertising the five values as available for backward compatibility. It is the how-to page for building a receipt today, and the removal is documented in the two places above.
setCms() and setFramework() are gone. What replaces them is a division an integrator cannot
get wrong: setModule() names what they wrote, setStack() names what it runs on.
$unitpay->setModule('unitpay-woocommerce', '2.1')
->setStack(['WordPress' => '6.5', 'WooCommerce' => '8.2']);
The three slots asked the integrator to classify their own environment, and the taxonomy did
not fit the install base. WooCommerce is neither a CMS nor a framework, so it had to be filed
under one of them or dropped; Bitrix plus an Aspro solution plus the payment module is four
layers into three slots; a Telegram-bot template running on nothing at all invited a made-up
value just to fill a slot in.
A stack entry is a known product — WordPress, Bitrix, Laravel — that a server can categorise
from its name with a table it can fix without shipping new SDKs. Grouping needs no
classification at all: "how many on WordPress 6.5" is a filter on the name. An unknown product
now arrives unclassified instead of misclassified, which is the better failure.
A module name is arbitrary and no table can recognise it, which is why it keeps a field of its
own rather than being the last stack entry.
"module": {"name": "unitpay-bitrix", "version": "3.1"},
"stack": [{"name": "Bitrix", "version": "22.0"}, {"name": "Aspro Optimus", "version": "1.8"}]
setStack() replaces rather than appends, so it is idempotent in a bootstrap that runs twice; a
forked template adds a line to the same array. Append can be introduced later, removing it
could not. An associative array makes a duplicate product name impossible and preserves the
outermost-host-first order, which is presentation only — the User-Agent reads like the stack it
describes, and grouping is order-independent.
The runtime is not part of the stack: lang, lang_version and platform are still filled in by
the SDK. The rule for an integrator is "if the SDK can find it out itself, it is not stack".
Bounds and input handling are unchanged in substance and now apply per entry, verified by
test: nothing throws, control characters are stripped at the source, each half is capped at
64/32 bytes, a blank half drops its own entry without taking its neighbours, a non-ASCII name
rides in the JSON header only, and an unencodable payload omits the header rather than sending
it blank. New with the list: at most eight entries, because three fixed slots bounded the
header for free and a list does not.
Telemetry/ now imports nothing — the unknown-slot exception went away with setSlot().
Nothing that shipped is being removed: setCms() and setFramework() were added in this same
unreleased 4.0.0.
docs/telemetry.md rewrites "Naming your integration" around the two calls: what you wrote and what it runs on. Adds the rule that keeps the question from coming up — the runtime is not stack, because the SDK reports it itself — worked examples for a CMS plugin, a four-layer stack, a CMS fork, a bare app and an empty stack, and the input-handling guarantees restated per entry with the eight-entry cap. A new section covers templates, which behave unlike modules in three ways worth stating: the version freezes at the moment someone cloned, the code is forked immediately, and the identification line is the one most likely to be renamed. It ships with the "do not rename, add yourself to the stack instead" snippet, which also gives whoever forked it a better option than overwriting the origin. The v4.0.0 changelog entry and the migration guide's telemetry bullet name the new API. Neither describes a removal: setCms() and setFramework() were added in this same unreleased release, so nothing that shipped is going away. ARCHITECTURE.md corrects the dependency line — Telemetry/ now imports nothing, the unknown-slot exception having gone with setSlot() — and the folder map. DESCRIPTION.md and examples/config.php follow. The benchmark reference records why neither competitor shape was kept: YooKassa's three slots ask the integrator to classify their own environment and cannot express four layers, and Stripe's single app_info answers "who is calling" while dropping the host versions, which are the signal worth collecting. Its quotes of YooKassa's own setCms/setFramework source stay as they are.
The v4 migration guide presented `Unitpay-Client` as carrying a `cms` key. No released version ever sent one: 3.x had no integration slot at all, and 4.0.0 replaced the short-lived cms/framework/module slots with `module` plus `stack` before the tag, so both sides of that diff block described a format that never existed. Section 4 now shows the header rename as the only break there and introduces `module` and `stack` as new optional fields, matching what `Telemetry\ClientInfo::clientJson()` actually emits. The CHANGELOG bullet loses the same never-shipped `"Bitrix/22.0"` framing. Three smaller inaccuracies found in the same pass: * the README docs table and the migration guide still named the removed slots; * `composer.json` described `stan-examples` as level 0 while `phpstan-examples.neon` pins level 2, and left it out of the `check` description entirely; * the `v4.0.0` heading carried 2026-07-25 while the entry now covers work committed on 2026-07-30. `composer check` green: 290 tests, 615 assertions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Breaking changes
TransportInterface::send(): string|falsebecamerequest(): Response. The old contractthrew away the status code, 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_fopenand malformed JSONall reached the caller as the same
UnitpayTransportException('Temporary server error. Please try again later.')— including thetwo that are permanent rather than temporary.
Unitpay\Http\Responsecarries the status, body,headers, errno and a
wasRequestSent()flag, and is built throughreceived()/failed()soa nonsensical state cannot be expressed. Only custom transport implementations are affected.
UnitpayTransportExceptionstays the base and still extendsInvalidArgumentException, so existingcatchblocks keep working, but it now has threesubclasses:
UnitpayNetworkException(no response, withgetErrno()/getTransportError()),UnitpayHttpException(non-2xx, withgetStatusCode()/getResponseBody()) andUnitpayResponseException(2xx whose body is not a JSON object). The blanket message is gone —if you matched on it, match on the class.
UnitpayReplayException. Asignature proves a webhook was genuine once, not that it is genuine now; until this release a
captured request replayed indefinitely.
UnitpayReplayExceptionextendsUnitpaySignatureException, so a handler that already rejects a bad signature needs no change.Widen or disable with
webhook()->setWebhookTolerance().X-Unitpay-Client→Unitpay-Client. RFC 6648retired the
X-convention in 2012, and four more SDKs are planned against this same header.Affects you only if something on your side reads the SDK's own outgoing headers.
PaymentObjectvalues were removed:EXCISE,GAMBLING_BET,GAMBLING_PRIZE,LOTTERY_PRIZE,COMPOSITE. The public API rejects all five, so no receipthas ever fiscalized with one. They were announced in 2.1.0 for removal in 3.0, deferred once by
Replace the single-file SDK with a layered PSR-4 package #26 to avoid stacking a dictionary change on the namespace break, and re-slated for 4.0 — this
is that removal.
CashItemdoes not validate$typeagainst a whitelist, so a merchant passingthe raw string
'excise'is unaffected; only code naming the constant breaks, and it breaksloudly at load time.
ext-ctypeis now declared incomposer.json.IpAllowlisthas always calledctype_digit()unconditionally, and unlikeext-json— compiled in unconditionally sincePHP 8.0 —
ctypeis a bundled extension a build can still drop with--disable-ctype, so thedependency was real and undeclared. A stricter install constraint, which is why it lands in a
major.
New
RetryingTransportwraps anotherTransportInterfaceandrepeats an attempt only when it provably never reached Unitpay, up to twice with capped
exponential backoff and jitter.
DefaultTransport::create()is the default stack;DefaultTransport::withoutRetries()is the documented off switch. The returned failure stateshow many attempts were made, so a retried failure is not indistinguishable from a single one.
new CurlTransport($connectTimeout, $timeout), defaultingto the previous 5s and 10s. A non-positive value is rejected at construction, since cURL reads
0 as "wait forever".
setModule('unitpay-woocommerce', '2.1')names what youwrote,
setStack(['WordPress' => '6.5', 'WooCommerce' => '8.2'])names what it runs on. Bothride in
User-AgentandUnitpay-Client;disableTelemetry()drops the JSON header entirelywhile the
User-Agentkeeps naming the SDK. Values are product names and versions supplied bythe integrator — no PII, no identifiers, no extra request.
none of them raises: a blank half is dropped rather than rejected, control characters are
stripped, each half is capped at 64/32 bytes, at most eight stack entries are sent, and an
invalidly encoded value no longer blanks the whole payload.
CurlTransport::sanitizeHeaders()drops any header line carrying CR or LF as a second line of defence.
secretKeyas a query parameter, so it sits in the arguments of the stack frames and
$e->getTrace()returns it in full when
zend.exception_ignore_argsis switched off. Not new — as old as theAPI contract — but 4.0 is the release that gives you typed accessors worth logging instead.
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.
AbstractService::request()used to drain the accumulated params on every method, sosetCashItems()followed by any lookup sent the 54-FZ receipt with that lookup — where it ismeaningless — and the payment created right after it carried no receipt at all. Nothing reported
the loss. Only
form(),initPayment()andoffsetAdvance()read and drain them now. This isalso what makes transport-level retries safe: a retry re-sends identical bytes instead of
re-entering a drained holder.
setAllowedIps()andaddAllowedIps()reject malformed entries instead of accepting them.A typo such as
31.186.100.4 9or a/33prefix produced an allowlist that matched nothing, soevery webhook was refused with a bare
IP address Error. The whole call is rejected rather thanthe offending entry alone, so the allowlist is never left half-configured.
$domainmust be a bare host and is validated at construction. It feedsthree URLs, so a value carrying a scheme or a path used to produce malformed URLs whose failure
surfaced much later.
ruleset written for the pre-3.0 single-file class was replaced by one carrying only the
exclusions that actually fire;
examples/came under static analysis viacomposer stan-examples; and both PHPStan scripts pinmemory_limit=512M— which closes theenvironment-dependence caveat Replace the single-file SDK with a layered PSR-4 package #26 left open as a follow-up.
>=7.4range; a taggedrelease verifies that
Unitpay::VERSIONmatches the tag before publishing anything; andcomposer auditno longer swallows failures.Explicit decisions
3.1.0 is never tagged. Packagist caches a tag, so shipping different code under a published
tag yields two versions with one name and silently divergent lock files. 3.0.0 stays as published;
the 3.1.0 work ships here.
The retry policy is deliberately far narrower than other payment SDKs'. Item "idempotency
key" was dropped from scope because the Unitpay API accepts none, which removes the safety net
Stripe and Adyen rely on. So only a failure that provably never left the client is retried: DNS
failure, refused connection, connect-phase timeout. A read timeout, a 5xx, a 409 and a 429 are
not retried — in every one of those cases the server may already have created the payment.
Two consequences worth calling out in review:
CURLE_OPERATION_TIMEDOUTfor both a connect timeout and a read timeout, so theerrno cannot separate them.
CURLINFO_CONNECT_TIMEcan, and the measurement table provingCURLINFO_PRETRANSFER_TIMEcannot is reproduced inCurlTransport::wasRequestSent()so thenext reader does not simplify it back.
file_get_contentsfallback sees no connect/read phase, so it reportsResponse::ERRNO_LOCALand is never retried. "Unknown" is not "not sent".
No compatibility shim and no second interface for the transport. Because 4.0.0 is a major, BC
is not a design constraint:
TransportInterfacechanged directly rather than being shadowed. Themigration is ~10 lines and is documented.
Only one client header is emitted, not both.
X-Unitpay-Clientshipped in 2.1.0, so the oldname keeps arriving from 2.x/3.x installs indefinitely and the backend has to recognise two names
regardless. Emitting both from 4.0.0 would double the header weight forever without removing that
obligation.
module+stackrather than namedcms/framework/moduleslots. The three-slottaxonomy shipped earlier on this branch and was replaced before the tag, so nothing published is
being removed. It asked the integrator to classify their own environment, and the categories do
not fit the install base: WooCommerce is neither a CMS nor a framework, and WordPress +
WooCommerce + plugin is four layers into three slots. Stripe's single
app_inforecord wasconsidered and rejected for the opposite reason — it answers "who is calling" and drops the host
versions, which are the signal worth collecting. No schema-version field, and no client-side
normalization of
platform/lang_version: vocabulary belongs to the server, which can befixed, whereas installed copies cannot.
A webhook carrying no
dateis accepted rather than refused.dateis inside the signedpayload, so an attacker cannot strip it to skip the window — removing any param changes the hash,
and the signature check runs first. An absent
datecan therefore only come from Unitpay itself,and failing closed would cost the merchant real notifications while preventing nothing. A
datethat is present but unparseable or stale still fails closed. The timezone is a fixed UTC+3
offset, parsed with
DateTimeImmutable::createFromFormatrather thanstrtotime(), which wouldresolve against the ambient
date.timezoneand make the same webhook pass on a Moscow server andfail on a UTC one.
Deliberately out of scope, each a separate decision rather than an oversight: a typed response
DTO (F015 — about the response type, not the transport contract),
declare(strict_types=1)(F014 — only viable together with money normalization, or it breaks integrators passing sums as
strings), and moving
secretKeyout of the query string (F022 — gated on the backend confirmingPOST). All three are recorded on the roadmap for 5.0.
Unchanged, and verified as such
The wire format and the signature payload are untouched —
SignatureBuilderis not modified bythis branch, and that includes the mandatory
unset($params['sign'], $params['signature'], $params[PHP_INT_MAX])guard against signature forgery, still covered by regression tests. Alsounchanged and still covered: constant-time comparison via
hash_equals(), theREMOTE_ADDR-onlyIP source (never the spoofable
X-Forwarded-For), TLS verification on the IP-feed fetch, thefail-safe allowlist refresh (
refreshAllowedIps()throws nothing and never empties the list — sixtests hold that), the
file_get_contentsfallback,form()'s buyer-visiblesdktoken, PHP >=7.4 support and the zero-Composer-dependency policy.
Tests & QA
masterto 290 tests / 615 assertions.tests/Http/ResponseTest.php,RetryingTransportTest.php,CurlTransportTest.php,DefaultTransportTest.php,tests/Webhook/WebhookReplayTest.php,tests/Model/Enum/PaymentObjectTest.php,tests/ComposerRequirementsTest.php.FrozenClockWebhookVerifiermoves the clock through thenow()seam instead ofsleeping;
SleeplessRetryingTransportoverridessleep()so the retry suite never waits.FakeTransportgained fullResponsequeueing while keeping the string shorthand, so thepre-existing tests needed no edits.
a duplicate payment:
testNeverRetriesAfterTheRequestReachedTheServerandtestNeverRetriesAFailureTheTransportCouldNotClassify.testTheVerdictDoesNotDependOnTheServerTimezoneruns the same webhook fixture underUTCandEurope/Moscow; it is what catches astrtotime()regression.ComposerRequirementsTestscanssrc/with the tokenizer for calls into extensions a build candisable and asserts each one is declared, and that
requirecontains nothing butphpandext-*. The zero-dependency stance is now a regression a test catches rather than a convention areviewer has to remember.
tests/Model/Enum/PaymentObjectTest.phppins both halves of the dictionary through reflection —the five removed values are absent, the surviving 25 are exact — so a merge cannot quietly
restore them.
Docs
docs/migration-v4.mdis new: the before/after for a custom transport (the only hard breakfor most integrators), the exception subclasses and when each fires, how to disable retries and
the replay window, the header rename, the removed
PaymentObjectvalues with a ready grep, and achecklist.
docs/getting-started.md(constructor, transports, timeouts, retries),docs/webhooks.md(thetolerance window, the UTC+3 assumption, and the
refreshAllowedIps()worst-case wall clock nowthat it inherits retries),
docs/telemetry.md(rewritten forsetModule()/setStack(), withworked examples for a CMS plugin, a four-layer stack, a bare app, and a section on templates) and
docs/api-methods.md(the exception hierarchy and which call raises which, plus how to log afailure without logging the key) were all updated.
examples/paymentInfo.phpnow catchesUnitpayHttpException,UnitpayResponseExceptionandUnitpayNetworkExceptionseparately;examples/config.phpcarries the identity calls as acommented-out sample, since a bare script should not report a module it does not have.
examples/is under static analysis now, so a stale sample fails the build rather than rotting.CHANGELOG.mdentry covers both the folded-in 3.1.0 batch and this work, and states whythe retry policy is narrower than other SDKs' so the restraint does not read as an oversight.
Verification
composer checkis green:parallel-lint72 files, php-cs-fixer 0 of 72, PHPStan level 8 onsrc/+tests/([OK] No errors), PHPStan level 2 onexamples/([OK] No errors), PHPMDclean, PHPUnit 290 tests / 615 assertions.
Caveat, stated plainly: no CI run exists for this branch yet. The
pushtrigger in.github/workflows/ci.ymlis restricted tomaster, so pushing a feature branch produces zeroworkflow runs — opening this PR is what will first exercise the 7.4–8.5 matrix. Everything above
was measured locally on PHP 8.5.8 against the host
vendor/, which is weaker evidence than CI:CI resolves dependencies from scratch per PHP version. Please let the PR checks run before merging
rather than treating the local result as the gate.
Merge note
The branch is
release/4.0.0andmasteris an ancestor, so this merges fast-forward. Please donot squash: the 25 commits are grouped so that each one builds and passes tests on its own, and the
breaking ones are individually reviewable.
After merge, tag
4.0.0— unprefixed, as since 2.0.0. The release workflow compares the tag withUnitpay::VERSION(src/Unitpay.php:30, currently'4.0.0') and refuses to publish on a mismatch.