Three guards reviewers reported as untested on #88, #87 and #84.
Download timer (src/api/client.ts): the idle deadline's timer is now unref()'d, so a timer left running can never keep the process alive. One test checks no timer is pending after a download completes and after one fails (it fails if either deadline.stop() call is removed); another checks the timer does not hold the process.
# in a request path: a test next to the ? rejection test.
Short APP1 segment (src/metadata-backup.ts): the Exif header is compared only when the segment length is at least 8, so the comparison never reaches past the segment. The test uses a length-7 segment holding Exif\0, which the old code returned as EXIF.
Each test was run against the code with its fix removed and failed.
Model: opus-5-5
Closes https://git.eeqj.de/sneak/quak/issues/89.
Three guards reviewers reported as untested on https://git.eeqj.de/sneak/quak/pulls/88, https://git.eeqj.de/sneak/quak/pulls/87 and https://git.eeqj.de/sneak/quak/pulls/84.
- **Download timer** (`src/api/client.ts`): the idle deadline's timer is now `unref()`'d, so a timer left running can never keep the process alive. One test checks no timer is pending after a download completes and after one fails (it fails if either `deadline.stop()` call is removed); another checks the timer does not hold the process.
- **`#` in a request path**: a test next to the `?` rejection test.
- **Short APP1 segment** (`src/metadata-backup.ts`): the `Exif` header is compared only when the segment length is at least 8, so the comparison never reaches past the segment. The test uses a length-7 segment holding `Exif\0`, which the old code returned as EXIF.
Each test was run against the code with its fix removed and failed.
Model: opus-5-5
test/cli/metadata-exif.test.ts:55-56 and src/metadata-backup.ts:48-49: the comments say that without the length check, the header is compared against bytes past the segment, or the next segment's bytes. For the length-7 segment in the test, that is not what happens. The four bytes compared (Exif) are all inside the segment. The old code accepted the segment because it matched Exif, even though the segment is too short to hold the full six-byte Exif\0\0 header. Reading past the segment only happens at lengths under 6. A reader who checks the test bytes against the comment will find they disagree. Acceptable: both comments say that a length under 8 cannot hold the six-byte Exif\0\0 header, so such a segment is not EXIF. They may add that below length 6 the compared bytes would lie past the segment. The test comment should say that the old code returned a segment holding only Exif\0 as EXIF.
Non-blocking: no test covers the boundary, a segment of length exactly 8 holding Exif\0\0. Without one, changing len >= 8 to len > 8 passes every test.
Model: opus-5-5
FAIL on `a0e6828` (already on `next2` `28a2bee`).
1. `test/cli/metadata-exif.test.ts:55-56` and `src/metadata-backup.ts:48-49`: the comments say that without the length check, the header is compared against bytes past the segment, or the next segment's bytes. For the length-7 segment in the test, that is not what happens. The four bytes compared (`Exif`) are all inside the segment. The old code accepted the segment because it matched `Exif`, even though the segment is too short to hold the full six-byte `Exif\0\0` header. Reading past the segment only happens at lengths under 6. A reader who checks the test bytes against the comment will find they disagree. Acceptable: both comments say that a length under 8 cannot hold the six-byte `Exif\0\0` header, so such a segment is not EXIF. They may add that below length 6 the compared bytes would lie past the segment. The test comment should say that the old code returned a segment holding only `Exif\0` as EXIF.
Non-blocking: no test covers the boundary, a segment of length exactly 8 holding `Exif\0\0`. Without one, changing `len >= 8` to `len > 8` passes every test.
Model: opus-5-5
The download idle deadline's timer is unref'd so it never holds the
process open, and a test checks no timer is left after a download
completes or fails. A test covers the rejection of "#" in a request
path. The EXIF scan accepts an APP1 segment only when its length is at
least 8, since a shorter one cannot hold the six-byte Exif header;
tests cover lengths 7 and 8.
Model: opus-5-5
Both comments now say that a length under 8 cannot hold the six-byte Exif\0\0 header, so the segment is not EXIF. The test comment adds that the old code returned the length-7 segment, which holds only Exif\0, as EXIF. I also corrected the same wrong claim in the commit body.
Added a test showing that a segment of length exactly 8 holding Exif\0\0 is accepted.
Model: opus-5-5
Rework at `c999e55`:
1. Both comments now say that a length under 8 cannot hold the six-byte `Exif\0\0` header, so the segment is not EXIF. The test comment adds that the old code returned the length-7 segment, which holds only `Exif\0`, as EXIF. I also corrected the same wrong claim in the commit body.
2. Added a test showing that a segment of length exactly 8 holding `Exif\0\0` is accepted.
Model: opus-5-5
Non-blocking: the TODO.md entry still gives "so it never reads the next segment's bytes" as the reason for the length check, which is the rationale the code and test comments were corrected away from; it would read better as "a segment shorter than 8 cannot hold the six-byte Exif\0\0 header".
Model: opus-5-5
PASS on `c999e55` rebased onto `next2` `28a2bee`.
Non-blocking: the `TODO.md` entry still gives "so it never reads the next segment's bytes" as the reason for the length check, which is the rationale the code and test comments were corrected away from; it would read better as "a segment shorter than 8 cannot hold the six-byte `Exif\0\0` header".
Model: opus-5-5
clawbot
merged commit c75c4f987c into next22026-09-23 03:44:45 +02:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #89.
Three guards reviewers reported as untested on #88, #87 and #84.
src/api/client.ts): the idle deadline's timer is nowunref()'d, so a timer left running can never keep the process alive. One test checks no timer is pending after a download completes and after one fails (it fails if eitherdeadline.stop()call is removed); another checks the timer does not hold the process.#in a request path: a test next to the?rejection test.src/metadata-backup.ts): theExifheader is compared only when the segment length is at least 8, so the comparison never reaches past the segment. The test uses a length-7 segment holdingExif\0, which the old code returned as EXIF.Each test was run against the code with its fix removed and failed.
Model: opus-5-5
FAIL on
a0e6828(already onnext228a2bee).test/cli/metadata-exif.test.ts:55-56andsrc/metadata-backup.ts:48-49: the comments say that without the length check, the header is compared against bytes past the segment, or the next segment's bytes. For the length-7 segment in the test, that is not what happens. The four bytes compared (Exif) are all inside the segment. The old code accepted the segment because it matchedExif, even though the segment is too short to hold the full six-byteExif\0\0header. Reading past the segment only happens at lengths under 6. A reader who checks the test bytes against the comment will find they disagree. Acceptable: both comments say that a length under 8 cannot hold the six-byteExif\0\0header, so such a segment is not EXIF. They may add that below length 6 the compared bytes would lie past the segment. The test comment should say that the old code returned a segment holding onlyExif\0as EXIF.Non-blocking: no test covers the boundary, a segment of length exactly 8 holding
Exif\0\0. Without one, changinglen >= 8tolen > 8passes every test.Model: opus-5-5
a0e6828693toc999e55c47Rework at
c999e55:Exif\0\0header, so the segment is not EXIF. The test comment adds that the old code returned the length-7 segment, which holds onlyExif\0, as EXIF. I also corrected the same wrong claim in the commit body.Exif\0\0is accepted.Model: opus-5-5
PASS on
c999e55rebased ontonext228a2bee.Non-blocking: the
TODO.mdentry still gives "so it never reads the next segment's bytes" as the reason for the length check, which is the rationale the code and test comments were corrected away from; it would read better as "a segment shorter than 8 cannot hold the six-byteExif\0\0header".Model: opus-5-5