From fd29fe2f99210c434995df97d6417f50d02c7ae4 Mon Sep 17 00:00:00 2001 From: Floyd May Date: Mon, 6 May 2019 08:51:49 -0500 Subject: [PATCH 1/3] throw errors when dash receives invalid args --- CHANGELOG.md | 1 + docs/vector.md | 3 ++- lib/mixins/vector.js | 22 +++++++++--------- tests/unit/document.spec.js | 1 + tests/unit/vector.spec.js | 45 +++++++++++++++++++++++++++++++++++++ 5 files changed, 59 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b01c936..325ad15 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ - Fix links to pages within the document - Add support for named destinations +- Throw errors when `dash(...)` is passed invalid lengths ### [v0.9.1] - 2019-04-30 diff --git a/docs/vector.md b/docs/vector.md index 5c6305e..07138d6 100644 --- a/docs/vector.md +++ b/docs/vector.md @@ -145,7 +145,8 @@ The output of this example looks like this. The `dash` method allows you to create non-continuous dashed lines. It takes a length specifying how long each dash should be, as well as an optional hash -describing the additional properties `space` and `phase`. +describing the additional properties `space` and `phase`. Lengths must be positive +numbers; `dash` will throw if passed invalid lengths. The `space` option defines the length of the space between each dash, and the `phase` option defines the starting point of the sequence of dashes. By default the `space` diff --git a/lib/mixins/vector.js b/lib/mixins/vector.js index 61dbac4..62046df 100644 --- a/lib/mixins/vector.js +++ b/lib/mixins/vector.js @@ -63,20 +63,18 @@ export default { dash(length, options = {}) { let phase; - if (length == null) { - return this; + const originalLength = length; + if (!Array.isArray(length)) { + length = [length, options.space || length]; } - if (Array.isArray(length)) { - length = length.map(v => number(v)).join(' '); - phase = options.phase || 0; - return this.addContent(`[${length}] ${number(phase)} d`); - } else { - const space = options.space != null ? options.space : length; - phase = options.phase || 0; - return this.addContent( - `[${number(length)} ${number(space)}] ${number(phase)} d` - ); + + const valid = length.every(x => Number.isFinite(x) && x > 0); + if(!valid) { + throw new Error(`dash(${JSON.stringify(originalLength)}, ${JSON.stringify(options)}) invalid, lengths must be numeric and greater than zero`); } + length = length.map(v => number(v)).join(' '); + phase = options.phase || 0; + return this.addContent(`[${length}] ${number(phase)} d`); }, undash() { diff --git a/tests/unit/document.spec.js b/tests/unit/document.spec.js index 3f26c3d..ef0c380 100644 --- a/tests/unit/document.spec.js +++ b/tests/unit/document.spec.js @@ -32,4 +32,5 @@ describe('PDFDocument', () => { expect(fontSpy).not.toBeCalled(); }); }); + }); diff --git a/tests/unit/vector.spec.js b/tests/unit/vector.spec.js index 8540bb8..9b9cdbe 100644 --- a/tests/unit/vector.spec.js +++ b/tests/unit/vector.spec.js @@ -111,5 +111,50 @@ describe('Vector Graphics', () => { `endobj` ]); }); + + describe('validation', () => { + + test('length 1', () => { + const doc = new PDFDocument(); + + expect(() => doc.dash(1)).not.toThrow(); + }); + + test('length 1.5', () => { + const doc = new PDFDocument(); + + expect(() => doc.dash(1.5)).not.toThrow(); + }); + + test('length 0 throws', () => { + const doc = new PDFDocument(); + + expect(() => doc.dash(0)).toThrow('dash(0, {}) invalid, lengths must be numeric and greater than zero'); + }); + + test('length -1 throws', () => { + const doc = new PDFDocument(); + + expect(() => doc.dash(-1)).toThrow('dash(-1, {}) invalid, lengths must be numeric and greater than zero'); + }); + + test('length null throws', () => { + const doc = new PDFDocument(); + + expect(() => doc.dash(null)).toThrow('dash(null, {}) invalid, lengths must be numeric and greater than zero'); + }); + + test('length array', () => { + const doc = new PDFDocument(); + + expect(() => doc.dash([2,3])).not.toThrow(); + }); + + test('length array containing zeros throws', () => { + const doc = new PDFDocument(); + + expect(() => doc.dash([2, 0, 3])).toThrow('dash([2,0,3], {}) invalid, lengths must be numeric and greater than zero'); + }); + }); }); }); From 77dce7734138d451fecb541b593503fd32f10e00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luiz=20Am=C3=A9rico?= Date: Tue, 7 May 2019 08:29:07 -0500 Subject: [PATCH 2/3] simplify string join expression Co-Authored-By: floyd-may --- lib/mixins/vector.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/mixins/vector.js b/lib/mixins/vector.js index 62046df..df0fcf5 100644 --- a/lib/mixins/vector.js +++ b/lib/mixins/vector.js @@ -72,7 +72,7 @@ export default { if(!valid) { throw new Error(`dash(${JSON.stringify(originalLength)}, ${JSON.stringify(options)}) invalid, lengths must be numeric and greater than zero`); } - length = length.map(v => number(v)).join(' '); + length = length.map(number).join(' '); phase = options.phase || 0; return this.addContent(`[${length}] ${number(phase)} d`); }, From 5eb7b0d9787d9ff92606ec1669670bf7b6caa064 Mon Sep 17 00:00:00 2001 From: Floyd May Date: Tue, 7 May 2019 08:32:25 -0500 Subject: [PATCH 3/3] code review tweaks --- lib/mixins/vector.js | 5 ++--- tests/unit/document.spec.js | 1 - 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/mixins/vector.js b/lib/mixins/vector.js index df0fcf5..c0a3d62 100644 --- a/lib/mixins/vector.js +++ b/lib/mixins/vector.js @@ -62,7 +62,6 @@ export default { }, dash(length, options = {}) { - let phase; const originalLength = length; if (!Array.isArray(length)) { length = [length, options.space || length]; @@ -72,9 +71,9 @@ export default { if(!valid) { throw new Error(`dash(${JSON.stringify(originalLength)}, ${JSON.stringify(options)}) invalid, lengths must be numeric and greater than zero`); } + length = length.map(number).join(' '); - phase = options.phase || 0; - return this.addContent(`[${length}] ${number(phase)} d`); + return this.addContent(`[${length}] ${number(options.phase || 0)} d`); }, undash() { diff --git a/tests/unit/document.spec.js b/tests/unit/document.spec.js index ef0c380..3f26c3d 100644 --- a/tests/unit/document.spec.js +++ b/tests/unit/document.spec.js @@ -32,5 +32,4 @@ describe('PDFDocument', () => { expect(fontSpy).not.toBeCalled(); }); }); - });