src/Appwrite/Platform/Modules/Storage/Http/Buckets/Files/View/Get.php (1)

147-149: Range validation fix is correct; minor comparison inconsistency with Download endpoint.

Changing $start >= $end to $start > $end correctly admits the bytes=0-0 single-byte range case. Note that this file still uses a loose != for $unit != 'bytes', while Download/Get.php (same PR) was updated to strict !==. The loose comparison works identically here since getRangeUnit() returns a string, but consistency between the two files is preferable.

🔧 Suggested fix

-            if ($unit != 'bytes' || $start > $end || $end >= $size) {
+            if ($unit !== 'bytes' || $start > $end || $end >= $size) {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/Appwrite/Platform/Modules/Storage/Http/Buckets/Files/View/Get.php` around
lines 147 - 149, Update the range validation to use a strict comparison for the
range unit like the Download endpoint: in the same conditional that now checks
($unit != 'bytes' || $start > $end || $end >= $size) replace the loose != with
!== so it reads ($unit !== 'bytes' || $start > $end || $end >= $size); this
keeps behavior identical (getRangeUnit() returns a string) but ensures
consistency with the other updated file and the Exception::STORAGE_INVALID_RANGE
check remains unchanged.

tests/e2e/Services/Storage/StorageBase.php (2)

696-706: Consider asserting Accept-Ranges and Content-Length on the large-file download.

The large-file download is the primary regression scenario called out in the PR description, yet the test does not assert the two headers that the fix introduces for this path. Adding these assertions here would directly anchor the fix to the test.

🧪 Suggested additions

 $this->assertEquals(200, $file7['headers']['status-code']);
 $this->assertEquals('attachment; filename="large-file.mp4"', $file7['headers']['content-disposition']);
 $this->assertEquals('video/mp4', $file7['headers']['content-type']);
+$this->assertEquals('bytes', $file7['headers']['accept-ranges']);
+$this->assertNotEmpty($file7['headers']['content-length']);
 $this->assertNotEmpty($fileData);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/e2e/Services/Storage/StorageBase.php` around lines 696 - 706, The test
for downloading the large file should also assert the new headers introduced by
the fix: after obtaining $file7 and $fileData in StorageBase.php (the GET to
'/storage/buckets/.../files/.../download' that returns large-file.mp4), add
assertions that $file7['headers']['accept-ranges'] equals 'bytes' (or contains
'bytes') and that $file7['headers']['content-length'] equals the filesize of the
source file (use filesize(realpath(__DIR__ .
'/../../../resources/disk-a/large-file.mp4')) or cast to string/int to match
header type) so the test verifies both Accept-Ranges and Content-Length for the
large-file download path.

---

622-630: Extend single-byte probe assertions to cover body and required headers.

The test validates the status and Content-Length, but per RFC 7233 §4.1 a 206 Partial Content response MUST include a Content-Range header, and the PR's stated goal includes advertising Accept-Ranges. Covering those here would complete the contract and guard against future regressions.

🧪 Suggested additions

 $this->assertEquals(206, $file51a['headers']['status-code']);
 $this->assertEquals('1', $file51a['headers']['content-length']);
+$this->assertEquals('bytes 0-0/47218', $file51a['headers']['content-range']);
+$this->assertEquals('bytes', $file51a['headers']['accept-ranges']);
+$path = __DIR__ . '/../../../resources/logo.png';
+$this->assertEquals(substr(file_get_contents($path), 0, 1), $file51a['body']);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/e2e/Services/Storage/StorageBase.php` around lines 622 - 630, The
single-byte probe test ($file51a) only checks status and content-length; add
assertions to verify the response includes the required headers and body: assert
the presence and value of the Content-Range header (e.g. startsWith or equals
"bytes 0-0/{size}" using $data['size'] if available), assert that Accept-Ranges
is "bytes", and assert the response body is exactly one byte
(strlen($file51a['body']) === 1) and matches the first byte of the source
content (e.g. substr($data['content'], 0, 1) or equivalent); use the existing
$file51a variable and the client->call result to locate where to add these
checks.

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro