Skip to content
This repository has been archived by the owner on Mar 13, 2023. It is now read-only.

Commit

Permalink
fix(commands): certain characters not (" and \) being parsed
Browse files Browse the repository at this point in the history
  • Loading branch information
moranje committed Feb 22, 2020
1 parent fee6e5f commit d53d68f
Show file tree
Hide file tree
Showing 4 changed files with 243 additions and 24 deletions.
17 changes: 7 additions & 10 deletions assets/info.plist
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@
<key>argumenttype</key>
<integer>1</integer>
<key>escaping</key>
<integer>36</integer>
<integer>102</integer>
<key>keyword</key>
<string>todos</string>
<key>queuedelaycustom</key>
Expand All @@ -119,8 +119,7 @@
. ./check-node.sh
escaped="$(echo "{query}" | sed 's/"/\\"/g')"
node $node_flags alfred-workflow-todoist.js "{\"name\": \"read\", \"args\": \"$escaped\"}"
node $node_flags alfred-workflow-todoist.js "{\"name\": \"read\", \"args\": \"{query}\"}"
</string>
<key>scriptargtype</key>
<integer>0</integer>
Expand Down Expand Up @@ -219,7 +218,7 @@ node $node_flags alfred-workflow-todoist.js "{query}"</string>
<key>argumenttype</key>
<integer>0</integer>
<key>escaping</key>
<integer>36</integer>
<integer>102</integer>
<key>keyword</key>
<string>todo</string>
<key>queuedelaycustom</key>
Expand All @@ -242,8 +241,7 @@ node $node_flags alfred-workflow-todoist.js "{query}"</string>
. ./check-node.sh
escaped="$(echo "{query}" | sed 's/"/\\"/g')"
node $node_flags alfred-workflow-todoist.js "{\"name\": \"parse\", \"args\": \"$escaped\"}"
node $node_flags alfred-workflow-todoist.js "{\"name\": \"parse\", \"args\": \"{query}\"}"
</string>
<key>scriptargtype</key>
<integer>0</integer>
Expand Down Expand Up @@ -294,7 +292,7 @@ node $node_flags alfred-workflow-todoist.js "{\"name\": \"parse\", \"args\": \"$
<key>argumenttype</key>
<integer>1</integer>
<key>escaping</key>
<integer>38</integer>
<integer>102</integer>
<key>keyword</key>
<string>todo:setting</string>
<key>queuedelaycustom</key>
Expand All @@ -317,8 +315,7 @@ node $node_flags alfred-workflow-todoist.js "{\"name\": \"parse\", \"args\": \"$
. ./check-node.sh
escaped="$(echo "{query}" | sed 's/"/\\"/g')"
node $node_flags alfred-workflow-todoist.js "{\"name\": \"readSettings\", \"args\": \"$escaped\"}"
node $node_flags alfred-workflow-todoist.js "{\"name\": \"readSettings\", \"args\": \"{query}\"}"
</string>
<key>scriptargtype</key>
<integer>0</integer>
Expand Down Expand Up @@ -412,7 +409,7 @@ node $node_flags alfred-workflow-todoist.js "{\"name\": \"readSettings\", \"args
<string></string>
</dict>
<key>version</key>
<string>0.0.0-development</string>
<string>6.0.0-alpha.8</string>
<key>webaddress</key>
<string>https://github.com/moranje/alfred-workflow-todoist</string>
</dict>
Expand Down
1 change: 0 additions & 1 deletion jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ module.exports = {
'/dist/',
'/tools/',
'alfred-workflow-todoist.ts',
'cli-args',
],

// A list of reporter names that Jest uses when writing coverage reports
Expand Down
73 changes: 60 additions & 13 deletions src/lib/cli-args.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { isPrimitive } from 'util';
import { TodoistTask, TodoistTaskOptions } from 'todoist-rest-api';
import { isPrimitive } from 'util';

import { ResourceName } from './todoist/local-rest-adapter';
import { Settings } from '@/lib/stores/settings-store';

import { ResourceName } from './todoist/local-rest-adapter';

type Arg =
| string
| TodoistTaskOptions
Expand Down Expand Up @@ -39,6 +40,16 @@ export type Call =
};

const [, , ...argv] = process.argv;
export const callNames: { [key: string]: boolean } = {
parse: true,
read: true,
readSettings: true,
openUrl: true,
create: true,
remove: true,
writeSetting: true,
refreshCache: true,
};

function assertValidArgs(args: Arg): asserts args is Arg {
if (args == null) {
Expand All @@ -49,13 +60,22 @@ function assertValidArgs(args: Arg): asserts args is Arg {
}

function assertValidCall(call: Call): asserts call is Call {
// istanbul ignore next: shouldn't be possible in codebase
if (!call || isPrimitive(call)) {
throw new TypeError(`The call should be a an object, was ${call}`);
}

if (!call.name || typeof call.name !== 'string') {
throw new TypeError(
`Expected call.name to be a string was ${call.name} (${typeof call.name})`
`Expected call.name to be a string was ${
call.name
} (of type ${typeof call.name})`
);
}

if (!callNames[call.name.toString()]) {
throw new TypeError(
`Expected call.name to be one of parse, read, readSettings, openUrl, create, remove, writeSetting, refreshCache, was ${call.name}`
);
}

Expand All @@ -68,22 +88,49 @@ function serialize(call: Call): string | never {
return JSON.stringify(call);
}

function escape(string: string): string {
// @ts-ignore: is valid
return String(string).replace(/["\\\b\f\n\r\t]/g, char => {
switch (char) {
case '"':
return '\\"';
case '\b':
return '\\b';
case '\f':
return '\\f';
case '\n':
return '\\n';
case '\r':
return '\\r';
case '\t':
return '\\t';
default:
return char;
}
});
}

function deserialize(serialized: string): Call | never {
if (typeof serialized !== 'string') {
throw new TypeError(
`Expected a string in deserialize, got ${serialized} (${typeof serialized})`
);
}
const escaped = serialized.replace(
/"args": "([\s\S]+?)"}/,
(match, input) => {
return `"args": "${escape(input)}"}`;
}
);

try {
const call = JSON.parse(serialized) as Call;
const call = JSON.parse(escaped) as Call;
assertValidCall(call);

return call;
} catch (error) {
throw new TypeError(
`Expected a JSON string, got '${serialized}' (${typeof serialized})`
);
if (error instanceof SyntaxError) {
throw new TypeError(
`Expected a JSON string, got '${escaped}' (${typeof escaped})`
);
}

throw new TypeError(error.message);
}
}

Expand Down Expand Up @@ -116,7 +163,7 @@ export function isUserFacingCall(): boolean {
let call;
try {
call = getCurrentCall();
} catch {
} catch (error) {
// Err on the side of caution.
return true;
}
Expand Down
176 changes: 176 additions & 0 deletions src/lib/tests/cli-args.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
import mockArgv from 'mock-argv';
// import { createCall } from '@/lib/cli-args';

type CallName =
| 'parse'
| 'read'
| 'readSettings'
| 'openUrl'
| 'create'
| 'remove'
| 'writeSetting'
| 'refreshCache'
| 'anything'; // for testing purposes

function escape(string: string): string {
return String(string).replace(/[\\]/g, function(char) {
switch (char) {
// case '"':
// return '\\"';
case '\\':
return '\\' + char;
}
});
}

function cliArgs(callName: CallName, input: any): string {
return `{"name": "${callName}", "args": "${escape(input.toString())}"}`;
}

describe('unit: CLI', () => {
beforeEach(() => jest.resetModules());

it('should escape characters invalid to JSON during deserialization', async () => {
expect.assertions(1);

const characters = '\\ " \t \n \r \b \f';
await mockArgv([cliArgs('read', characters)], async () => {
const { getCurrentCall } = await import('@/lib/cli-args');

const cli = getCurrentCall();

expect(cli.args).toBe(characters);
});
});

it('should throw if call is not a valid JSON string', async () => {
expect.assertions(1);

await mockArgv([{}], async () => {
const { getCurrentCall } = await import('@/lib/cli-args');

expect(() => {
const cli = getCurrentCall();
}).toThrow(/Expected a JSON string/);
});
});

it("should throw if call doesn't have a name property", async () => {
expect.assertions(1);

await mockArgv(['{}'], async () => {
const { getCurrentCall } = await import('@/lib/cli-args');

expect(() => {
const cli = getCurrentCall();
}).toThrow(/Expected call.name to be a string/);
});
});

it("should throw if call doesn't have an args property", async () => {
expect.assertions(1);

await mockArgv(['{"name": "read"}'], async () => {
const { getCurrentCall } = await import('@/lib/cli-args');

expect(() => {
const cli = getCurrentCall();
}).toThrow(/Property args should not be null or undefined/);
});
});

it('should serialize to a JSON parsable string', async () => {
expect.assertions(1);

const { createCall } = await import('@/lib/cli-args');

const characters = '\\ " \t \n \r \b \f';
const call = createCall({
name: 'create',
args: characters,
});

expect(JSON.parse(call).args).toBe(characters);
});

it('should label "parse" as a user facing call', async () => {
expect.assertions(1);

await mockArgv([cliArgs('parse', '')], async () => {
const { isUserFacingCall } = await import('@/lib/cli-args');
expect(isUserFacingCall()).toBe(true);
});
});

it('should label "read" as a user facing call', async () => {
expect.assertions(1);

await mockArgv([cliArgs('read', '')], async () => {
const { isUserFacingCall } = await import('@/lib/cli-args');
expect(isUserFacingCall()).toBe(true);
});
});

it('should label "readSettings" as a user facing call', async () => {
expect.assertions(1);

await mockArgv([cliArgs('readSettings', '')], async () => {
const { isUserFacingCall } = await import('@/lib/cli-args');
expect(isUserFacingCall()).toBe(true);
});
});

it('should label "openUrl" as non-user facing call', async () => {
expect.assertions(1);

await mockArgv([cliArgs('openUrl', '')], async () => {
const { isUserFacingCall } = await import('@/lib/cli-args');
expect(isUserFacingCall()).toBe(false);
});
});

it('should label "create" as non-user facing call', async () => {
expect.assertions(1);

await mockArgv([cliArgs('create', '')], async () => {
const { isUserFacingCall } = await import('@/lib/cli-args');
expect(isUserFacingCall()).toBe(false);
});
});

it('should label "remove" as non-user facing call', async () => {
expect.assertions(1);

await mockArgv([cliArgs('remove', '')], async () => {
const { isUserFacingCall } = await import('@/lib/cli-args');
expect(isUserFacingCall()).toBe(false);
});
});

it('should label "writeSetting" as non-user facing call', async () => {
expect.assertions(1);

await mockArgv([cliArgs('writeSetting', '')], async () => {
const { isUserFacingCall } = await import('@/lib/cli-args');
expect(isUserFacingCall()).toBe(false);
});
});

it('should label "refreshCache" as non-user facing call', async () => {
expect.assertions(1);

await mockArgv([cliArgs('refreshCache', '')], async () => {
const { isUserFacingCall } = await import('@/lib/cli-args');
expect(isUserFacingCall()).toBe(false);
});
});

it('should label unknown calls as user facing', async () => {
expect.assertions(1);

await mockArgv([cliArgs('anything', '')], async () => {
const { isUserFacingCall } = await import('@/lib/cli-args');
expect(isUserFacingCall()).toBe(true);
});
});
});

0 comments on commit d53d68f

Please sign in to comment.