Skip to content

fix: use strict null checks and explicit property initialization#617

Merged
rowan-m merged 1 commit intogoogle:mainfrom
SNO7E-G:fix/strict-property-safety
Mar 29, 2026
Merged

fix: use strict null checks and explicit property initialization#617
rowan-m merged 1 commit intogoogle:mainfrom
SNO7E-G:fix/strict-property-safety

Conversation

@SNO7E-G
Copy link
Copy Markdown
Contributor

@SNO7E-G SNO7E-G commented Mar 27, 2026

Hi!

This is a small, strict-typing and safety improvement for ReCaptcha.php.

While auditing the class, I noticed that the 5 optional configuration properties ($hostname, $apkPackageName, $action, $threshold, $timeoutSeconds) are strictly typed but never explicitly initialized. Right now, the class relies on isset() in the verify() method to act as a guard rail.

While that technically works, it leaves a potential footgun: if anyone ever adds a new internal method that tries to read one of those properties without wrapping it in isset(), PHP 8 will crash with a fatal \Error ("Typed property must not be accessed before initialization").

This PR makes things explicitly safe and native to PHP 8:

  • Explicit Initialization: Initializes all 5 optional properties with ?type = null so they always have a predictable, safe state.
  • Strict Comparisons: Replaces the legacy isset() checks with null !== $this->property comparisons. The behavior is the same, but it doesn't rely on error-suppression masking.
  • Fixed the "0" Edge Case: I noticed we were using empty($secret) and empty($response). Because empty("0") returns true in PHP, the library would actually reject the string "0" as invalid input! Since the parameters are already strictly typed as string in the method signature, I replaced empty() with a strict '' === check. The string "0" is now correctly accepted as a valid string value. (I added two specific unit tests to prove and lock in this fix!)
  • Consistency: Replaced !is_null($requestMethod) with null !== $requestMethod just to keep the coding style consistent throughout the file.

What Changed

File Lines Summary
src/ReCaptcha/ReCaptcha.php +13 / -13 Null-safe initialization, strict comparisons
tests/ReCaptcha/ReCaptchaTest.php +14 / -0 Added 2 new tests verifying that "0" is treated as a valid string

All tests pass (up from 66 to 68 tests, 183 assertions) and PHPStan Level Max is completely green.

@coveralls
Copy link
Copy Markdown

Coverage Status

coverage: 100.0%. remained the same
when pulling 26bc7f4 on SNO7E-G:fix/strict-property-safety
into ff0beab on google:main.

@rowan-m rowan-m self-requested a review March 27, 2026 09:50
@rowan-m rowan-m self-assigned this Mar 27, 2026
@rowan-m rowan-m merged commit 5183304 into google:main Mar 29, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants