Skip to content

Commit 8820219

Browse files
matmenclaude
andcommitted
Throw a RangeError for NaN and infinite arguments
Colors, coordinates, sizes, angles, factors, encoder levels, the font size and stroke, frame durations/offsets and loop counts go through util/options.ts' finite(), which converts like before (numeric strings work) and names the argument. NaN colors drew black, NaN coordinates the first pixel, infinite factors saturated everything. TextLayout keeps Infinity as "no limit", NaN throws instead of meaning no limit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YJgRK5qpwJEbHPuFv6kgAQ
1 parent 6e92127 commit 8820219

7 files changed

Lines changed: 137 additions & 50 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,14 @@
7070

7171
### Fixes
7272

73+
- NaN and infinite arguments throw a RangeError naming the argument instead of silently drawing black, transparent
74+
or nothing: colors (`fill`, `setPixelAt`, `drawBox`, `drawCircle`, `trim`, `colorToAlpha`, `Image.gradient`, the
75+
text and stroke colors, `Image.rgbaToColor` and the other color conversions, so `Image.rgbToColor(NaN, 0, 0)` is no
76+
longer black), coordinates and sizes (`getPixelAt`, `crop`, `composite`, `drawBox`, `drawCircle`, `contain`),
77+
angles and amounts (`rotate`, `hueShift`, `roundCorners`, `fisheye`, `cropCircle`), infinite factors (`opacity`,
78+
`red`, `lightness`, `brightness`, `contrast`, `gamma`, ...), the PNG compression and JPEG quality, the font size,
79+
frame durations and offsets, and loop counts. Numeric strings keep working. `TextLayout`'s `maxWidth` and `maxHeight`
80+
accept `Infinity` (no limit), NaN throws instead of meaning no limit.
7381
- The outline of `Image.renderText` (`strokeWidth`) showed through translucent text, it now only shows around the
7482
glyphs (opaque text renders as before).
7583
- The outline of `Image.renderText` follows the glyphs' antialiased edges instead of their pixels, so it is smooth

‎src/animation.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { Frame } from './frame.ts';
77
import { ImageType } from './image-type.ts';
88
import { optimize, OptimizeOptions, OptimizedFrame } from './util/optimize.ts';
99
import { Metadata, resolveMetadata, pngText, exifData, gifComments } from './metadata.ts';
10-
import { isOptions } from './util/options.ts';
10+
import { isOptions, finite } from './util/options.ts';
1111
import { MAX_PIXELS, decoding } from './util/limits.ts';
1212

1313
// loop counts: ImageScript (like GIF) counts repetitions (-1 forever, 0 play once), APNG and WebP count plays (0 forever)
@@ -99,7 +99,7 @@ export class Animation extends Array<Frame> {
9999
if (!(this[i] instanceof Frame))
100100
throw new TypeError(`Frame ${i} is not an instance of Frame`);
101101

102-
if (loopCount < -1 || isNaN(loopCount))
102+
if (finite(loopCount, 'loop count') < -1)
103103
throw new RangeError('Invalid loop count');
104104

105105
this.loopCount = loopCount;

‎src/frame.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { Image, ResizeMode, ResizeOptions, Color, TrimOptions } from './image.ts';
2-
import { isOptions } from './util/options.ts';
2+
import { isOptions, finite } from './util/options.ts';
33

44
/** Names of the frame disposal modes, see {@link Frame.disposalMode} */
55
export type FrameDisposalModeName = 'any' | 'keep' | 'previous' | 'background';
@@ -81,8 +81,10 @@ export class Frame extends Image {
8181
constructor(width: number, height: number, durationOrOptions?: number | FrameOptions, xOffset?: number, yOffset?: number, disposalMode?: FrameDisposalModeName | FrameDisposalModeId) {
8282
const { duration = 100, xOffset: x = 0, yOffset: y = 0, disposalMode: disposal = Frame.DISPOSAL_KEEP } =
8383
isOptions(durationOrOptions) ? durationOrOptions : { duration: durationOrOptions, xOffset, yOffset, disposalMode };
84-
if (isNaN(duration) || duration < 0)
84+
if (finite(duration, 'frame duration') < 0)
8585
throw new RangeError('Invalid frame duration');
86+
finite(x, 'x offset');
87+
finite(y, 'y offset');
8688

8789
super(width, height);
8890
this.duration = duration;

‎src/image.ts‎

Lines changed: 70 additions & 43 deletions
Large diffs are not rendered by default.

‎src/text-layout.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,11 +51,12 @@ export class TextLayout {
5151
constructor(options: TextLayoutOptions = {}) {
5252
const { maxWidth, maxHeight, wrapStyle, verticalAlign, horizontalAlign, wrapHardBreaks } = options;
5353

54-
this.maxWidth = maxWidth || Infinity;
54+
// (undefined and 0 mean no limit, NaN is invalid)
55+
this.maxWidth = maxWidth === undefined || maxWidth === 0 ? Infinity : Number(maxWidth);
5556
if (isNaN(this.maxWidth) || this.maxWidth < 1)
5657
throw new RangeError('Invalid maxWidth');
5758

58-
this.maxHeight = maxHeight || Infinity;
59+
this.maxHeight = maxHeight === undefined || maxHeight === 0 ? Infinity : Number(maxHeight);
5960
if (isNaN(this.maxHeight) || this.maxHeight < 1)
6061
throw new RangeError('Invalid maxHeight');
6162

‎src/util/options.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,16 @@
22
export function isOptions<T>(value: T): value is Extract<T, object> {
33
return typeof value === 'object' && value !== null && !ArrayBuffer.isView(value);
44
}
5+
6+
/**
7+
* A number argument, converted like arithmetic does (numeric strings keep working); NaN and the infinities throw
8+
* a RangeError naming the argument
9+
*/
10+
export function finite(value: number, name: string): number {
11+
const number = Number(value);
12+
if (!Number.isFinite(number)) throw new RangeError(`Invalid ${name}: ${value}`);
13+
return number;
14+
}
15+
16+
/** A color argument: a finite number (as RGBA, 0xrrggbbaa) */
17+
export const color = (value: number, name = 'color'): number => finite(value, name);

‎tests/regressions.js‎

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,9 @@ const rgba = (image, x, y) => Image.colorToRGBA(image.getPixelAt(x, y));
358358
// color components are rounded and clamped instead of wrapping
359359
{
360360
assert(Image.rgbaToColor(256, -1, 127.6, 300) === 0xff0080ff, 'rgbaToColor should clamp and round, got ' + Image.rgbaToColor(256, -1, 127.6, 300).toString(16));
361-
assert(Image.rgbToColor(NaN, 0, 0) === 0x000000ff, 'rgbToColor(NaN) should be 0');
361+
let threw = false;
362+
try { Image.rgbToColor(NaN, 0, 0); } catch (e) { threw = e instanceof RangeError; }
363+
assert(threw, 'rgbToColor(NaN) should throw a RangeError');
362364
}
363365

364366
// disposal modes read back as the names they are set with
@@ -655,4 +657,38 @@ const rgba = (image, x, y) => Image.colorToRGBA(image.getPixelAt(x, y));
655657
const nested = await Image.renderSVG(`<svg ${NS} width="100" height="100">${'<g>'.repeat(100000)}<rect width="10" height="10"/>${'</g>'.repeat(100000)}</svg>`, {size: 100, mode: Image.SVG_MODE_WIDTH});
656658
assert(nested.width === 100, 'deeply nested elements should render');
657659
}
660+
{
661+
// NaN and the infinities throw instead of drawing black, transparent or nothing; numeric strings still work
662+
const {Frame, Animation, TextLayout} = require('..');
663+
const image = () => new Image(8, 8).fill(0xff0000ff);
664+
const calls = {
665+
rgbaToColor: v => Image.rgbaToColor(v, 0, 0, 255), hslaToColor: v => Image.hslaToColor(v, 0.5, 0.5, 1), colorToRGBA: v => Image.colorToRGBA(v),
666+
getPixelAt: v => image().getPixelAt(v, 1), setPixelAt: v => image().setPixelAt(1, 1, v), fill: v => image().fill(v), contain: v => image().contain(v, 4),
667+
crop: v => image().crop(v, 0, 2, 2), trim: v => image().trim(v), drawBox: v => image().drawBox(0, 0, v, 2, 0), drawCircle: v => image().drawCircle(2, 2, v, 0),
668+
cropCircle: v => image().cropCircle(false, v), opacity: v => image().opacity(v), red: v => image().red(v), lightness: v => image().lightness(v),
669+
composite: v => image().composite(image(), v, 0), hueShift: v => image().hueShift(v), brightness: v => image().brightness(v), gamma: v => image().gamma(v),
670+
colorToAlpha: v => image().colorToAlpha(v), rotate: v => image().rotate(v, false), gradient: v => Image.gradient({0: v, 1: 0}),
671+
roundCorners: v => image().roundCorners(v), fisheye: v => image().fisheye(v), encode: v => image().encode(v), encodeJPEG: v => image().encodeJPEG(v),
672+
frame: v => new Frame(4, 4, v), animation: v => new Animation([new Frame(4, 4)], v),
673+
};
674+
for (const [name, call] of Object.entries(calls))
675+
for (const value of [NaN, Infinity, -Infinity]) {
676+
let error = null;
677+
try {
678+
await call(value);
679+
} catch (e) {
680+
error = e;
681+
}
682+
if (!(error instanceof RangeError)) panic(`${name}(${value}) should throw a RangeError, got ${error}`);
683+
}
684+
const font = require('fs').readFileSync('./tests/fonts/opensans bold.ttf');
685+
for (const [name, call] of Object.entries({size: v => Image.renderText(font, v, 'a'), color: v => Image.renderText(font, 12, 'a', v),
686+
maxWidth: v => new TextLayout({maxWidth: v})}))
687+
if (!await Promise.resolve().then(() => call(NaN)).then(() => false, e => e instanceof RangeError)) panic(`renderText: a NaN ${name} should throw a RangeError`);
688+
// (no limit is a valid layout)
689+
new TextLayout({maxWidth: Infinity, maxHeight: Infinity});
690+
691+
const strings = image().crop('1', '1', '4', '4').rotate('45').opacity('0.5').composite(image(), '1', '2');
692+
if (strings.width !== 6) panic('numeric strings should keep working');
693+
}
658694
})();

0 commit comments

Comments
 (0)