Skip to content

Fix temporary URL on docker setup#3373

Merged
ildyria merged 6 commits intomasterfrom
test-fix-urls
May 27, 2025
Merged

Fix temporary URL on docker setup#3373
ildyria merged 6 commits intomasterfrom
test-fix-urls

Conversation

@ildyria
Copy link
Copy Markdown
Member

@ildyria ildyria commented May 26, 2025

Fixes #3367

Fixes stupid behaviour of Laravel again... Matthias would be proud. 😃

In theory we should use the $request->hasCorrectSignature() method here. However, for some stupid unknown reason, the path value is added to the server Query String. This completely invalidates the signature check.
For example the url http://localhost:8000/image/small2x/c3/3d/c661c594a5a781cd44db06828783.png?expires=1748380289
will be verified as : http://localhost:8000/image/small2x/c3/3d/c661c594a5a781cd44db06828783.png?/image/small2x/c3/3d/c661c594a5a781cd44db06828783.png&expires=1748380289
which makes the signature check fail as the hmac does not match.

Tested to work locally on my docker compose install.

Enhancements to secure image links:

  • Added SignatureExpiredException: Introduced a new exception class SignatureExpiredException to handle cases where a secure link's signature has expired.
  • Refactored signature validation logic: Updated the SecurePathController to differentiate between invalid and expired signatures, logging detailed errors for invalid cases and throwing appropriate exceptions. Added private methods hasCorrectSignature and signatureHasNotExpired to encapsulate these checks.
  • Improved URL generation: Enhanced the pathToUrl method in HasUrlGenerator to handle cases where signed URLs are not used, ensuring compatibility with secure image link configurations.

Expanded test coverage:

  • New test cases for secure links: Added tests for expired signatures (testExpiredSignature) and broken signatures (testBrokenSignature) to validate the new exception and logic. Refactored existing tests to use helper methods for enabling secure and temporary links. [1] [2] [3]
  • Refactored test setup: Introduced helper methods setSecureLink and setTemporaryLink in SecureImageLinksTest to streamline test configuration and improve clarity.

@ildyria ildyria requested a review from a team as a code owner May 26, 2025 22:07
@ildyria ildyria added High Priority High priority issues Review: easy Easy review expected: probably just need a quick to go through. labels May 26, 2025
@codecov
Copy link
Copy Markdown

codecov Bot commented May 26, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.02%. Comparing base (48e6d35) to head (66f7b3d).
Report is 4 commits behind head on master.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ildyria ildyria merged commit de63f58 into master May 27, 2025
35 checks passed
@ildyria ildyria deleted the test-fix-urls branch May 27, 2025 06:17
ildyria added a commit that referenced this pull request May 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

High Priority High priority issues Review: easy Easy review expected: probably just need a quick to go through.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

403 InvalidSignatureException on temporary image links with Docker setup

2 participants