Skip to content
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.

Sign up for GitHub

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

Improve yaml serialization quoting #4217

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
+93 −62
Draft

Improve yaml serialization quoting #4217

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 1 addition & 1 deletion lib/src/source/path.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ class ResolvedPathDescription extends ResolvedDescription {
'relative': true,
};
}
return {'path': description.path, 'relative': p.relative('false')};
return {'path': description.path, 'relative': false};
}

@override
Expand Down
30 changes: 23 additions & 7 deletions lib/src/utils.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -504,11 +504,27 @@ String prefixLines(String text, {String prefix = '| ', String? firstPrefix}) {
return lines.join('\n');
}

/// The subset of strings that don't need quoting in YAML.
///
/// This pattern does not strictly follow the plain scalar grammar of YAML,
/// which means some strings may be unnecessarily quoted, but it's much simpler.
final _unquotableYamlString = RegExp(r'^[a-zA-Z_-][a-zA-Z_0-9-]*$');
/// Does this string need quoting in yaml?
///
/// This mechanism does not strictly follow the plain scalar grammar of YAML,
/// which means some strings may be unnecessarily quoted.
bool _needsYamlQuotes(String s) {
// These must be quoted
if ([
Copy link
Member

Choose a reason for hiding this comment

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

List can be const. (Small save, but save, over creating a new list on every invocation.)

'true', 'True', 'TRUE', //
'false', 'False', 'FALSE',
'null', 'Null', 'NULL',
].contains(s)) return true;
// Numbers must be quoted
if (RegExp(r'^[-+]?[0-9]*\.?[0-9]*(e[0-9]+)?$').hasMatch(s)) return true;
Copy link
Member

Choose a reason for hiding this comment

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

This is a potentially dangerous RegExp because of the \d*\.?\d* sequence. It's probably not exponential, but it can be quadratic in the length to reject something like 00000000000000000000000....0000000x

Consider changing to \d*(\.\d*)? (And be aware that it still matches . alone.)

// Hex numbers must be quoted
if (RegExp(r'^0x[0-9]+$').hasMatch(s)) return true;
Copy link
Member

Choose a reason for hiding this comment

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

Should probably be [0-9a-fA-F].


// Single non-number words containing a few special cases are ok.
if (RegExp(r'^[\.a-zA-Z_0-9-]+$').hasMatch(s)) return false;
// We are unsure, better safe than sorry
return true;
}

/// Converts [data], which is a parsed YAML object, to a pretty-printed string,
/// using indentation for maps.
Expand Down Expand Up @@ -536,7 +552,7 @@ String yamlToString(Object? data) {
first = false;

var keyString = key;
if (key is! String || !_unquotableYamlString.hasMatch(key)) {
if (key is! String || _needsYamlQuotes(key)) {
keyString = jsonEncode(key);
}

Expand All @@ -552,7 +568,7 @@ String yamlToString(Object? data) {
var string = data;

// Don't quote plain strings if not needed.
if (data is! String || !_unquotableYamlString.hasMatch(data)) {
if (data is! String || _needsYamlQuotes(data)) {
string = jsonEncode(data);
}

Expand Down
6 changes: 3 additions & 3 deletions test/testdata/goldens/dependency_services/dependency_services_test/Adding transitive.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ packages:
sha256: $SHA256
url: "http://localhost:$PORT"
source: hosted
version: "1.2.3"
version: 1.2.3
sdks:
dart: ">=3.0.2 <4.0.0"
-------------------------------- END OF OUTPUT ---------------------------------
Expand Down Expand Up @@ -179,14 +179,14 @@ packages:
sha256: $SHA256
url: "http://localhost:$PORT"
source: hosted
version: "2.2.3"
version: 2.2.3
transitive:
dependency: transitive
description:
name: transitive
sha256: $SHA256
url: "http://localhost:$PORT"
source: hosted
version: "1.0.0"
version: 1.0.0
sdks:
dart: ">=3.0.2 <4.0.0"
16 changes: 8 additions & 8 deletions ...estdata/goldens/dependency_services/dependency_services_test/Can update a git package.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,19 @@ packages:
description:
path: "."
ref: HEAD
resolved-ref: "1ea665d6de007c60308949db4c6ae8049c2395f0"
resolved-ref: 1ea665d6de007c60308949db4c6ae8049c2395f0
url: "../bar.git"
source: git
version: "1.0.0"
version: 1.0.0
foo:
dependency: "direct main"
description:
path: "."
ref: HEAD
resolved-ref: "9e8b8ad5091bec2c4724b1e0c8ff9d6e32a7eaa4"
resolved-ref: 9e8b8ad5091bec2c4724b1e0c8ff9d6e32a7eaa4
url: "../foo.git"
source: git
version: "1.0.0"
version: 1.0.0
sdks:
dart: ">=3.0.2 <4.0.0"
-------------------------------- END OF OUTPUT ---------------------------------
Expand Down Expand Up @@ -218,18 +218,18 @@ packages:
description:
path: "."
ref: HEAD
resolved-ref: "1ea665d6de007c60308949db4c6ae8049c2395f0"
resolved-ref: 1ea665d6de007c60308949db4c6ae8049c2395f0
url: "../bar.git"
source: git
version: "1.0.0"
version: 1.0.0
foo:
dependency: "direct main"
description:
path: "."
ref: HEAD
resolved-ref: "385bb2860e7ebf9519993b2548d6cab74a6008ef"
resolved-ref: 385bb2860e7ebf9519993b2548d6cab74a6008ef
url: "../foo.git"
source: git
version: "2.0.0"
version: 2.0.0
sdks:
dart: ">=3.0.2 <4.0.0"
12 changes: 6 additions & 6 deletions test/testdata/goldens/dependency_services/dependency_services_test/Compatible.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,23 +13,23 @@ packages:
sha256: $SHA256
url: "http://localhost:$PORT"
source: hosted
version: "1.2.3"
version: 1.2.3
boo:
dependency: "direct main"
description:
name: boo
sha256: $SHA256
url: "http://localhost:$PORT"
source: hosted
version: "1.2.3"
version: 1.2.3
foo:
dependency: "direct main"
description:
name: foo
sha256: $SHA256
url: "http://localhost:$PORT"
source: hosted
version: "1.2.3"
version: 1.2.3
sdks:
dart: ">=3.0.2 <4.0.0"
-------------------------------- END OF OUTPUT ---------------------------------
Expand Down Expand Up @@ -383,22 +383,22 @@ packages:
sha256: $SHA256
url: "http://localhost:$PORT"
source: hosted
version: "1.2.3"
version: 1.2.3
boo:
dependency: "direct main"
description:
name: boo
sha256: $SHA256
url: "http://localhost:$PORT"
source: hosted
version: "1.2.3"
version: 1.2.3
foo:
dependency: "direct main"
description:
name: foo
sha256: $SHA256
url: "http://localhost:$PORT"
source: hosted
version: "1.2.4"
version: 1.2.4
sdks:
dart: ">=3.0.2 <4.0.0"
4 changes: 2 additions & 2 deletions .../goldens/dependency_services/dependency_services_test/Finds smallest possible upgrade.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ packages:
sha256: $SHA256
url: "http://localhost:$PORT"
source: hosted
version: "1.1.1"
version: 1.1.1
sdks:
dart: ">=3.0.2 <4.0.0"
-------------------------------- END OF OUTPUT ---------------------------------
Expand Down Expand Up @@ -167,6 +167,6 @@ packages:
sha256: $SHA256
url: "http://localhost:$PORT"
source: hosted
version: "1.1.1"
version: 1.1.1
sdks:
dart: ">=3.0.2 <4.0.0"
4 changes: 2 additions & 2 deletions test/testdata/goldens/dependency_services/dependency_services_test/Ignoring version.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ packages:
sha256: $SHA256
url: "http://localhost:$PORT"
source: hosted
version: "1.0.0"
version: 1.0.0
sdks:
dart: ">=3.0.2 <4.0.0"
-------------------------------- END OF OUTPUT ---------------------------------
Expand Down Expand Up @@ -112,6 +112,6 @@ packages:
sha256: $SHA256
url: "http://localhost:$PORT"
source: hosted
version: "1.0.0"
version: 1.0.0
sdks:
dart: ">=3.0.2 <4.0.0"
12 changes: 6 additions & 6 deletions ...data/goldens/dependency_services/dependency_services_test/Preserves no content-hashes.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,21 @@ packages:
name: bar
url: "http://localhost:$PORT"
source: hosted
version: "1.2.3"
version: 1.2.3
boo:
dependency: "direct main"
description:
name: boo
url: "http://localhost:$PORT"
source: hosted
version: "1.2.3"
version: 1.2.3
foo:
dependency: "direct main"
description:
name: foo
url: "http://localhost:$PORT"
source: hosted
version: "1.2.3"
version: 1.2.3
sdks:
dart: ">=3.0.2 <4.0.0"
-------------------------------- END OF OUTPUT ---------------------------------
Expand Down Expand Up @@ -365,20 +365,20 @@ packages:
name: bar
url: "http://localhost:$PORT"
source: hosted
version: "1.2.3"
version: 1.2.3
boo:
dependency: "direct main"
description:
name: boo
url: "http://localhost:$PORT"
source: hosted
version: "1.2.3"
version: 1.2.3
foo:
dependency: "direct main"
description:
name: foo
url: "http://localhost:$PORT"
source: hosted
version: "1.2.4"
version: 1.2.4
sdks:
dart: ">=3.0.2 <4.0.0"
8 changes: 4 additions & 4 deletions ...dependency_services/dependency_services_test/Preserves pub.dartlang.org as hosted url.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,15 @@ packages:
sha256: $SHA256
url: https://pub.dartlang.org
source: hosted
version: "1.2.3"
version: 1.2.3
foo:
dependency: "direct main"
description:
name: foo
sha256: $SHA256
url: https://pub.dartlang.org
source: hosted
version: "1.2.3"
version: 1.2.3
sdks:
dart: ">=3.0.2 <4.0.0"
-------------------------------- END OF OUTPUT ---------------------------------
Expand Down Expand Up @@ -339,14 +339,14 @@ packages:
sha256: $SHA256
url: "https://pub.dartlang.org"
source: hosted
version: "1.2.3"
version: 1.2.3
foo:
dependency: "direct main"
description:
name: foo
sha256: $SHA256
url: "https://pub.dartlang.org"
source: hosted
version: "1.2.4"
version: 1.2.4
sdks:
dart: ">=3.0.2 <4.0.0"
8 changes: 4 additions & 4 deletions ...tdata/goldens/dependency_services/dependency_services_test/Relative paths are allowed.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,15 @@ packages:
path: "../bar"
relative: true
source: path
version: "1.0.0"
version: 1.0.0
foo:
dependency: "direct main"
description:
name: foo
sha256: $SHA256
url: "http://localhost:$PORT"
source: hosted
version: "1.0.0"
version: 1.0.0
sdks:
dart: ">=3.0.2 <4.0.0"
-------------------------------- END OF OUTPUT ---------------------------------
Expand Down Expand Up @@ -177,14 +177,14 @@ packages:
path: "../bar"
relative: true
source: path
version: "1.0.0"
version: 1.0.0
foo:
dependency: "direct main"
description:
name: foo
sha256: $SHA256
url: "http://localhost:$PORT"
source: hosted
version: "2.0.0"
version: 2.0.0
sdks:
dart: ">=3.0.2 <4.0.0"
6 changes: 3 additions & 3 deletions test/testdata/goldens/dependency_services/dependency_services_test/Removing transitive.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,15 @@ packages:
sha256: $SHA256
url: "http://localhost:$PORT"
source: hosted
version: "1.2.3"
version: 1.2.3
transitive:
dependency: transitive
description:
name: transitive
sha256: $SHA256
url: "http://localhost:$PORT"
source: hosted
version: "1.0.0"
version: 1.0.0
sdks:
dart: ">=3.0.2 <4.0.0"
-------------------------------- END OF OUTPUT ---------------------------------
Expand Down Expand Up @@ -217,6 +217,6 @@ packages:
sha256: $SHA256
url: "http://localhost:$PORT"
source: hosted
version: "2.2.3"
version: 2.2.3
sdks:
dart: ">=3.0.2 <4.0.0"
4 changes: 2 additions & 2 deletions ...rvices/dependency_services_test/Smallest possible upgrade can upgrade beyond breaking.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ packages:
sha256: $SHA256
url: "http://localhost:$PORT"
source: hosted
version: "1.1.1"
version: 1.1.1
sdks:
dart: ">=3.0.2 <4.0.0"
-------------------------------- END OF OUTPUT ---------------------------------
Expand Down Expand Up @@ -140,6 +140,6 @@ packages:
sha256: $SHA256
url: "http://localhost:$PORT"
source: hosted
version: "1.1.1"
version: 1.1.1
sdks:
dart: ">=3.0.2 <4.0.0"
Loading
Loading