diff --git a/src/Ushahidi/Modules/V5/Http/Resources/Media/MediaResource.php b/src/Ushahidi/Modules/V5/Http/Resources/Media/MediaResource.php index a70cb564ad..9d93f7a36c 100644 --- a/src/Ushahidi/Modules/V5/Http/Resources/Media/MediaResource.php +++ b/src/Ushahidi/Modules/V5/Http/Resources/Media/MediaResource.php @@ -64,56 +64,75 @@ protected function formatOFilename($value) if (empty($value)) { return null; } - $url = $this->urlOFilename($value); - if ($url === null) { - return null; - } - // Substitute % for %25 to avoid double-encoding issues in browsers - $url = str_replace('%', '%25', $url); - return $url; + return $this->resolveMediaUrl($value); } - protected function urlOFilename($value) + /** + * Resolve the public URL for a media file, trying multiple name-encoding + * strategies to account for how the file may have been stored historically. + * + * Given a DB value like "uploads/some file.jpg", we try: + * + * 1st) Raw name: "uploads/some file.jpg" + * → Object in storage is literally named "some file.jpg" + * → S3 URL: https://s3.../uploads/some%20file.jpg (S3 encodes it) + * → Return as-is. The URL is already correct. + * + * 2nd) Single-encoded: "uploads/some%20file.jpg" + * → Object in storage is literally named "some%20file.jpg" + * (old code used rawurlencode before saving to storage) + * → CDN URL: https://cdn.../uploads/some%20file.jpg + * → Problem: browser decodes %20 → requests "some file.jpg" → 404 + * → Fix: escape % → %25 so URL becomes .../some%2520file.jpg + * browser decodes %25 → "%" and requests "some%20file.jpg" ✓ + * + * 3rd) Double-encoded: "uploads/some%2520file.jpg" + * → Object in storage is literally named "some%2520file.jpg" + * → Same browser-decoding issue, same %25 fix applied. + * + * @param string $value The o_filename value from the database + * @return string|null The public URL, or null if not found + */ + protected function resolveMediaUrl($value) { - // Removes path from image file name, encodes the filename, and joins the path and filename together $url_path = explode("/", $value); $filename = array_pop($url_path); - array_push($url_path, $filename); - $path = implode("/", $url_path); + // 1st try: raw filename as stored in the DB + // e.g. "some file.jpg" → look for object "some file.jpg" + $path = implode("/", array_merge($url_path, [$filename])); $result = Storage::url($path); - // If the result is a non-empty string if (is_string($result) && !empty($result)) { - // Success + // URL from storage is already properly encoded (e.g. S3 returns %20 for spaces) return $result; } - // For some time, we would store files with - // URL-encoded names. Try that as a fallback - $filename = rawurlencode((array_pop($url_path))); - array_push($url_path, $filename); - $path = implode("/", $url_path); - + // 2nd try: single rawurlencode — for files stored with encoded names + // e.g. "some file.jpg" → rawurlencode → "some%20file.jpg" + // look for object literally named "some%20file.jpg" + $encodedOnce = rawurlencode($filename); + $path = implode("/", array_merge($url_path, [$encodedOnce])); $result = Storage::url($path); - // If the result is a non-empty string if (is_string($result) && !empty($result)) { - // Success - return $result; + // The object name has literal "%" chars (e.g. "some%20file.jpg"). + // The CDN URL contains those raw %, which browsers would decode. + // Escape % → %25 so browsers preserve the literal percent sign. + // e.g. ".../some%20file.jpg" → ".../some%2520file.jpg" + // browser decodes %25→% and correctly requests "some%20file.jpg" + return str_replace('%', '%25', $result); } - // Try one more round of urlencoding, just in case - $filename = rawurlencode((array_pop($url_path))); - array_push($url_path, $filename); - $path = implode("/", $url_path); - + // 3rd try: double rawurlencode — for doubly-encoded legacy names + // e.g. "some%20file.jpg" → rawurlencode → "some%2520file.jpg" + // look for object literally named "some%2520file.jpg" + $encodedTwice = rawurlencode($encodedOnce); + $path = implode("/", array_merge($url_path, [$encodedTwice])); $result = Storage::url($path); - // If the result is a non-empty string if (is_string($result) && !empty($result)) { - // Success - return $result; + // Same %-escaping logic as the 2nd try + return str_replace('%', '%25', $result); } - // If we reach here, we failed to find the file return null; } } diff --git a/tests/Unit/Modules/V5/Http/Resources/Media/MediaResourceTest.php b/tests/Unit/Modules/V5/Http/Resources/Media/MediaResourceTest.php new file mode 100644 index 0000000000..e26c45a659 --- /dev/null +++ b/tests/Unit/Modules/V5/Http/Resources/Media/MediaResourceTest.php @@ -0,0 +1,198 @@ +setAccessible(true); + return $method->invoke($resource, $value); + } + + /** + * Helper: call the protected formatOFilename method via reflection. + */ + private function callFormatOFilename($value): ?string + { + $resource = new MediaResource((object) []); + $method = new \ReflectionMethod($resource, 'formatOFilename'); + $method->setAccessible(true); + return $method->invoke($resource, $value); + } + + /** + * Mock Storage::url() so it returns a URL only for the given $knownPaths. + * + * @param array $knownPaths map of storage path => returned URL + */ + private function mockStorageUrl(array $knownPaths): void + { + Storage::shouldReceive('url') + ->andReturnUsing(function (string $path) use ($knownPaths) { + return $knownPaths[$path] ?? null; + }); + } + + // --------------------------------------------------------------- + // formatOFilename: empty / null handling + // --------------------------------------------------------------- + + public function testFormatOFilenameReturnsNullForEmptyString(): void + { + $this->assertNull($this->callFormatOFilename('')); + } + + public function testFormatOFilenameReturnsNullForNull(): void + { + $this->assertNull($this->callFormatOFilename(null)); + } + + // --------------------------------------------------------------- + // 1st try: raw filename found — URL returned as-is + // --------------------------------------------------------------- + + public function testRawFilenameFoundReturnsUrlAsIs(): void + { + // File "uploads/photo.jpg" exists with its raw name in storage. + // Storage returns a properly-encoded URL — no % escaping needed. + $this->mockStorageUrl([ + 'uploads/photo.jpg' => 'https://s3.example.com/uploads/photo.jpg', + ]); + + $result = $this->callResolveMediaUrl('uploads/photo.jpg'); + + $this->assertSame('https://s3.example.com/uploads/photo.jpg', $result); + } + + public function testRawFilenameWithSpacesFoundReturnsUrlAsIs(): void + { + // File "uploads/some file.jpg" exists with spaces in its name. + // S3 returns the URL with %20 already encoded — no extra escaping. + $this->mockStorageUrl([ + 'uploads/some file.jpg' => 'https://s3.example.com/uploads/some%20file.jpg', + ]); + + $result = $this->callResolveMediaUrl('uploads/some file.jpg'); + + // The %20 from S3 must NOT be escaped to %2520 + $this->assertSame('https://s3.example.com/uploads/some%20file.jpg', $result); + } + + // --------------------------------------------------------------- + // 2nd try: single-encoded name found — % escaped to %25 + // --------------------------------------------------------------- + + public function testSingleEncodedFallbackEscapesPercent(): void + { + // DB has "uploads/some file.jpg" but in storage the object is literally + // named "some%20file.jpg" (old code used rawurlencode before saving). + // 1st try ("uploads/some file.jpg") → not found (null). + // 2nd try ("uploads/some%20file.jpg") → found. + $this->mockStorageUrl([ + 'uploads/some%20file.jpg' => 'https://cdn.example.com/uploads/some%20file.jpg', + ]); + + $result = $this->callResolveMediaUrl('uploads/some file.jpg'); + + // The % must be escaped to %25 so browsers don't decode %20 as a space. + // "some%20file.jpg" → "some%2520file.jpg" + $this->assertSame('https://cdn.example.com/uploads/some%2520file.jpg', $result); + } + + // --------------------------------------------------------------- + // 3rd try: double-encoded name found — % escaped to %25 + // --------------------------------------------------------------- + + public function testDoubleEncodedFallbackEscapesPercent(): void + { + // DB has "uploads/some file.jpg" but in storage the object is literally + // named "some%2520file.jpg" (doubly encoded). + // 1st try ("uploads/some file.jpg") → not found. + // 2nd try ("uploads/some%20file.jpg") → not found. + // 3rd try ("uploads/some%2520file.jpg") → found. + $this->mockStorageUrl([ + 'uploads/some%2520file.jpg' => 'https://cdn.example.com/uploads/some%2520file.jpg', + ]); + + $result = $this->callResolveMediaUrl('uploads/some file.jpg'); + + // % escaped to %25 in the returned URL + $this->assertSame('https://cdn.example.com/uploads/some%252520file.jpg', $result); + } + + // --------------------------------------------------------------- + // Not found in any strategy — returns null + // --------------------------------------------------------------- + + public function testReturnsNullWhenFileNotFoundInAnyStrategy(): void + { + // Storage returns null for every path tried + $this->mockStorageUrl([]); + + $result = $this->callResolveMediaUrl('uploads/missing.jpg'); + + $this->assertNull($result); + } + + // --------------------------------------------------------------- + // Simple filenames (no spaces or special chars) + // --------------------------------------------------------------- + + public function testSimpleFilenameNoSpecialChars(): void + { + // A plain filename with no encoding concerns. + // rawurlencode("photo.jpg") === "photo.jpg", so 1st and 2nd try + // resolve to the same path. The 1st try should match. + $this->mockStorageUrl([ + 'uploads/photo.jpg' => 'https://s3.example.com/uploads/photo.jpg', + ]); + + $result = $this->callResolveMediaUrl('uploads/photo.jpg'); + + $this->assertSame('https://s3.example.com/uploads/photo.jpg', $result); + } + + // --------------------------------------------------------------- + // Deeply nested paths + // --------------------------------------------------------------- + + public function testNestedDirectoryPathPreserved(): void + { + $this->mockStorageUrl([ + 'a/b/c/photo.jpg' => 'https://s3.example.com/a/b/c/photo.jpg', + ]); + + $result = $this->callResolveMediaUrl('a/b/c/photo.jpg'); + + $this->assertSame('https://s3.example.com/a/b/c/photo.jpg', $result); + } + + // --------------------------------------------------------------- + // Priority: 1st try wins even if 2nd would also match + // --------------------------------------------------------------- + + public function testFirstTryTakesPriorityOverSecond(): void + { + // Both raw and encoded paths exist. 1st try should win, + // and the URL should NOT have % escaping applied. + $this->mockStorageUrl([ + 'uploads/some file.jpg' => 'https://s3.example.com/uploads/some%20file.jpg', + 'uploads/some%20file.jpg' => 'https://cdn.example.com/uploads/some%20file.jpg', + ]); + + $result = $this->callResolveMediaUrl('uploads/some file.jpg'); + + // 1st try URL returned as-is (no %25 escaping) + $this->assertSame('https://s3.example.com/uploads/some%20file.jpg', $result); + } +}