Unverified Commit a926715d authored by Jonah Williams's avatar Jonah Williams Committed by GitHub

[flutter_tools] URI encode dart-define values (#57873)

parent ba847d54
......@@ -328,7 +328,7 @@ Future<void> buildGradleApp({
command.add('-Pshrink=true');
}
if (androidBuildInfo.buildInfo.dartDefines?.isNotEmpty ?? false) {
command.add('-Pdart-defines=${androidBuildInfo.buildInfo.dartDefines.join(',')}');
command.add('-Pdart-defines=${encodeDartDefines(androidBuildInfo.buildInfo.dartDefines)}');
}
if (shouldBuildPluginAsAar) {
// Pass a system flag instead of a project flag, so this flag can be
......
......@@ -139,7 +139,7 @@ class BuildInfo {
Map<String, String> toEnvironmentConfig() {
return <String, String>{
if (dartDefines?.isNotEmpty ?? false)
'DART_DEFINES': dartDefines.join(','),
'DART_DEFINES': encodeDartDefines(dartDefines),
if (dartObfuscation != null)
'DART_OBFUSCATION': dartObfuscation.toString(),
if (extraFrontEndOptions?.isNotEmpty ?? false)
......@@ -655,3 +655,25 @@ String getWindowsBuildDirectory() {
String getFuchsiaBuildDirectory() {
return globals.fs.path.join(getBuildDirectory(), 'fuchsia');
}
/// Defines specified via the `--dart-define` command-line option.
///
/// These values are URI-encoded and then combined into a comma-separated string.
const String kDartDefines = 'DartDefines';
/// Encode a List of dart defines in a URI string.
String encodeDartDefines(List<String> defines) {
return defines.map(Uri.encodeComponent).join(',');
}
/// Dart defines are encoded inside [environmentDefines] as a comma-separated list.
List<String> decodeDartDefines(Map<String, String> environmentDefines) {
if (!environmentDefines.containsKey(kDartDefines) || environmentDefines[kDartDefines].isEmpty) {
return const <String>[];
}
return environmentDefines[kDartDefines]
.split(',')
.map<Object>(Uri.decodeComponent)
.cast<String>()
.toList();
}
......@@ -58,9 +58,6 @@ const String kFileSystemScheme = 'FileSystemScheme';
/// If provided, must be used along with [kFileSystemScheme].
const String kFileSystemRoots = 'FileSystemRoots';
/// Defines specified via the `--dart-define` command-line option.
const String kDartDefines = 'DartDefines';
/// The define to control what iOS architectures are built for.
///
/// This is expected to be a comma-separated list of architectures. If not
......@@ -257,7 +254,7 @@ class KernelSnapshot extends Target {
extraFrontEndOptions: extraFrontEndOptions,
fileSystemRoots: fileSystemRoots,
fileSystemScheme: fileSystemScheme,
dartDefines: parseDartDefines(environment),
dartDefines: decodeDartDefines(environment.defines),
packageConfig: packageConfig,
);
if (output == null || output.errorCount != 0) {
......@@ -399,11 +396,3 @@ abstract class CopyFlutterAotBundle extends Target {
environment.buildDir.childFile('app.so').copySync(outputFile.path);
}
}
/// Dart defines are encoded inside [Environment] as a comma-separated list.
List<String> parseDartDefines(Environment environment) {
if (!environment.defines.containsKey(kDartDefines) || environment.defines[kDartDefines].isEmpty) {
return const <String>[];
}
return environment.defines[kDartDefines].split(',');
}
......@@ -164,7 +164,7 @@ class Dart2JSTarget extends Target {
final String packageFile = globalPackagesPath;
final File outputKernel = environment.buildDir.childFile('app.dill');
final File outputFile = environment.buildDir.childFile('main.dart.js');
final List<String> dartDefines = parseDartDefines(environment);
final List<String> dartDefines = decodeDartDefines(environment.defines);
final String enabledExperiments = environment.defines[kEnableExperiment];
// Run the dart2js compilation in two stages, so that icon tree shaking can
......
......@@ -139,7 +139,7 @@ Future<void> buildWithAssemble({
kTrackWidgetCreation: trackWidgetCreation?.toString(),
kIconTreeShakerFlag: treeShakeIcons ? 'true' : null,
if (dartDefines != null && dartDefines.isNotEmpty)
kDartDefines: dartDefines.join(','),
kDartDefines: encodeDartDefines(dartDefines),
},
artifacts: globals.artifacts,
fileSystem: globals.fs,
......
......@@ -6,6 +6,7 @@ import 'package:meta/meta.dart';
import '../base/common.dart';
import '../base/file_system.dart';
import '../build_info.dart';
import '../build_system/build_system.dart';
import '../build_system/depfile.dart';
import '../build_system/targets/android.dart';
......
......@@ -50,7 +50,7 @@ Future<void> buildWeb(
kTargetFile: target,
kInitializePlatform: initializePlatform.toString(),
kHasWebPlugins: hasWebPlugins.toString(),
kDartDefines: buildInfo.dartDefines.join(','),
kDartDefines: encodeDartDefines(buildInfo.dartDefines),
kCspMode: csp.toString(),
kIconTreeShakerFlag: buildInfo.treeShakeIcons.toString(),
if (experiments.isNotEmpty)
......
......@@ -354,7 +354,7 @@ void main() {
expect(configLines, containsAll(<String>[
'set(FLUTTER_ROOT "$_kTestFlutterRoot")',
'set(PROJECT_DIR "${fileSystem.currentDirectory.path}")',
' "DART_DEFINES=\\"foo.bar=2,fizz.far=3\\""',
' "DART_DEFINES=\\"foo.bar%3D2,fizz.far%3D3\\""',
' "DART_OBFUSCATION=\\"true\\""',
' "EXTRA_FRONT_END_OPTIONS=\\"--enable-experiment=non-nullable\\""',
' "EXTRA_GEN_SNAPSHOT_OPTIONS=\\"--enable-experiment=non-nullable\\""',
......
......@@ -293,7 +293,7 @@ void main() {
expect(props.findAllElements('TREE_SHAKE_ICONS').first.text, 'true');
expect(props.findAllElements('EXTRA_GEN_SNAPSHOT_OPTIONS').first.text, '--enable-experiment=non-nullable');
expect(props.findAllElements('EXTRA_FRONT_END_OPTIONS').first.text, '--enable-experiment=non-nullable');
expect(props.findAllElements('DART_DEFINES').first.text, 'foo=a,bar=b');
expect(props.findAllElements('DART_DEFINES').first.text, 'foo%3Da,bar%3Db');
expect(props.findAllElements('DART_OBFUSCATION').first.text, 'true');
expect(props.findAllElements('SPLIT_DEBUG_INFO').first.text, r'C:\foo\');
expect(props.findAllElements('FLUTTER_TARGET').first.text, r'lib\main.dart');
......
......@@ -110,11 +110,37 @@ void main() {
expect(buildInfo.toEnvironmentConfig(), <String, String>{
'TREE_SHAKE_ICONS': 'true',
'TRACK_WIDGET_CREATION': 'true',
'DART_DEFINES': 'foo=2,bar=2',
'DART_DEFINES': 'foo%3D2,bar%3D2',
'DART_OBFUSCATION': 'true',
'SPLIT_DEBUG_INFO': 'foo/',
'EXTRA_FRONT_END_OPTIONS': '--enable-experiment=non-nullable,bar',
'EXTRA_GEN_SNAPSHOT_OPTIONS': '--enable-experiment=non-nullable,fizz',
});
});
testWithoutContext('encodeDartDefines encodes define values with URI encode compnents', () {
expect(encodeDartDefines(<String>['"hello"']), '%22hello%22');
expect(encodeDartDefines(<String>['https://www.google.com']), 'https%3A%2F%2Fwww.google.com');
expect(encodeDartDefines(<String>['2,3,4', '5']), '2%2C3%2C4,5');
expect(encodeDartDefines(<String>['true', 'false', 'flase']), 'true,false,flase');
expect(encodeDartDefines(<String>['1232,456', '2']), '1232%2C456,2');
});
testWithoutContext('decodeDartDefines decodes URI encoded dart defines', () {
expect(decodeDartDefines(<String, String>{
kDartDefines: '%22hello%22'
}), <String>['"hello"']);
expect(decodeDartDefines(<String, String>{
kDartDefines: 'https%3A%2F%2Fwww.google.com'
}), <String>['https://www.google.com']);
expect(decodeDartDefines(<String, String>{
kDartDefines: '2%2C3%2C4,5'
}), <String>['2,3,4', '5']);
expect(decodeDartDefines(<String, String>{
kDartDefines: 'true,false,flase'
}), <String>['true', 'false', 'flase']);
expect(decodeDartDefines(<String, String>{
kDartDefines: '1232%2C456,2'
}), <String>['1232,456', '2']);
});
}
......@@ -6,6 +6,7 @@ import 'package:file_testing/file_testing.dart';
import 'package:flutter_tools/src/artifacts.dart';
import 'package:flutter_tools/src/base/file_system.dart';
import 'package:flutter_tools/src/base/platform.dart';
import 'package:flutter_tools/src/build_info.dart';
import 'package:flutter_tools/src/build_system/build_system.dart';
import 'package:flutter_tools/src/build_system/depfile.dart';
import 'package:flutter_tools/src/build_system/targets/dart.dart';
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment