Modernize tracker for 4.0.0: PHP 8.1, strict types, static analysis, full test coverage - #152
Merged
Merged
Conversation
- Drop PHP 7.2-8.0 support; require ^8.1. - Bump PHPUnit to ^10.5 (PHP 8.1 floor rules out PHPUnit 11). - Add PHPStan ^2, PHP_CodeSniffer ^3.10, the Matomo coding standard (matomo-org/matomo-coding-standards via VCS repo) and the phpcs composer-installer plugin. - Pin config.platform.php to 8.1.0 and allow the installer plugin. - Add composer scripts: test, phpstan, phpcs, phpcbf.
- phpunit.yml: drop PHP 7.2-8.0 from the matrix, bump to actions/checkout@v4, and install the json + curl extensions. - Add quality.yml running PHPStan and PHP_CodeSniffer on PHP 8.1 for every pull request and push to master.
Add declare(strict_types=1) to both source files and give every method proper parameter and return type hints aligned with how Matomo core coerces each tracking parameter. Declare all class properties explicitly with concrete/nullable types and array-shape PHPDocs (the class keeps #[AllowDynamicProperties] for consumer BC). Optional 'unset' parameters and their backing properties/getters move from the legacy `= false` sentinel to nullable (`?T = null`); fluent setters now declare `: self`, and the do* tracking methods declare `: string|bool`. Fixes latent type issues that become fatal TypeErrors under strict types (previously masked by weak-mode coercion), including: - sendRequest() now returns string|bool (bulk mode really returns true, not the coerced "1"); cascaded to all do* methods. - forceDotAsSeparatorForDecimalPoint() no longer returns null/false from a : string method. - setCustomTrackingParameter() casts the dimension id to int. - doTrackSiteSearch()/getUrlTrackSiteSearch() use ?int and only emit &search_count when a count is provided (no longer always sends 0). - getUrlTrackCrash()/getUrlTrackEcommerceOrder() cast int/id values before urlencode(); getRequest() casts lat/long before urlencode(). - getUrlTrackEvent() guards the nullable name/value correctly. - clientHints is reset to [] (not false) in bulk mode. - getCustomVariablesFromCookie() validates decoded cookie data. BREAKING CHANGE: passing mismatched scalar types now throws TypeError instead of being silently coerced, and unset sentinels are null rather than false. This targets a new major release.
Analyse both source files at level max with the PHP 8.1 platform. No baseline: the codebase is fully clean at max level.
Add phpcs.xml.dist referencing the Matomo coding standard with a 400-char line limit and scoped exceptions: the two source files intentionally mix a class with global helper functions/includes and stay in the global namespace for backwards compatibility. Apply the resulting fixes (blank line after the open tag, control structure / operator spacing, trailing newline) and clean up the test file: reorder the file header, add constant visibility, and rename the test methods to camelCase.
- phpunit.xml.dist: run --migrate-configuration (drop the removed verbose attribute, refresh the schema to 10.5, add cacheDirectory); keep backupGlobals. - Make the data-provider methods static as required by PHPUnit 10. - Ignore the new .phpunit.cache directory.
- README: state the PHP 8.1+ requirement and add a Development section covering the composer test/phpstan/phpcs scripts. - CHANGELOG: add the 4.0.0 entry describing the breaking changes (PHP floor, strict types, false->null sentinels, do* return type, search_count / stream-fallback behaviour) and the new tooling. - .gitattributes: export-ignore phpstan.neon.dist and phpcs.xml.dist so dist archives stay lean.
All properties are now declared explicitly, so the tracker no longer creates dynamic properties and the attribute is unnecessary. Dropping it makes undeclared dynamic property writes raise the standard PHP 8.2 deprecation, in line with the strict, fully-typed rewrite.
The method documentation always promised that the User ID can be unset again, but the string type hint made that impossible. Accept null to reset it, matching the null sentinel used everywhere else.
- Cast cookie names to string before strpos(): cookies with purely numeric names are exposed as integer keys in $_COOKIE and caused a TypeError under strict_types, breaking tracker construction. - Do not read $http_response_header when the stream fallback request failed without an HTTP response; previously this caused a TypeError in parseIncomingCookies() instead of returning false. - Send latitude/longitude values of 0.0 instead of silently dropping coordinates on the equator or prime meridian. - Guard the POST URL split against URLs without a query string and replace an always-true elseif condition with else. - Remove dead branches in setEcommerceView() that guarded against values the string type hints now make impossible. - Restore the dropped $category docblock in doTrackSiteSearch(), fix the setClientHints() fullVersionList docblock shape, remove leftover array() syntax and strval() calls, and document all behavior changes in the 4.0.0 changelog.
- Add tests for every tracking URL parameter, all do*/getUrlTrack* methods, custom variables/dimensions, ecommerce, client hints, attribution info, visitor/user id handling, bulk tracking, cookie handling and request preparation, including regression tests for the behavior changes of the 4.0.0 release (search_count omission, bulk true return, null sentinels, numeric cookie names). - Introduce a TestableMatomoTracker capturing requests and cookies instead of performing network or header I/O. - Analyse the test suite with PHPStan at max level. - Add a phpunit <source> section and a CI coverage job (pcov).
![aikido-pr-checks[bot]](https://avatars.githubusercontent.com/in/898896?s=60&v=4){kind=small}
…param helper Findings from auditing tracker usage across Matomo core and plugins: - setUrlReferrer()/setUrlReferer() accept ?string again so callers can unset the referrer with null (used by several Matomo fixtures). - setCustomTrackingParameter() accepts string|array again; array values are serialized via http_build_query exactly as before 4.0.0, restoring multi-value parameters used by FormAnalytics and HeatmapSessionRecording. - Add setDebugTrackingParameter(string, string) (@internal): appends a raw, unvalidated parameter that overrides any built-in of the same name, so Matomo integration tests can verify server-side handling of malformed values without bypassing the tracker. Cleared per request. Adds unit tests and CHANGELOG entries for all three.
- Pin shivammathur/setup-php to a commit hash (2.37.2) instead of the floating @v2 tag, mitigating supply-chain risk (Aikido high finding). - Set persist-credentials: false on actions/checkout steps so the GITHUB_TOKEN is not left in the git config for later steps (Aikido).
Drop the pcov-based coverage job for now; the standard PHPUnit matrix, PHPStan and PHPCS jobs remain.
Core treats an absent 'revenue' as 'use the goal's configured revenue', and defaults the optional ecommerce amounts to false (not 0), so a real zero is meaningful and must be distinguishable from 'not provided'. - doTrackGoal()/getUrlTrackGoal() (+ Matomo_/Piwik_ helpers) take ?float $revenue = null: null omits revenue, 0.0 now sends revenue=0. - doTrackEcommerceOrder()/getUrlTrackEcommerceOrder()/getUrlTrackEcommerce() take ?float $subTotal/$tax/$shipping/$discount = null, sent only when provided; the required grand total is now always sent (0.0 -> revenue=0). Previously an explicit 0/0.0 was silently dropped by the !empty() guards. Adds tests and a CHANGELOG entry.
sgiehl
force-pushed
the
modernize-php-8.1-strict-types-static-analysis
branch
from
July 25, 2026 17:02
4607868 to
faec8a4
Compare
- URL-encode the cip (override IP) tracking parameter before adding it to the request, matching every other value and preventing tracking-parameter injection when an application sets cip from untrusted input. Fixes #151. - Remove the curl_close() call guarded by PHP_VERSION_ID < 80000: it is dead code now that PHP 8.1 is the minimum, and curl_close() is a no-op (and deprecated in PHP 8.5) on all supported versions. Fixes #149.
Append &ca=1 (custom action) to the event and content-tracking URLs, matching the JS tracker. This prevents Matomo from falling back to recording these requests as page views when the handling plugin is disabled. Fixes #80.
Change the default requestTimeout from 600s to 5s and connect timeout from 300s to 2s, so a slow or unreachable Matomo can no longer block the calling page for up to 10 minutes. Callers can still raise them via setRequestTimeout()/setRequestConnectTimeout(). Fixes #88.
Allow callers to pass additional cURL options (e.g. CURLOPT_IPRESOLVE, CURLOPT_HTTP_VERSION) that are applied after the built-in options, so they can tune the request without the library committing to opinionated defaults. Fixes #92.
Previously the cURL transport threw a RuntimeException on DNS/connection failures while the stream transport silently returned false. Both now throw by default (preserving cURL behavior and making the two transports consistent). Call setExceptionsEnabled(false) to make failed requests return false instead, so tracking never breaks the calling application. Refs #105.
sgiehl
force-pushed
the
modernize-php-8.1-strict-types-static-analysis
branch
from
July 25, 2026 19:11
2c54e59 to
5e85858
Compare
…exceptions - URL-encode the remaining raw query values: _refts (from attribution data), customData (data) and pageCharset (cs), so a value can no longer inject extra tracking parameters. - Validate the visitor ID read from the first-party cookie as a 16-character hexadecimal string in loadVisitorIdCookie(), matching setVisitorId(). - Request-failure exceptions now include only the target host, never the full URL (which can carry token_auth/PII in its query string).
- Build the outgoing Cookie header as url-encoded pairs joined with '; ' (was http_build_query, which joined with '&'), and parse every incoming Set-Cookie header by its first '=' so multiple cookies accumulate (parse_str previously overwrote the whole set per header and applied query-string bracket semantics). getIncomingTrackerCookie() now returns string|false. - setAttributionInfo() no longer echoes the supplied JSON in its exception message and marks the parameter #[\SensitiveParameter].
…ports, bulk) - Mark sendRequest() $url and $data as #[\SensitiveParameter] so they are redacted from exception stack traces (token_auth/PII). - Force POST when token_auth is placed in the request body, so the stream transport no longer sends it as a GET body that Matomo ignores. - Stream transport sets ignore_errors so HTTP 4xx/5xx return the response body (parity with cURL) instead of a failure. - Bulk tracking uses a >=30s timeout and retains the queued actions when a batch fails to send (previously discarded), allowing retry. - Docs: clarify setUserId(null) does not unlink the current server-side visit; drop the stale 'coverage reported in CI' CHANGELOG claim. Adds tests for all of the above.
- getUrlTrackAction()/doTrackAction() now URL-encode the action type (not just the value), so a crafted or plugin-specific action parameter cannot inject an additional query-string parameter. - Extract the visitor-id hex character set into a single HEX_CHARACTERS constant used by both setVisitorId() and loadVisitorIdCookie(). - Make cURL detection overridable via hasCurlSupport() so the stream transport can be exercised in tests; add coverage for the stream failure (host-only exception message) and fail-safe paths.
- README: add an 'Error handling and timeouts' section covering the default exceptions, the short 5s/2s timeouts, setExceptionsEnabled(), setRequestTimeout()/setRequestConnectTimeout() and bulk retention. - CHANGELOG: correct the multi-value BC note to reference pre-3.4.0 and mention that the download/link action type is now URL-encoded. - phpcs.xml.dist: explain that the 400-char line limit matches Matomo core's own coding standard rather than being an arbitrary value.
sgiehl
marked this pull request as ready for review
July 27, 2026 11:40
sendRequest() appends token_auth into the URL/body and passes them to prepareCurlOptions()/prepareStreamOptions(). #[\SensitiveParameter] only redacts the frame where the parameter is declared, so mark those params too: should either builder (or something it calls) ever throw, the token and PII carried in $url/$data are redacted from that frame as well.
The blanket 'mismatched scalar now throws a TypeError' claim was misleading: strict/weak mode is set by the *caller's* file, so a normal (non-strict) consumer still gets weak-mode coercion. Legacy calls that passed false to mean 'not known' (doTrackEvent value, addEcommerceItem price, setLatitude/setLongitude, ...) now silently send 0/0.0/'' instead of omitting the parameter. Add an explicit upgrade note and correct the TypeError wording so the transition is spelled out.
…ders setCurlOptions() applied caller options via array_replace(), so a caller adding a single header wholesale-replaced the tracker's own header list. For POST/bulk requests that dropped the Content-Type, making Matomo unable to parse the body — a silent failure. Merge (append) custom HTTP headers onto the built-in ones instead, and document the special-casing. Also extend the sensitive-parameter test to assert redaction on the transport option builders (prepareCurlOptions/prepareStreamOptions), not just sendRequest, since the URL/body are forwarded to them.
mneudert
approved these changes
Jul 27, 2026
This was referenced Jul 27, 2026
Closed
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
2 participants
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.
Summary
Modernizes the Matomo PHP Tracker for a 4.0.0 major release:
declare(strict_types=1)core/Tracker/Request.php)falsesentinels tonull, removes#[AllowDynamicProperties], and fixes several latent type bugs0is now distinct from "unset"),setUrlReferrer(?string)andsetCustomTrackingParameter(string|array), plus an@internal setDebugTrackingParameter()test helperLatent type bugs fixed along the way:
$_COOKIE) caused aTypeErrorunder strict types, breaking tracker construction entirelyTypeErrorinparseIncomingCookies()(on PHP < 8.4) instead of returningfalse0.0were silently droppedsetUserId(null)now de-assigns the User ID, as the docs always promisedTests
The unit test suite grew from 17 to 136 tests (418 assertions), covering everything testable without network I/O:
do*/getUrlTrack*method incl. all exception pathsTestableMatomoTrackerdouble captures requests/cookies instead of doing I/ORelated issues
cip(override IP) parameter is now URL-encoded like every other tracking value.curl_close()(dead code now that PHP 8.1 is the minimum).catracking parameter for tracking requests that aren't page views #80 — event and content requests now send&ca=1, so Matomo doesn't record them as page views when the handling plugin is disabled.setCurlOptions()to pass additional cURL options for the request.do*methods now declare an explicit, documentedstring|boolreturn type.setExceptionsEnabled(false)opts into graceful (return-false) handling so tracking never breaks the page.apiUrlproperty.Breaking changes
See the 4.0.0 section in
CHANGELOG.mdfor the complete list.Checks
Local: PHPUnit (136 tests) ✅ · PHPStan max ✅ · PHPCS (Matomo standard) ✅ — on PHP 8.3.
CI validates PHP 8.1–8.5 across Linux/Windows/macOS plus PHPStan and PHPCS.