Skip to content

Commit

Permalink
✨ Implement RFC6868 ^ parameter value escaping
Browse files Browse the repository at this point in the history
Closes #8

Drive-by: Split unescaping quoted parameter values from unescaping
single property values

Drive-by: Parameter value unescaping nags no longer ignored

Drive-by: No unescape `\n` in double-quoted parameter values

Drive-by: Support RFC6350 `LABEL` property, which is not in ToC
  • Loading branch information
MarcelWaldvogel committed Nov 19, 2021
1 parent 412d873 commit d6e4f72
Show file tree
Hide file tree
Showing 9 changed files with 297 additions and 18 deletions.
13 changes: 13 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,24 @@ project adheres to [Semantic Versioning](https://semver.org/).
- Support RFC6715 properties `EXPERTISE`, `INTEREST`, `HOBBY`, `ORG-DIRECTORY`
and parameters `LEVEL`, `INDEX`
- Support RFC6474 properties `BIRTHPLACE`, `DEATHPLACE`, `DEATHDATE`
- Support RFC6868 double-quoted parameter value escaping using `^` (no more
support for `\n` for these, see changes below).
- Support RFC6350 parameter `LABEL`, which does not have its own section, but is
only mentioned (essentially "in passing") in the `ADR` description.

## Fixed

- Did not nag at all about escaping warnings in double-quoted parameter values

## Changed

- Backslash sequences in double-quoted parameter values (e.g., `\n`) are passed
along transparently, in accordance with RFC6868. `\n` in parameter values _not
enclosed in double quotes_ will still be processed, as mandated for the
`LABEL` parameter in
[RFC6350, `ADR` section](https://datatracker.ietf.org/doc/html/rfc6350#section-6.3.1).
- As of RFC6868 support, single-valued property values no longer use the same
unescaping code as double-quoted parameter values.
- Ignore "should not happen" code from coverage

# 0.3.1 - 2021-11-08
Expand Down
8 changes: 8 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,14 @@ itself.
- `PARAM_UNESCAPED_COMMA`: A local warning. A parameter accepting only a single
value contained an unescaped comma. This may indicate incomplete character
escaping or trying to provide multiple values where they are not allowed.
- `PARAM_BAD_BACKSLASH`: A local warning. In a double-quoted parameter value, a
backslash was found. Escaping in quoted parameter values should be according
to RFC6868, using circumflexes (`^`). This indicates a possible problem in the
input file; the backslash was not treated as a special character.
- `PARAM_BAD_CIRCUMFLEX`: A local warning. In a double-quoted parameter value, a
circumflex (`^`) was found, which was not part of an escape sequence. This
indicates a possible problem in the input file; that circumflex was not
treated as a special character.
- `VALUE_INVALID`: A local error. A property with a required value had a
different value.
- `VALUE_UNESCAPED_COMMA`: A local warning. A property accepting only a single
Expand Down
5 changes: 5 additions & 0 deletions src/errorCodes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ export const errors = {
PARAM_NOT_SINGLE: ["Internal error: Single parameter isn't", true],
PARAM_DUPLICATE: ['Illegal duplicate parameter', true],
PARAM_UNESCAPED_COMMA: ['Unescaped comma in parameter value', false],
PARAM_BAD_BACKSLASH: ['Backslash found in parameter value', false],
PARAM_BAD_CIRCUMFLEX: [
'Circumflex not part of escape sequence in parameter value',
false,
],
// Problems concerning the actual property value
VALUE_INVALID: ['Invalid property value', true],
VALUE_UNESCAPED_COMMA: ['Unescaped comma in value', false],
Expand Down
9 changes: 7 additions & 2 deletions src/parse.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { nag, Nag, nagVC, VCardNagAttributes } from './errors';
import { maybeArray, NonEmptyArray } from './nonEmptyArray';
import { scanSingleValue } from './scan';
import { scanSingleParamValue } from './scan';
import { isPropertyChar, nameToKey } from './utils';
import {
atLeastOnceProperties,
Expand Down Expand Up @@ -401,7 +401,12 @@ export function scanParamValues(
return null;
}
parameterValues.push(
scanSingleValue(line.substring(index + 1, closingQuote), null),
scanSingleParamValue(
line.substring(index + 1, closingQuote),
(error) => {
nagVC(nags, error, { property, parameter, line });
},
),
);
index = closingQuote + 1;
} else {
Expand Down
1 change: 0 additions & 1 deletion src/runtimeTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// Modeled after [https://github.com/colinhacks/zod](Zod).

import { errorKeys } from './errorCodes';
import { nagVC } from './errors';
import { NonEmptyArray } from './nonEmptyArray';
import { scan1DValue, scan2DValue, scanSingleValue } from './scan';

Expand Down
52 changes: 51 additions & 1 deletion src/scan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { errorKeys } from './errorCodes';
import { NonEmptyArray } from './nonEmptyArray';

/**
* Remove escape sequences from a quoted parameter string.
* Remove escape sequences from a single value string.
* Essentially just understands \n, as almost all other escaping is either unnecessary or illegal.
* @param s Escaped (formerly quoted) parameter value string
* @returns Unescaped string
Expand Down Expand Up @@ -36,6 +36,56 @@ export function scanSingleValue(
return value;
}

/**
* Handle escape sequences in a quoted parameter string.
* - RFC6868 (`^^`, `^'`, and `^n`)
* - Nag about `\n`
* @param s Escaped (formerly quoted) parameter value string
* @returns Unescaped string
*/
export function scanSingleParamValue(
s: string,
errorCallback: ((error: errorKeys) => void) | null,
): string {
let index = 0;
let value = '';
let warnings: Set<errorKeys> = new Set();
while (index < s.length) {
const char = s.charAt(index);
if (char === '^') {
const escaped = s.charAt(index + 1);
switch (escaped) {
case '^':
value += '^';
break;
case "'":
value += '"';
break;
case 'n': // RFC says U+005E only
value += '\n';
break;
default:
value += '^' + escaped;
warnings.add('PARAM_BAD_CIRCUMFLEX');
break;
}
index += 2;
} else {
if (char === ',') {
warnings.add('PARAM_UNESCAPED_COMMA');
} else if (char === '\\') {
warnings.add('PARAM_BAD_BACKSLASH');
}
value += char;
index++;
}
}
if (errorCallback) {
warnings.forEach(errorCallback);
}
return value;
}

/**
* Splits a property value at semicolons *OR* commas.
* Observes escaped commas/semicolons, and replaces \n.
Expand Down
114 changes: 100 additions & 14 deletions src/tests/parse.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -391,27 +391,70 @@ describe('String parameter parsing', () => {
parameters: { LANGUAGE: 'a,b;c:d' },
end: 20,
});
expect(nags).toStrictEqual([]);
expect(nags).toStrictEqual([
{
attributes: {
line: 'x;LANGUAGE="a,b;c:d":x',
parameter: 'LANGUAGE',
property: 'X',
},
description: 'Unescaped comma in parameter value',
isError: false,
key: 'PARAM_UNESCAPED_COMMA',
},
]);
});
it('should handle newlines in quoted values', () => {
let nags: Nag<VCardNagAttributes>[] = [];
expect(
parseParameters('x;LANGUAGE="A\\nB\\\\nC":x', 1, nags, 'X'),
parseParameters('x;LANGUAGE="A^nB\\^nC":x', 1, nags, 'X'),
).toStrictEqual({
parameters: { LANGUAGE: 'A\nB\\nC' },
parameters: { LANGUAGE: 'A\nB\\\nC' },
end: 21,
});
expect(nags).toStrictEqual([]);
expect(nags).toStrictEqual([
{
attributes: {
line: 'x;LANGUAGE="A^nB\\^nC":x',
parameter: 'LANGUAGE',
property: 'X',
},
description: 'Backslash found in parameter value',
isError: false,
key: 'PARAM_BAD_BACKSLASH',
},
]);
});
it('should ignore most other backslashes in quoted values', () => {
it('should nag about backslashes in quoted values', () => {
let nags: Nag<VCardNagAttributes>[] = [];
expect(
parseParameters('x;LANGUAGE="a\\,b;c:d\\":x', 1, nags, 'X'),
).toStrictEqual({
parameters: { LANGUAGE: 'a,b;c:d' },
parameters: { LANGUAGE: 'a\\,b;c:d\\' },
end: 22,
});
expect(nags).toStrictEqual([]);
expect(nags).toStrictEqual([
{
attributes: {
line: 'x;LANGUAGE="a\\,b;c:d\\":x',
parameter: 'LANGUAGE',
property: 'X',
},
description: 'Backslash found in parameter value',
isError: false,
key: 'PARAM_BAD_BACKSLASH',
},
{
attributes: {
line: 'x;LANGUAGE="a\\,b;c:d\\":x',
parameter: 'LANGUAGE',
property: 'X',
},
description: 'Unescaped comma in parameter value',
isError: false,
key: 'PARAM_UNESCAPED_COMMA',
},
]);
});
});

Expand Down Expand Up @@ -522,27 +565,70 @@ describe('Multi-string parameter parsing', () => {
parameters: { TYPE: ['a,b;c:d'] },
end: 16,
});
expect(nags).toStrictEqual([]);
expect(nags).toStrictEqual([
{
attributes: {
line: 'x;TYPE="a,b;c:d":x',
parameter: 'TYPE',
property: 'X',
},
description: 'Unescaped comma in parameter value',
isError: false,
key: 'PARAM_UNESCAPED_COMMA',
},
]);
});
it('should handle newlines in quoted values', () => {
let nags: Nag<VCardNagAttributes>[] = [];
expect(
parseParameters('x;TYPE="A\\nB\\\\nC":x', 1, nags, 'X'),
parseParameters('x;TYPE="A^n\\nB\\^nC":x', 1, nags, 'X'),
).toStrictEqual({
parameters: { TYPE: ['A\nB\\nC'] },
end: 17,
parameters: { TYPE: ['A\n\\nB\\\nC'] },
end: 19,
});
expect(nags).toStrictEqual([]);
expect(nags).toStrictEqual([
{
attributes: {
line: 'x;TYPE="A^n\\nB\\^nC":x',
parameter: 'TYPE',
property: 'X',
},
description: 'Backslash found in parameter value',
isError: false,
key: 'PARAM_BAD_BACKSLASH',
},
]);
});
it('should ignore most other backslashes in quoted values', () => {
let nags: Nag<VCardNagAttributes>[] = [];
expect(
parseParameters('x;TYPE="a\\,b;c:d\\":x', 1, nags, 'X'),
).toStrictEqual({
parameters: { TYPE: ['a,b;c:d'] },
parameters: { TYPE: ['a\\,b;c:d\\'] },
end: 18,
});
expect(nags).toStrictEqual([]);
expect(nags).toStrictEqual([
{
attributes: {
line: 'x;TYPE="a\\,b;c:d\\":x',
parameter: 'TYPE',
property: 'X',
},
description: 'Backslash found in parameter value',
isError: false,
key: 'PARAM_BAD_BACKSLASH',
},
{
attributes: {
line: 'x;TYPE="a\\,b;c:d\\":x',
parameter: 'TYPE',
property: 'X',
},
description: 'Unescaped comma in parameter value',
isError: false,
key: 'PARAM_UNESCAPED_COMMA',
},
]);
});
it('should handle multiple quoted values', () => {
let nags: Nag<VCardNagAttributes>[] = [];
Expand Down
Loading

0 comments on commit d6e4f72

Please sign in to comment.