Skip to content

Commit 7c145e7

Browse files
committed
Looking at test coverage suggested some more tests.
I hadn't tested comment-based concatenation hazards and there was a non-operational branch in the interpolation loop.
1 parent ee421e6 commit 7c145e7

File tree

4 files changed

+27
-12
lines changed

4 files changed

+27
-12
lines changed

lib/es6/Lexer.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,11 @@ const PREFIX_BEFORE_DELIMITER = new RegExp(
2525
// Comment
2626
// https://dev.mysql.com/doc/refman/5.7/en/comments.html
2727
// https://dev.mysql.com/doc/refman/5.7/en/ansi-diff-comments.html
28-
'--(?=' + WS + ')[^\\r\\n]*' +
29-
'|#[^\\r\\n]*' +
28+
// If we do not see a newline at the end of a comment, then it is
29+
// a concatenation hazard; a fragment concatened at the end would
30+
// start in a comment context.
31+
'--(?=' + WS + ')[^\\r\\n]*[\r\n]' +
32+
'|#[^\\r\\n]*[\r\n]' +
3033
'|/[*][\\s\\S]*?[*]/'
3134
) +
3235
'|' +

lib/es6/Template.js

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -66,14 +66,13 @@ function interpolateSqlIntoFragment (
6666
const chunk = chunks[i];
6767
// The count of values must be 1 less than the surrounding
6868
// chunks of literal text.
69-
if (i !== 0) {
70-
const delimiter = delimiters[i - 1];
71-
const value = values[i - 1];
72-
if (delimiter) {
73-
result += escapeDelimitedValue(value, delimiter, timeZone);
74-
} else {
75-
result += SqlString.escape(value, stringifyObjects, timeZone);
76-
}
69+
const delimiter = delimiters[i - 1];
70+
const value = values[i - 1];
71+
72+
if (delimiter) {
73+
result += escapeDelimitedValue(value, delimiter, timeZone);
74+
} else {
75+
result += SqlString.escape(value, stringifyObjects, timeZone);
7776
}
7877

7978
result += chunk;

test/unit/es6/Template.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,5 +124,19 @@ test('template tag', {
124124
runTagTest(
125125
"SELECT X FROM T WHERE X IN (123, 'foo', `foo`, 1 + 1)",
126126
() => sql`SELECT X FROM T WHERE X IN (${values})`);
127+
},
128+
'unclosed-sq': function () {
129+
assert.throws(() => sql`SELECT '${'foo'}`);
130+
},
131+
'unclosed-dq': function () {
132+
assert.throws(() => sql`SELECT "foo`);
133+
},
134+
'unclosed-bq': function () {
135+
assert.throws(() => sql`SELECT \`${'foo'}`);
136+
},
137+
'unclosed-comment': function () {
138+
// Ending in a comment is a concatenation hazard.
139+
// See comments in lib/es6/Lexer.js.
140+
assert.throws(() => sql`SELECT (${0}) -- comment`);
127141
}
128142
});

test/unit/test-SqlString.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -260,8 +260,7 @@ test('SqlString.format', {
260260
'double quest marks passes pre-escaped id': function () {
261261
var sql = SqlString.format(
262262
'SELECT ?? FROM ?? WHERE id = ?',
263-
[SqlString.identifier('table.id'),
264-
SqlString.identifier('table'), 42]);
263+
[SqlString.identifier('table.id'), SqlString.identifier('table'), 42]);
265264
assert.equal(sql, 'SELECT `table`.`id` FROM `table` WHERE id = 42');
266265
},
267266

0 commit comments

Comments
 (0)