From 96ae68d91477a13ad58cde70b92eb3a1dd5f5e31 Mon Sep 17 00:00:00 2001 From: Evert Pot Date: Wed, 2 Aug 2023 20:19:11 -0400 Subject: [PATCH 1/6] Checking for TypeError --- .github/workflows/test.yml | 4 ++-- test/httpwg-tests.js | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 9c99e9b..705fa68 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -5,9 +5,9 @@ name: Node.js CI on: push: - branches: [ main ] + branches: [ main, 'version/*.x' ] pull_request: - branches: [ main ] + branches: [ main, 'version/*.x' ] jobs: build: diff --git a/test/httpwg-tests.js b/test/httpwg-tests.js index 362eeda..cbab54d 100644 --- a/test/httpwg-tests.js +++ b/test/httpwg-tests.js @@ -234,6 +234,9 @@ function makeSerializeTest(test) { try { expect(output).to.deep.equal(expected); } catch (e) { + if (e instanceof TypeError) { + throw new Error('Test emitted a TypeError, but we should only emit ParseErrors', { cause: e }); + } if (test.can_fail) { // Optional failure this.skip('can_fail was true'); From 828c9da2973ad5f45abd67876c89d7649c773dcd Mon Sep 17 00:00:00 2001 From: Evert Pot Date: Wed, 2 Aug 2023 20:27:19 -0400 Subject: [PATCH 2/6] Fix test --- test/httpwg-tests.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/test/httpwg-tests.js b/test/httpwg-tests.js index cbab54d..cec945f 100644 --- a/test/httpwg-tests.js +++ b/test/httpwg-tests.js @@ -118,13 +118,13 @@ function makeParseTest(test) { if (test.must_fail) { expect(hadError).to.equal(true, 'Parsing this should result in a failure'); + expect(caughtError instanceof ParseError).to.equal(true); } else { if (hadError) { - // There was an error + if (test.can_fail) { - // Failure is OK - expect(hadError).to.equal(true); + expect(caughtError instanceof ParseError).to.equal(true); } else { // Failure is NOT OK throw new Error('We should not have failed but got an error: ' + caughtError.message); @@ -223,6 +223,7 @@ function makeSerializeTest(test) { if (hadError) { // There was an error if (test.can_fail) { + // Failure is OK expect(hadError).to.equal(true); } else { @@ -234,9 +235,7 @@ function makeSerializeTest(test) { try { expect(output).to.deep.equal(expected); } catch (e) { - if (e instanceof TypeError) { - throw new Error('Test emitted a TypeError, but we should only emit ParseErrors', { cause: e }); - } + if (test.can_fail) { // Optional failure this.skip('can_fail was true'); From e442452ab65b10b2ff1fa4e4e0da1de020d58fa2 Mon Sep 17 00:00:00 2001 From: Evert Pot Date: Wed, 2 Aug 2023 21:15:49 -0400 Subject: [PATCH 3/6] Making sure we never emit TypeError --- src/parser.ts | 18 ++++++++++++------ test/httpwg-tests.js | 35 ++++++++++++++++++++++++++--------- 2 files changed, 38 insertions(+), 15 deletions(-) diff --git a/src/parser.ts b/src/parser.ts index 10fec48..a2bf589 100644 --- a/src/parser.ts +++ b/src/parser.ts @@ -164,6 +164,9 @@ export default class Parser { private parseBareItem(): BareItem { const char = this.lookChar(); + if (char === undefined) { + throw new ParseError(this.pos, 'Unexpected end of string'); + } if (char.match(/^[-0-9]/)) { return this.parseIntegerOrDecimal(); } @@ -284,7 +287,7 @@ export default class Parser { } else if (char === '"') { return outputString; } else if (!isAscii(char)) { - throw new Error('Strings must be in the ASCII range'); + throw new ParseError(this.pos, 'Strings must be in the ASCII range'); } else { outputString += char; } @@ -305,7 +308,7 @@ export default class Parser { while(!this.eof()) { const char = this.lookChar(); - if (!/^[:/!#$%&'*+\-.^_`|~A-Za-z0-9]$/.test(char)) { + if (char===undefined || !/^[:/!#$%&'*+\-.^_`|~A-Za-z0-9]$/.test(char)) { return new Token(outputString); } outputString += this.getChar(); @@ -352,7 +355,7 @@ export default class Parser { private parseKey(): string { - if (!this.lookChar().match(/^[a-z*]/)) { + if (!this.lookChar()?.match(/^[a-z*]/)) { throw new ParseError(this.pos, 'A key must begin with an asterisk or letter (a-z)'); } @@ -360,7 +363,7 @@ export default class Parser { while(!this.eof()) { const char = this.lookChar(); - if (!/^[a-z0-9_\-.*]$/.test(char)) { + if (char===undefined || !/^[a-z0-9_\-.*]$/.test(char)) { return outputString; } outputString += this.getChar(); @@ -372,8 +375,10 @@ export default class Parser { /** * Looks at the next character without advancing the cursor. + * + * Returns undefined if we were at the end of the string. */ - private lookChar():string { + private lookChar():string|undefined { return this.input[this.pos]; @@ -436,8 +441,9 @@ export default class Parser { } const isDigitRegex = /^[0-9]$/; -function isDigit(char: string): boolean { +function isDigit(char: string|undefined): boolean { + if (char===undefined) return false; return isDigitRegex.test(char); } diff --git a/test/httpwg-tests.js b/test/httpwg-tests.js index cec945f..11c8f94 100644 --- a/test/httpwg-tests.js +++ b/test/httpwg-tests.js @@ -1,6 +1,15 @@ const expect = require('chai').expect; -const parser = require('../dist/parser'); -const serializer = require('../dist/serializer'); +const { + parseItem, + parseList, + parseDictionary, + + serializeItem, + serializeList, + serializeDictionary, + + ParseError, +} = require('../dist'); const { Token, ByteSequence } = require('../dist'); const base32Encode = require('base32-encode'); const base32Decode = require('base32-decode'); @@ -100,13 +109,13 @@ function makeParseTest(test) { try { switch(test.header_type) { case 'item' : - result = parser.parseItem(input); + result = parseItem(input); break; case 'list' : - result = parser.parseList(input); + result = parseList(input); break; case 'dictionary' : - result = parser.parseDictionary(input); + result = parseDictionary(input); break; default: throw new Error('Unsupported header type: ' + test.header_type); @@ -118,7 +127,15 @@ function makeParseTest(test) { if (test.must_fail) { expect(hadError).to.equal(true, 'Parsing this should result in a failure'); - expect(caughtError instanceof ParseError).to.equal(true); + + if (!(caughtError instanceof ParseError)) { + console.error('Original error:'); + console.error(caughtError); + throw new Error( + `Errors during the parsing phase should be of type "ParseError" We got: "${caughtError.constructor.name}"`, + {cause: caughtError} + ); + } } else { if (hadError) { @@ -200,13 +217,13 @@ function makeSerializeTest(test) { try { switch(test.header_type) { case 'item' : - output = serializer.serializeItem(unpackTestValue(input)); + output = serializeItem(unpackTestValue(input)); break; case 'list' : - output = serializer.serializeList(unpackTestValue(input)); + output = serializeList(unpackTestValue(input)); break; case 'dictionary' : - output = serializer.serializeDictionary(unpackDictionary(input)); + output = serializeDictionary(unpackDictionary(input)); break; default: throw new Error('Unsupported header type: ' + test.header_type); From fc25fc3d93b6267cd0bddd2e3a93d927ff828053 Mon Sep 17 00:00:00 2001 From: Evert Pot Date: Wed, 2 Aug 2023 21:19:17 -0400 Subject: [PATCH 4/6] Update changelog --- changelog.md | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/changelog.md b/changelog.md index e6dd8eb..3febceb 100644 --- a/changelog.md +++ b/changelog.md @@ -1,6 +1,14 @@ ChangeLog ========= +1.0.1 (????-??-??) +------------------ + +* This library emitted `TypeError` or a plain `Error` in a few places in the + parser, where it should have been `ParseError` this is corrected everywhere + now. + + 1.0.0 (2023-06-13) ------------------ @@ -84,9 +92,9 @@ ChangeLog * First version! * Parses all of the [04 draft of the specification][1]. -[1]: https://tools.ietf.org/html/draft-ietf-httpbis-header-structure-04 [2]: -https://tools.ietf.org/html/draft-ietf-httpbis-header-structure-09 [3]: -https://tools.ietf.org/html/draft-ietf-httpbis-header-structure-10 [4]: -https://tools.ietf.org/html/draft-ietf-httpbis-header-structure-13 [5]: -https://datatracker.ietf.org/doc/html/rfc8941 [6]: -https://github.com/httpwg/structured-field-tests +[1]: https://tools.ietf.org/html/draft-ietf-httpbis-header-structure-04 +[2]: https://tools.ietf.org/html/draft-ietf-httpbis-header-structure-09 +[3]: https://tools.ietf.org/html/draft-ietf-httpbis-header-structure-10 +[4]: https://tools.ietf.org/html/draft-ietf-httpbis-header-structure-13 +[5]: https://datatracker.ietf.org/doc/html/rfc8941 +[6]: https://github.com/httpwg/structured-field-tests From 21ce510112904ac1ce98a17022b228e3663fc30c Mon Sep 17 00:00:00 2001 From: Evert Pot Date: Wed, 2 Aug 2023 21:21:25 -0400 Subject: [PATCH 5/6] Fix branch rules --- .github/workflows/test.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 705fa68..36315eb 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -5,9 +5,9 @@ name: Node.js CI on: push: - branches: [ main, 'version/*.x' ] + branches: [ main, 'version/x.*' ] pull_request: - branches: [ main, 'version/*.x' ] + branches: [ main, 'version/x.*' ] jobs: build: From 7dc86206d69e98703f69439dd3ba2870806c3629 Mon Sep 17 00:00:00 2001 From: Evert Pot Date: Wed, 2 Aug 2023 21:22:39 -0400 Subject: [PATCH 6/6] Try a different wildcard --- .github/workflows/test.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 36315eb..99ccf3e 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -5,9 +5,9 @@ name: Node.js CI on: push: - branches: [ main, 'version/x.*' ] + branches: [ main, 'version/**' ] pull_request: - branches: [ main, 'version/x.*' ] + branches: [ main, 'version/**' ] jobs: build: