Add header sanitization and update dependencies - #13
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe SDK sanitizes Sendy response headers, tightens internal types, strengthens token refresh assertions, updates version handling, upgrades PHPStan, configures weekly dependency updates, and adds PHP 8.5 to the test matrix. ChangesSDK hardening and maintenance
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Http/Response.php`:
- Around line 103-107: Update normalizeHeaders() to validate each header name
against the HTTP token grammar before lowercasing or returning it, rejecting or
excluding malformed names containing carriage returns, line feeds, or other
non-token characters. Preserve normalization of valid headers and ensure invalid
names cannot reach Connection.php’s x-sendy- filter or public sendyHeaders.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2d2498eb-3b9a-4740-84dc-668cc977c75d
📒 Files selected for processing (2)
src/Connection.phpsrc/Http/Response.php
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/ConnectionTest.php (1)
260-263: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAvoid the five-second expiry window in this test.
The setup gives the existing token only five seconds of validity. A slow CI run can cross that deadline before
checkOrAcquireAccessToken()handles the revoked refresh token, causing a falseClientExceptionfailure. Use a generous TTL such astime() + 3600.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/ConnectionTest.php` around lines 260 - 263, Update the token setup in the test around checkOrAcquireAccessToken so the existing access token expires at least an hour in the future, using a TTL such as time() + 3600 instead of the current five-second window. Keep the revoked refresh-token behavior and assertions for getAccessToken() and getRefreshToken() unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@bump_version.sh`:
- Line 20: Update the version validation regex in bump_version.sh to require
each numeric component to match 0 or a nonzero digit followed by digits, while
preserving multi-digit versions and the existing prerelease format. Apply the
same constraint to the version pattern in tests/ConnectionTest.php, and add
regression coverage rejecting versions such as 1.2.01.
---
Outside diff comments:
In `@tests/ConnectionTest.php`:
- Around line 260-263: Update the token setup in the test around
checkOrAcquireAccessToken so the existing access token expires at least an hour
in the future, using a TTL such as time() + 3600 instead of the current
five-second window. Keep the revoked refresh-token behavior and assertions for
getAccessToken() and getRefreshToken() unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 244cdddb-f06d-48ff-affd-3bdc911797d9
📒 Files selected for processing (2)
bump_version.shtests/ConnectionTest.php
Summary by CodeRabbit
x-sendy-*headers are exposed, with normalized values.