From 40625439bb6ce6114461515bdcdb62a9468d5ac8 Mon Sep 17 00:00:00 2001 From: "otoneko." Date: Sat, 29 Aug 2026 20:47:04 +0900 Subject: [PATCH 1/3] fix(util): stop a prerelease comparing as newer than its own release MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit toSegments() split on "." without first dropping a "-rc.1"/"+build5" suffix, so parseInt's usual truncate-at-the-first-non-digit-character behavior ("0-rc" -> 0) left the suffix's own numeric parts as extra, uncompared segments. "12.0.0-rc.1" parsed as [12, 0, 0, 1] against "12.0.0"'s [12, 0, 0], so isNewerVersion('12.0.0', '12.0.0-rc.1') came back true — a prerelease read as newer than the release it precedes, backwards from real semver precedence, and the reverse comparison flagged the release itself as not up to date even when it just shipped. Fixed by dropping everything from the first "-"/"+" before segmenting. This can't tell a prerelease apart from its release anymore (both parse to the same core), but every real caller (npm dist-tags, Twemoji/Google Fonts release tags) is a plain core version anyway, and comparing bare cores can only ever call them equal — never flip which one is "newer". Added version.test.ts; isNewerVersion had no dedicated tests despite package update, Twemoji update and font update checks all depending on it. --- src/util/version.test.ts | 52 ++++++++++++++++++++++++++++++++++++++++ src/util/version.ts | 13 ++++++++-- 2 files changed, 63 insertions(+), 2 deletions(-) create mode 100644 src/util/version.test.ts diff --git a/src/util/version.test.ts b/src/util/version.test.ts new file mode 100644 index 0000000..ddabeee --- /dev/null +++ b/src/util/version.test.ts @@ -0,0 +1,52 @@ +import { describe, expect, it } from 'vitest' +import { isNewerVersion } from './version' + +describe('isNewerVersion', () => { + it('is true when the minor version increases', () => { + expect(isNewerVersion('9.0.1', '9.1.0')).toBe(true) + }) + + it('is false when the versions are equal', () => { + expect(isNewerVersion('9.1.0', '9.1.0')).toBe(false) + }) + + it('is false when latest is older', () => { + expect(isNewerVersion('9.1.0', '9.0.1')).toBe(false) + }) + + it('treats a missing segment as zero', () => { + expect(isNewerVersion('1.0', '1.0.1')).toBe(true) + expect(isNewerVersion('1.0.1', '1.0')).toBe(false) + }) + + it('strips a leading v so tag styles compare the same way', () => { + expect(isNewerVersion('v30', 'v31')).toBe(true) + expect(isNewerVersion('30', 'v31')).toBe(true) + }) + + it('compares left to right by numeric value, not lexically', () => { + expect(isNewerVersion('1.9.0', '1.10.0')).toBe(true) + }) + + it('falls back to treating an unparseable segment as zero', () => { + // "not-a-version" has no digits at all, so every segment is 0 — the same + // as comparing against "0.0.0". + expect(isNewerVersion('1.0.0', 'not-a-version')).toBe(false) + expect(isNewerVersion('not-a-version', '1.0.0')).toBe(true) + }) + + it('does not treat a prerelease as newer than the release it precedes', () => { + expect(isNewerVersion('12.0.0', '12.0.0-rc.1')).toBe(false) + }) + + it('does not treat a release as newer than its own prerelease', () => { + // Not full semver — a prerelease and its release compare as equal here — + // but it must never flip in the wrong direction (see the previous case). + expect(isNewerVersion('12.0.0-rc.1', '12.0.0')).toBe(false) + }) + + it('ignores build metadata the same way', () => { + expect(isNewerVersion('1.2.3+build5', '1.2.3')).toBe(false) + expect(isNewerVersion('1.2.3', '1.2.3+build5')).toBe(false) + }) +}) diff --git a/src/util/version.ts b/src/util/version.ts index 7c80cf0..b409936 100644 --- a/src/util/version.ts +++ b/src/util/version.ts @@ -21,8 +21,17 @@ export function isNewerVersion(current: string, latest: string): boolean { } function toSegments(version: string): number[] { - return version - .replace(/^v/, '') + // Drop any prerelease/build suffix (`-rc.1`, `+build5`) before splitting on + // `.`: left in, "12.0.0-rc.1" parsed as [12, 0, 0, 1] — parseInt truncates + // "0-rc" down to the digits it starts with, so the trailing ".1" became a + // real extra segment, making the prerelease compare as *newer* than the + // release it precedes. Comparing bare cores instead can't tell a + // prerelease apart from its release, but that's a smaller, safer + // imprecision than the reversal — and every real caller here (npm + // dist-tags, Twemoji/Google Fonts release tags) is a plain core version. + const core = version.replace(/^v/, '').split(/[-+]/)[0] as string + + return core .split('.') .map((part) => Number.parseInt(part, 10)) .map((n) => (Number.isFinite(n) ? n : 0)) From 4437cb521c1cf940765275c0d29abd6999cf69a9 Mon Sep 17 00:00:00 2001 From: "otoneko." Date: Sat, 29 Aug 2026 20:47:14 +0900 Subject: [PATCH 2/3] fix(render): round desaturateRegion's box outward to integer pixels MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit box comes straight from ratio-based layout math (widthRatio * canvas width, etc.) and is rarely integer-aligned, but getImageData/putImageData need integer x/y/width/height. Passed through as-is, the fractional edge of the region could be clipped or misaligned in the ctx.filter-unsupported grayscale fallback, depending on how the binding coerces the values. Floors the origin and ceils the far edge instead, so the integer rectangle is always a superset of the original fractional one — the desaturated region can end up a fraction of a pixel larger than painted, never smaller. Added a test exercising this fallback path directly (drawAvatar had no tests at all): forced supportsFilter() to false via a new test seam and checked a pixel at the box's fractional far edge, which failed before this fix and passes after it. --- src/render/avatar.test.ts | 39 ++++++++++++++++++++++++++++++++++++++- src/render/avatar.ts | 23 ++++++++++++++++++++--- 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/src/render/avatar.test.ts b/src/render/avatar.test.ts index 16cda5f..b93d0fe 100644 --- a/src/render/avatar.test.ts +++ b/src/render/avatar.test.ts @@ -2,7 +2,12 @@ import { readFile } from 'node:fs/promises' import { fileURLToPath } from 'node:url' import { afterEach, describe, expect, it, vi } from 'vitest' import type { AvatarFetcher } from './avatar' -import { loadAvatar } from './avatar' +import { + drawAvatar, + loadAvatar, + resetFilterDetectionForTests, + setFilterSupportedForTests, +} from './avatar' import { avatarCacheInfo, clearAvatarCache, configureAvatarCache } from './avatarCache' import { createCanvas } from './canvasFactory' @@ -148,3 +153,35 @@ describe('loadAvatar', () => { expect(fetcher.calls).toHaveLength(2) }) }) + +describe('drawAvatar grayscale fallback (no ctx.filter support)', () => { + afterEach(() => { + resetFilterDetectionForTests() + }) + + it('desaturates the whole painted box, including its fractional far edge', async () => { + setFilterSupportedForTests(false) + + const canvas = createCanvas(5, 5) + const ctx = canvas.getContext('2d') + const image = await loadAvatar(redSquare()) + + drawAvatar(ctx, image, { + theme: { + grayscale: true, + position: 'left', + widthRatio: 1, + fit: 'cover', + shape: 'rectangle', + fallback: null, + }, + // A non-integer box: painting/desaturating must cover pixel (4, 4) too, + // not just the 4x4 region a truncated width/height would leave. + box: { x: 0, y: 0, width: 4.9, height: 4.9 }, + }) + + const { data } = ctx.getImageData(4, 4, 1, 1) + expect(data[0]).toBe(data[1]) + expect(data[1]).toBe(data[2]) + }) +}) diff --git a/src/render/avatar.ts b/src/render/avatar.ts index f69e0f5..a7ecc2c 100644 --- a/src/render/avatar.ts +++ b/src/render/avatar.ts @@ -158,9 +158,21 @@ function luma(r: number, g: number, b: number): number { return 0.2126 * r + 0.7152 * g + 0.0722 * b } -/** Desaturates a region in place, for when `ctx.filter` isn't available. */ +/** + * Desaturates a region in place, for when `ctx.filter` isn't available. + * + * `box` comes from ratio-based layout math and is rarely integer-aligned; + * `getImageData`/`putImageData` need integer pixels, so this rounds outward + * (floor the origin, ceil the far edge) rather than truncating, to always + * cover the whole painted area instead of clipping a row or column of it. + */ function desaturateRegion(ctx: SKRSContext2D, box: AvatarBox): void { - const image = ctx.getImageData(box.x, box.y, box.width, box.height) + const x = Math.floor(box.x) + const y = Math.floor(box.y) + const width = Math.ceil(box.x + box.width) - x + const height = Math.ceil(box.y + box.height) - y + + const image = ctx.getImageData(x, y, width, height) const { data } = image for (let i = 0; i < data.length; i += 4) { const value = luma(data[i] as number, data[i + 1] as number, data[i + 2] as number) @@ -168,7 +180,7 @@ function desaturateRegion(ctx: SKRSContext2D, box: AvatarBox): void { data[i + 1] = value data[i + 2] = value } - ctx.putImageData(image, box.x, box.y) + ctx.putImageData(image, x, y) } export interface DrawAvatarOptions { @@ -244,3 +256,8 @@ export function drawAvatar( export function resetFilterDetectionForTests(): void { filterSupported = null } + +/** Test seam: forces `supportsFilter()`'s result, to exercise the fallback path deterministically. */ +export function setFilterSupportedForTests(value: boolean): void { + filterSupported = value +} From 882c6469262218a32941f0d69fb431426f469c85 Mon Sep 17 00:00:00 2001 From: "otoneko." Date: Sat, 29 Aug 2026 20:47:25 +0900 Subject: [PATCH 3/3] fix(cli): use HEAD for env's reachability check instead of GET MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit checkReachable() only cares whether a request completes at all — even an error response proves the network and TLS are fine, per its own doc comment — so it never needed the body a GET downloads. registry.npmjs.org's `/makeitaquote/latest` in particular returns a full package manifest just to confirm the registry is reachable. Adds a head() method to HttpClient (identical to get(), just a different method) rather than a special case: a non-2xx or even a 405 for HEAD itself still proves reachability under this function's own definition, so no per-host allowance is needed for a server that doesn't implement HEAD well. --- src/cli/env.ts | 11 +++++++++-- src/http/client.ts | 5 ++++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/cli/env.ts b/src/cli/env.ts index caacdcc..9508e70 100644 --- a/src/cli/env.ts +++ b/src/cli/env.ts @@ -58,10 +58,17 @@ async function checkWritable(dir: string): Promise { } } -/** A response, even an error one, means the network and TLS are fine. */ +/** + * A response, even an error one, means the network and TLS are fine. + * + * HEAD rather than GET: this only asks whether the host is reachable, not + * for anything in the body, so there's no reason to download one — even an + * error status or a 405 for HEAD itself still proves the network and TLS + * are fine, which is all this checks. + */ async function checkReachable(url: string): Promise { try { - await http.get(url, { throwHttpErrors: false }) + await http.head(url, { throwHttpErrors: false }) return true } catch { return false diff --git a/src/http/client.ts b/src/http/client.ts index c63b299..00221d2 100644 --- a/src/http/client.ts +++ b/src/http/client.ts @@ -17,6 +17,8 @@ export interface RequestOptions { export interface HttpClient { get(url: string, options?: RequestOptions): Promise post(url: string, options?: RequestOptions): Promise + /** HEADs `url` — the same reachability signal as `get`, without downloading the body. */ + head(url: string, options?: RequestOptions): Promise /** GETs `url` and reads the body into a `Buffer` — the shape every asset fetcher needs. */ getBuffer(url: string, signal?: AbortSignal): Promise } @@ -61,7 +63,7 @@ export function createClient(options: HttpOptions = {}): HttpClient { }) async function request( - method: 'GET' | 'POST', + method: 'GET' | 'HEAD' | 'POST', url: string, options: RequestOptions, ): Promise { @@ -105,6 +107,7 @@ export function createClient(options: HttpOptions = {}): HttpClient { return { get, + head: (url, options = {}) => request('HEAD', url, options), post: (url, options = {}) => request('POST', url, options), getBuffer: async (url, signal) => { const response = await get(url, signal ? { signal } : {})