Pin three guards reviewers found untested (download timer, URL fragment, short APP1 segment) #89

Closed
opened 2026-09-23 03:14:57 +02:00 by clawbot · 1 comment
Collaborator

Three gaps that reviewers reported as non-blocking while passing merged units. Each is a missing guard on code that is already correct.

  1. From #88: nothing fails if deadline.stop() is removed from release in deadlineStream or from the error path in getFileStream (src/api/client.ts, around lines 100 and 410). Without it, a finished or failed download leaves a 60 s timer holding the process open. Call unref() on that timer so it can never keep the process alive, and add a test that no timer is pending after a download completes or fails.
  2. From #87: the rejection of # in buildURL (src/api/client.ts, around line 111) has no test. Add one next to the ? rejection test.
  3. From #84: an APP1 segment whose length is under 8 still has its Exif header compared against the bytes of the next segment (src/metadata-backup.ts, around line 49). Check the length is at least 8 before comparing, and add a test with a short APP1 segment.

Definition of done

  • Each item is fixed, with a test that fails when the fix is reverted.
  • make check green; TODO.md updated in the same commit.

Model: opus-5-5

Three gaps that reviewers reported as non-blocking while passing merged units. Each is a missing guard on code that is already correct. 1. From https://git.eeqj.de/sneak/quak/pulls/88: nothing fails if `deadline.stop()` is removed from `release` in `deadlineStream` or from the error path in `getFileStream` (`src/api/client.ts`, around lines 100 and 410). Without it, a finished or failed download leaves a 60 s timer holding the process open. Call `unref()` on that timer so it can never keep the process alive, and add a test that no timer is pending after a download completes or fails. 2. From https://git.eeqj.de/sneak/quak/pulls/87: the rejection of `#` in `buildURL` (`src/api/client.ts`, around line 111) has no test. Add one next to the `?` rejection test. 3. From https://git.eeqj.de/sneak/quak/pulls/84: an APP1 segment whose length is under 8 still has its `Exif` header compared against the bytes of the next segment (`src/metadata-backup.ts`, around line 49). Check the length is at least 8 before comparing, and add a test with a short APP1 segment. ## Definition of done - Each item is fixed, with a test that fails when the fix is reverted. - `make check` green; `TODO.md` updated in the same commit. Model: opus-5-5
clawbot self-assigned this 2026-09-23 03:14:57 +02:00
Author
Collaborator

Implemented in #91: the download timer is unref'd, the short APP1 segment is length-checked, and each of the three guards has a test that fails without it.

Model: opus-5-5

Implemented in https://git.eeqj.de/sneak/quak/pulls/91: the download timer is unref'd, the short APP1 segment is length-checked, and each of the three guards has a test that fails without it. Model: opus-5-5
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/quak#89