Skip to content

Better escaping of String literals #126

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Apr 3, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions json_serializable/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
* Throw an exception if a private field or an ignored field is referenced by a
required constructor argument.

* More comprehensive escaping of string literals.

### `package:json_serializable/type_helper.dart`

* **Breaking** The `nullable` parameter on `TypeHelper.serialize` and
Expand Down
100 changes: 73 additions & 27 deletions json_serializable/lib/src/utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -13,39 +13,85 @@ import 'package:source_gen/source_gen.dart';

/// Returns a quoted String literal for [value] that can be used in generated
/// Dart code.
// TODO: still need handle triple singe/double quotes within `value`
String escapeDartString(String value) {
if (value.contains('\n')) {
return "r'''\n$value'''";
var hasSingleQuote = false;
var hasDoubleQuote = false;
var hasDollar = false;
var canBeRaw = true;

value = value.replaceAllMapped(_escapeRegExp, (match) {
var value = match[0];
if (value == "'") {
hasSingleQuote = true;
return value;
} else if (value == '"') {
hasDoubleQuote = true;
return value;
} else if (value == r'$') {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could treat \ the same as $ wrt. "canBeRaw", a string with content "$\ can be raw as r'"$\'.

I can see that it doesn't work with the way this operation currently works - you can't decide later that the string can't be raw, because then you have to go back and fix previous \ characters, so I guess this is a good enough trade-off since backslashes are rare.

The alternative is to scan the string once first to figure out what it contains, and then fix it up in one second operation.

hasDollar = true;
return value;
}

canBeRaw = false;
return _escapeMap[value] ?? _getHexLiteral(value);
});

if (!hasDollar) {
if (hasSingleQuote) {
if (!hasDoubleQuote) {
return '"$value"';
}
// something
} else {
// trivial!
return "'$value'";
}
}

var containsDollar = value.contains(r'$');

if (value.contains("'")) {
if (value.contains('"')) {
// `value` contains both single and double quotes.
// The only safe way to wrap the content is to escape all of the
// problematic characters.
var string = value
.replaceAll(r'$', r'\$')
.replaceAll('"', r'\"')
.replaceAll("'", r"\'");
return "'$string'";
} else if (containsDollar) {
// `value` contains "'" and "$", but not '"'.
// Safely wrap it in a raw string within double-quotes.
return 'r"$value"';
if (hasDollar && canBeRaw) {
if (hasSingleQuote) {
if (!hasDoubleQuote) {
// quote it with single quotes!
return 'r"$value"';
}
} else {
// quote it with single quotes!
return "r'$value'";
}
return '"$value"';
} else if (containsDollar) {
// `value` contains "$", but no "'"
// wrap it in a raw string using single quotes
return "r'$value'";
}

// `value` contains no problematic characters - except for '"' maybe.
// Wrap it in standard single-quotes.
return "'$value'";
// The only safe way to wrap the content is to escape all of the
// problematic characters - `$`, `'`, and `"`
var string = value.replaceAll(_dollarQuoteRegexp, r'\');
return "'$string'";
}

final _dollarQuoteRegexp = new RegExp(r"""(?=[$'"])""");

/// A [Map] between whitespace characters & `\` and their escape sequences.
const _escapeMap = const {
'\b': r'\b', // 08 - backspace
'\t': r'\t', // 09 - tab
'\n': r'\n', // 0A - new line
'\v': r'\v', // 0B - vertical tab
'\f': r'\f', // 0C - form feed
'\r': r'\r', // 0D - carriage return
'\x7F': r'\x7F', // delete
r'\': r'\\' // backslash
};

final _escapeMapRegexp = _escapeMap.keys.map(_getHexLiteral).join();

/// A [RegExp] that matches whitespace characters that should be escaped and
/// single-quote, double-quote, and `$`
final _escapeRegExp =
new RegExp('[\$\'"\\x00-\\x07\\x0E-\\x1F$_escapeMapRegexp]');

/// Given single-character string, return the hex-escaped equivalent.
String _getHexLiteral(String input) {
var rune = input.runes.single;
var value = rune.toRadixString(16).toUpperCase().padLeft(2, '0');
return '\\x$value';
}

String commonNullPrefix(
Expand Down
4 changes: 2 additions & 2 deletions json_serializable/test/json_literal_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@ import 'test_utils.dart';
main() {
test('literal round-trip', () {
var dataFilePath =
p.join(getPackagePath(), 'test', 'test_files', 'data.json');
p.join(getPackagePath(), 'test', 'test_files', 'json_literal.json');
var dataFile = new File(dataFilePath);

var dataString = loudEncode(json.decode(dataFile.readAsStringSync()));
// FYI: nice to re-write the test data when it's changed to keep it pretty
// ... but not a good idea to ship this
// dataFile.writeAsStringSync(dataString);
// dataFile.writeAsStringSync(dataString.replaceAll('\u007F', '\\u007F'));
var dartString = loudEncode(data);

expect(dartString, dataString);
Expand Down
4 changes: 2 additions & 2 deletions json_serializable/test/test_files/json_literal.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
import 'package:json_annotation/json_annotation.dart';
part 'json_literal.g.dart';

@JsonLiteral('data.json')
@JsonLiteral('json_literal.json')
List get data => _$dataJsonLiteral;

@JsonLiteral('data.json', asConst: true)
@JsonLiteral('json_literal.json', asConst: true)
List get asConst => _$asConstJsonLiteral;
38 changes: 30 additions & 8 deletions json_serializable/test/test_files/json_literal.g.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,15 @@ part of 'json_literal.dart';
// **************************************************************************

final _$dataJsonLiteral = [
r'''

new lines are fun!
''',
{
'backspace': '\b',
'tab': '\t',
'new line': '\n',
'vertical tab': '\v',
'form feed': '\r',
'carriage return': '\r',
'delete': '\x7F'
},
'simple string',
"'string with single quotes'",
'"string with double quotes"',
Expand All @@ -25,6 +30,12 @@ new lines are fun!
'""hello""',
r'""$double quotes and dollar signs""',
'\$scary with \'single quotes\' and triple-doubles \"\"\"oh no!',
'Dollar signs: \$ vs \\\$ vs \\\\\$',
'Slashes \\nice slash\\',
'slashes \\ and dollars \$ with white \n space',
"'''triple quoted strings should be\nfine!'''",
'"""as with triple-double-quotes"""',
'\"\"\"as with triple-double-quotes even when \'mixed\'\"\"\"',
null,
true,
false,
Expand All @@ -42,10 +53,15 @@ new lines are fun!
}
];
const _$asConstJsonLiteral = const [
r'''

new lines are fun!
''',
const {
'backspace': '\b',
'tab': '\t',
'new line': '\n',
'vertical tab': '\v',
'form feed': '\r',
'carriage return': '\r',
'delete': '\x7F'
},
'simple string',
"'string with single quotes'",
'"string with double quotes"',
Expand All @@ -56,6 +72,12 @@ new lines are fun!
'""hello""',
r'""$double quotes and dollar signs""',
'\$scary with \'single quotes\' and triple-doubles \"\"\"oh no!',
'Dollar signs: \$ vs \\\$ vs \\\\\$',
'Slashes \\nice slash\\',
'slashes \\ and dollars \$ with white \n space',
"'''triple quoted strings should be\nfine!'''",
'"""as with triple-double-quotes"""',
'\"\"\"as with triple-double-quotes even when \'mixed\'\"\"\"',
null,
true,
false,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
[
"\nnew lines are fun!\n",
{
"backspace": "\b",
"tab": "\t",
"new line": "\n",
"vertical tab": "\u000b",
"form feed": "\r",
"carriage return": "\r",
"delete": "\u007F"
},
"simple string",
"'string with single quotes'",
"\"string with double quotes\"",
Expand All @@ -10,6 +18,12 @@
"\"\"hello\"\"",
"\"\"$double quotes and dollar signs\"\"",
"$scary with 'single quotes' and triple-doubles \"\"\"oh no!",
"Dollar signs: $ vs \\$ vs \\\\$",
"Slashes \\nice slash\\",
"slashes \\ and dollars $ with white \n space",
"'''triple quoted strings should be\nfine!'''",
"\"\"\"as with triple-double-quotes\"\"\"",
"\"\"\"as with triple-double-quotes even when 'mixed'\"\"\"",
null,
true,
false,
Expand Down