Skip to content

Conversation

@vtlxx
Copy link

@vtlxx vtlxx commented Dec 28, 2025

Why?

This update clarifies a non-obvious behavior in WebhookSignature::verifyHeader. Previously, it was not clear from the docblock that timestamp validation is skipped not only when tolerance is null, but also when 0 is passed. Since the actual check only runs when $tolerance > 0, passing 0 effectively disables tolerance validation.

The signature header parsing logic previously relied on implicit assumptions about the header format. By explicitly validating key=value pairs, the code now safely ignores malformed values (e.g. t, v1) that would previously throw exceptions, instead of attempting to process them.

What?

  • Clarify in the WebhookSignature::verifyHeader docblock that timestamp validation is skipped when tolerance is null or 0
  • Add an explicit 2 === \count($itemParts) check in WebhookSignature::getTimestamp to safely ignore malformed header parts (e.g. t, v1) and prevent exceptions
  • Add the same 2 === \count($itemParts) check in WebhookSignature::getSignatures

@vtlxx vtlxx requested a review from a team as a code owner December 28, 2025 21:59
@vtlxx vtlxx requested review from mbroshi-stripe and removed request for a team December 28, 2025 21:59
@cla-assistant
Copy link

cla-assistant bot commented Dec 28, 2025

CLA assistant check
All committers have signed the CLA.

@cla-assistant
Copy link

cla-assistant bot commented Dec 28, 2025

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@mbroshi-stripe mbroshi-stripe requested review from prathmesh-stripe and removed request for mbroshi-stripe December 29, 2025 01:13
foreach ($items as $item) {
$itemParts = \explode('=', $item, 2);
if ('t' === $itemParts[0]) {
if (2 === \count($itemParts) && 't' === $itemParts[0]) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a case when you receive a malformed values from Stripe?
I'm not sure if we would want to ignore a case without throwing an exception when we receive a malformed value. SDK should throw the exception(maybe a better one) and give the user the choice to catch/ignore it.

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.

2 participants