[flutter_tools] check for permission issues when copying file (#63540)
diff --git a/packages/flutter_tools/lib/src/base/error_handling_file_system.dart b/packages/flutter_tools/lib/src/base/error_handling_file_system.dart index c039adc..2b29c74 100644 --- a/packages/flutter_tools/lib/src/base/error_handling_file_system.dart +++ b/packages/flutter_tools/lib/src/base/error_handling_file_system.dart
@@ -195,6 +195,17 @@ } @override + RandomAccessFile openSync({FileMode mode = FileMode.read}) { + return _runSync<RandomAccessFile>( + () => delegate.openSync( + mode: mode, + ), + platform: _platform, + failureMessage: 'Flutter failed to open a file at "${delegate.path}"', + ); + } + + @override String toString() => delegate.toString(); } @@ -385,9 +396,17 @@ // https://docs.microsoft.com/en-us/windows/win32/debug/system-error-codes const int kDeviceFull = 112; const int kUserMappedSectionOpened = 1224; + const int kAccessDenied = 5; final int errorCode = e.osError?.errorCode ?? 0; // Catch errors and bail when: switch (errorCode) { + case kAccessDenied: + throwToolExit( + '$message. The flutter tool cannot access the file.\n' + 'Please ensure that the SDK and/or project is installed in a location ' + 'that has read/write permissions for the current user.' + ); + break; case kDeviceFull: throwToolExit( '$message. The target device is full.'
diff --git a/packages/flutter_tools/lib/src/commands/create.dart b/packages/flutter_tools/lib/src/commands/create.dart index e81d3d4..ab9d31c 100644 --- a/packages/flutter_tools/lib/src/commands/create.dart +++ b/packages/flutter_tools/lib/src/commands/create.dart
@@ -32,18 +32,19 @@ import '../template.dart'; const List<String> _kAvailablePlatforms = <String>[ - 'ios', - 'android', - 'windows', - 'linux', - 'macos', - 'web', - ]; + 'ios', + 'android', + 'windows', + 'linux', + 'macos', + 'web', +]; const String _kNoPlatformsErrorMessage = ''' The plugin project was generated without specifying the `--platforms` flag, no new platforms are added. To add platforms, run `flutter create -t plugin --platforms <platforms> .` under the same -directory. You can also find detailed instructions on how to add platforms in the `pubspec.yaml` at https://flutter.dev/docs/development/packages-and-plugins/developing-packages#plugin-platforms. +directory. You can also find detailed instructions on how to add platforms in the `pubspec.yaml` +at https://flutter.dev/docs/development/packages-and-plugins/developing-packages#plugin-platforms. '''; class CreateCommand extends FlutterCommand { @@ -790,7 +791,10 @@ final Template template = await Template.fromName( templateName, fileSystem: globals.fs, + logger: globals.logger, + templateRenderer: globals.templateRenderer, templateManifest: templateManifest, + pub: pub, ); return template.render(directory, context, overwriteExisting: overwrite); }
diff --git a/packages/flutter_tools/lib/src/commands/ide_config.dart b/packages/flutter_tools/lib/src/commands/ide_config.dart index af891d6..a5844fe 100644 --- a/packages/flutter_tools/lib/src/commands/ide_config.dart +++ b/packages/flutter_tools/lib/src/commands/ide_config.dart
@@ -254,6 +254,8 @@ null, fileSystem: globals.fs, templateManifest: null, + logger: globals.logger, + templateRenderer: globals.templateRenderer, ); return template.render( globals.fs.directory(dirPath),
diff --git a/packages/flutter_tools/lib/src/project.dart b/packages/flutter_tools/lib/src/project.dart index fad3e5c..c91dfa6 100644 --- a/packages/flutter_tools/lib/src/project.dart +++ b/packages/flutter_tools/lib/src/project.dart
@@ -15,6 +15,7 @@ import 'base/logger.dart'; import 'build_info.dart'; import 'bundle.dart' as bundle; +import 'dart/pub.dart'; import 'features.dart'; import 'flutter_manifest.dart'; import 'globals.dart' as globals; @@ -657,7 +658,14 @@ } Future<void> _overwriteFromTemplate(String path, Directory target) async { - final Template template = await Template.fromName(path, fileSystem: globals.fs, templateManifest: null); + final Template template = await Template.fromName( + path, + fileSystem: globals.fs, + templateManifest: null, + logger: globals.logger, + templateRenderer: globals.templateRenderer, + pub: pub, + ); template.render( target, <String, dynamic>{ @@ -810,7 +818,14 @@ } Future<void> _overwriteFromTemplate(String path, Directory target) async { - final Template template = await Template.fromName(path, fileSystem: globals.fs, templateManifest: null); + final Template template = await Template.fromName( + path, + fileSystem: globals.fs, + templateManifest: null, + logger: globals.logger, + templateRenderer: globals.templateRenderer, + pub: pub, + ); template.render( target, <String, dynamic>{
diff --git a/packages/flutter_tools/lib/src/template.dart b/packages/flutter_tools/lib/src/template.dart index 54d87fb..094321b 100644 --- a/packages/flutter_tools/lib/src/template.dart +++ b/packages/flutter_tools/lib/src/template.dart
@@ -8,10 +8,11 @@ import 'base/common.dart'; import 'base/file_system.dart'; +import 'base/logger.dart'; +import 'base/template.dart'; import 'cache.dart'; import 'dart/package_map.dart'; import 'dart/pub.dart'; -import 'globals.dart' as globals hide fs; /// Expands templates in a directory to a destination. All files that must /// undergo template expansion should end with the '.tmpl' extension. All files @@ -32,8 +33,12 @@ class Template { Template(Directory templateSource, Directory baseDir, this.imageSourceDir, { @required FileSystem fileSystem, + @required Logger logger, + @required TemplateRenderer templateRenderer, @required Set<Uri> templateManifest, }) : _fileSystem = fileSystem, + _logger = logger, + _templateRenderer = templateRenderer, _templateManifest = templateManifest { _templateFilePaths = <String, String>{}; @@ -42,14 +47,9 @@ } final List<FileSystemEntity> templateFiles = templateSource.listSync(recursive: true); - - for (final FileSystemEntity entity in templateFiles) { - if (entity is! File) { - // We are only interesting in template *file* URIs. - continue; - } + for (final FileSystemEntity entity in templateFiles.whereType<File>()) { if (_templateManifest != null && !_templateManifest.contains(Uri.file(entity.absolute.path))) { - globals.logger.printTrace('Skipping ${entity.absolute.path}, missing from the template manifest.'); + _logger.printTrace('Skipping ${entity.absolute.path}, missing from the template manifest.'); // Skip stale files in the flutter_tools directory. continue; } @@ -68,20 +68,28 @@ static Future<Template> fromName(String name, { @required FileSystem fileSystem, @required Set<Uri> templateManifest, + @required Logger logger, + @required TemplateRenderer templateRenderer, + @required Pub pub, }) async { // All named templates are placed in the 'templates' directory final Directory templateDir = _templateDirectoryInPackage(name, fileSystem); - final Directory imageDir = await _templateImageDirectory(name, fileSystem); + final Directory imageDir = await _templateImageDirectory(name, fileSystem, logger, pub); return Template( templateDir, templateDir, imageDir, fileSystem: fileSystem, + logger: logger, + templateRenderer: templateRenderer, templateManifest: templateManifest, ); } final FileSystem _fileSystem; + final Logger _logger; final Set<Uri> _templateManifest; + final TemplateRenderer _templateRenderer; + static const String templateExtension = '.tmpl'; static const String copyTemplateExtension = '.copy.tmpl'; static const String imageTemplateExtension = '.img.tmpl'; @@ -102,7 +110,7 @@ try { destination.createSync(recursive: true); } on FileSystemException catch (err) { - globals.printError(err.toString()); + _logger.printError(err.toString()); throwToolExit('Failed to flutter create at ${destination.path}.'); return 0; } @@ -198,22 +206,22 @@ if (overwriteExisting) { finalDestinationFile.deleteSync(recursive: true); if (printStatusWhenWriting) { - globals.printStatus(' $relativePathForLogging (overwritten)'); + _logger.printStatus(' $relativePathForLogging (overwritten)'); } } else { // The file exists but we cannot overwrite it, move on. if (printStatusWhenWriting) { - globals.printTrace(' $relativePathForLogging (existing - skipped)'); + _logger.printTrace(' $relativePathForLogging (existing - skipped)'); } return; } } else { if (printStatusWhenWriting) { - globals.printStatus(' $relativePathForLogging (created)'); + _logger.printStatus(' $relativePathForLogging (created)'); } } - fileCount++; + fileCount += 1; finalDestinationFile.createSync(recursive: true); final File sourceFile = _fileSystem.file(absoluteSourcePath); @@ -222,6 +230,7 @@ // not need mustache rendering but needs to be directly copied. if (sourceFile.path.endsWith(copyTemplateExtension)) { + _validateReadPermissions(sourceFile); sourceFile.copySync(finalDestinationFile.path); return; @@ -233,6 +242,7 @@ if (sourceFile.path.endsWith(imageTemplateExtension)) { final File imageSourceFile = _fileSystem.file(_fileSystem.path.join( imageSourceDir.path, relativeDestinationPath.replaceAll(imageTemplateExtension, ''))); + _validateReadPermissions(imageSourceFile); imageSourceFile.copySync(finalDestinationFile.path); return; @@ -242,8 +252,9 @@ // rendering via mustache. if (sourceFile.path.endsWith(templateExtension)) { + _validateReadPermissions(sourceFile); final String templateContents = sourceFile.readAsStringSync(); - final String renderedContents = globals.templateRenderer.renderString(templateContents, context); + final String renderedContents = _templateRenderer.renderString(templateContents, context); finalDestinationFile.writeAsStringSync(renderedContents); @@ -252,12 +263,20 @@ // Step 5: This file does not end in .tmpl but is in a directory that // does. Directly copy the file to the destination. - + _validateReadPermissions(sourceFile); sourceFile.copySync(finalDestinationFile.path); }); return fileCount; } + + /// Attempt open/close the file to ensure that read permissions are correct. + /// + /// If this fails with a certain error code, the [ErrorHandlingFileSystem] will + /// trigger a tool exit with a better message. + void _validateReadPermissions(File file) { + file.openSync().closeSync(); + } } Directory _templateDirectoryInPackage(String name, FileSystem fileSystem) { @@ -268,25 +287,25 @@ // Returns the directory containing the 'name' template directory in // flutter_template_images, to resolve image placeholder against. -Future<Directory> _templateImageDirectory(String name, FileSystem fileSystem) async { +Future<Directory> _templateImageDirectory(String name, FileSystem fileSystem, Logger logger, Pub pub) async { final String toolPackagePath = fileSystem.path.join( Cache.flutterRoot, 'packages', 'flutter_tools'); final String packageFilePath = fileSystem.path.join(toolPackagePath, kPackagesFileName); // Ensure that .packgaes is present. if (!fileSystem.file(packageFilePath).existsSync()) { - await _ensurePackageDependencies(toolPackagePath); + await _ensurePackageDependencies(toolPackagePath, pub); } PackageConfig packageConfig = await loadPackageConfigWithLogging( fileSystem.file(packageFilePath), - logger: globals.logger, + logger: logger, ); Uri imagePackageLibDir = packageConfig['flutter_template_images']?.packageUriRoot; // Ensure that the template image package is present. if (imagePackageLibDir == null || !fileSystem.directory(imagePackageLibDir).existsSync()) { - await _ensurePackageDependencies(toolPackagePath); + await _ensurePackageDependencies(toolPackagePath, pub); packageConfig = await loadPackageConfigWithLogging( fileSystem.file(packageFilePath), - logger: globals.logger, + logger: logger, ); imagePackageLibDir = packageConfig['flutter_template_images']?.packageUriRoot; } @@ -298,7 +317,7 @@ // Runs 'pub get' for the given path to ensure that .packages is created and // all dependencies are present. -Future<void> _ensurePackageDependencies(String packagePath) async { +Future<void> _ensurePackageDependencies(String packagePath, Pub pub) async { await pub.get( context: PubContext.pubGet, directory: packagePath,
diff --git a/packages/flutter_tools/test/general.shard/base/error_handling_file_system_test.dart b/packages/flutter_tools/test/general.shard/base/error_handling_file_system_test.dart index 47ab03f..8e8dbeb 100644 --- a/packages/flutter_tools/test/general.shard/base/error_handling_file_system_test.dart +++ b/packages/flutter_tools/test/general.shard/base/error_handling_file_system_test.dart
@@ -59,6 +59,9 @@ encoding: anyNamed('encoding'), flush: anyNamed('flush'), )).thenThrow(FileSystemException('', '', OSError('', errorCode))); + when(mockFile.openSync( + mode: anyNamed('mode'), + )).thenThrow(FileSystemException('', '', OSError('', errorCode))); } void setupCreateTempMocks({ @@ -79,6 +82,7 @@ group('throws ToolExit on Windows', () { const int kDeviceFull = 112; const int kUserMappedSectionOpened = 1224; + const int kUserPermissionDenied = 5; MockFileSystem mockFileSystem; ErrorHandlingFileSystem fs; @@ -91,6 +95,28 @@ when(mockFileSystem.path).thenReturn(MockPathContext()); }); + testWithoutContext('when access is denied', () async { + setupWriteMocks( + mockFileSystem: mockFileSystem, + fs: fs, + errorCode: kUserPermissionDenied, + ); + + final File file = fs.file('file'); + + const String expectedMessage = 'The flutter tool cannot access the file'; + expect(() async => await file.writeAsBytes(<int>[0]), + throwsToolExit(message: expectedMessage)); + expect(() async => await file.writeAsString(''), + throwsToolExit(message: expectedMessage)); + expect(() => file.writeAsBytesSync(<int>[0]), + throwsToolExit(message: expectedMessage)); + expect(() => file.writeAsStringSync(''), + throwsToolExit(message: expectedMessage)); + expect(() => file.openSync(), + throwsToolExit(message: expectedMessage)); + }); + testWithoutContext('when writing to a full device', () async { setupWriteMocks( mockFileSystem: mockFileSystem,
diff --git a/packages/flutter_tools/test/src/common.dart b/packages/flutter_tools/test/src/common.dart index d4d9382..2378f8f 100644 --- a/packages/flutter_tools/test/src/common.dart +++ b/packages/flutter_tools/test/src/common.dart
@@ -389,3 +389,24 @@ ); } } + +/// A file system that allows preconfiguring certain entities. +/// +/// This is useful for inserting mocks/entities which throw errors or +/// have other behavior that is not easily configured through the +/// filesystem interface. +class ConfiguredFileSystem extends ForwardingFileSystem { + ConfiguredFileSystem(FileSystem delegate, {@required this.entities}) : super(delegate); + + final Map<String, FileSystemEntity> entities; + + @override + File file(dynamic path) { + return (entities[path] as File) ?? super.file(path); + } + + @override + Directory directory(dynamic path) { + return (entities[path] as Directory) ?? super.directory(path); + } +}
diff --git a/packages/flutter_tools/test/template_test.dart b/packages/flutter_tools/test/template_test.dart index 3d82a28..361ea52 100644 --- a/packages/flutter_tools/test/template_test.dart +++ b/packages/flutter_tools/test/template_test.dart
@@ -3,29 +3,26 @@ // found in the LICENSE file. import 'package:file/memory.dart'; +import 'package:file_testing/file_testing.dart'; import 'package:flutter_tools/src/base/file_system.dart'; +import 'package:flutter_tools/src/base/logger.dart'; +import 'package:flutter_tools/src/base/template.dart'; import 'package:flutter_tools/src/cache.dart'; import 'package:flutter_tools/src/dart/pub.dart'; import 'package:flutter_tools/src/template.dart'; -import 'package:flutter_tools/src/globals.dart' as globals; import 'package:mockito/mockito.dart'; - import 'src/common.dart'; -import 'src/testbed.dart'; void main() { - Testbed testbed; - - setUp(() { - testbed = Testbed(); - }); - - test('Template.render throws ToolExit when FileSystem exception is raised', () => testbed.run(() { + testWithoutContext('Template.render throws ToolExit when FileSystem exception is raised', () { + final MemoryFileSystem fileSystem = MemoryFileSystem.test(); final Template template = Template( - globals.fs.directory('examples'), - globals.fs.currentDirectory, + fileSystem.directory('examples'), + fileSystem.currentDirectory, null, - fileSystem: globals.fs, + fileSystem: fileSystem, + logger: BufferLogger.test(), + templateRenderer: FakeTemplateRenderer(), templateManifest: null, ); final MockDirectory mockDirectory = MockDirectory(); @@ -33,9 +30,33 @@ expect(() => template.render(mockDirectory, <String, Object>{}), throwsToolExit()); - })); + }); - test('Template.render replaces .img.tmpl files with files from the image source', () => testbed.run(() { + testWithoutContext('Template.render attempts to read byte from template file before copying', () { + final MemoryFileSystem baseFileSystem = MemoryFileSystem.test(); + baseFileSystem.file('templates/foo.copy.tmpl').createSync(recursive: true); + final ConfiguredFileSystem fileSystem = ConfiguredFileSystem( + baseFileSystem, + entities: <String, FileSystemEntity>{ + '/templates/foo.copy.tmpl': FakeFile('/templates/foo.copy.tmpl'), + }, + ); + + final Template template = Template( + fileSystem.directory('templates'), + fileSystem.currentDirectory, + null, + fileSystem: fileSystem, + logger: BufferLogger.test(), + templateRenderer: FakeTemplateRenderer(), + templateManifest: null, + ); + + expect(() => template.render(fileSystem.directory('out'), <String, Object>{}), + throwsA(isA<FileSystemException>())); + }); + + testWithoutContext('Template.render replaces .img.tmpl files with files from the image source', () { final MemoryFileSystem fileSystem = MemoryFileSystem(); final Directory templateDir = fileSystem.directory('templates'); final Directory imageSourceDir = fileSystem.directory('template_images'); @@ -52,43 +73,77 @@ imageSourceDir, fileSystem: fileSystem, templateManifest: null, + logger: BufferLogger.test(), + templateRenderer: FakeTemplateRenderer(), ); template.render(destination, <String, Object>{}); final File destinationImage = destination.childFile(imageName); - expect(destinationImage.existsSync(), true); + expect(destinationImage, exists); expect(destinationImage.readAsBytesSync(), equals(sourceImage.readAsBytesSync())); - })); + }); - test('Template.fromName runs pub get if .packages is missing', () => testbed.run(() async { + testWithoutContext('Template.fromName runs pub get if .packages is missing', () async { final MemoryFileSystem fileSystem = MemoryFileSystem(); + final MockPub pub = MockPub(); + when(pub.get( + context: PubContext.pubGet, + directory: anyNamed('directory'), + generateSyntheticPackage: false, + )).thenThrow(UnsupportedError('')); + Cache.flutterRoot = '/flutter'; // Attempting to run pub in a test throws. - await expectLater(Template.fromName('app', fileSystem: fileSystem, templateManifest: null), - throwsUnsupportedError); - })); + await expectLater(Template.fromName( + 'app', + fileSystem: fileSystem, + templateManifest: null, + logger: BufferLogger.test(), + pub: pub, + templateRenderer: FakeTemplateRenderer(), + ), + throwsUnsupportedError, + ); + }); - test('Template.fromName runs pub get if .packages is missing flutter_template_images', () => testbed.run(() async { + testWithoutContext('Template.fromName runs pub get if .packages is missing flutter_template_images', () async { final MemoryFileSystem fileSystem = MemoryFileSystem(); + final MockPub pub = MockPub(); + when(pub.get( + context: PubContext.pubGet, + directory: anyNamed('directory'), + generateSyntheticPackage: false, + )).thenThrow(UnsupportedError('')); + Cache.flutterRoot = '/flutter'; final File packagesFile = fileSystem.directory(Cache.flutterRoot) - .childDirectory('packages') - .childDirectory('flutter_tools') - .childFile('.packages'); + .childDirectory('packages') + .childDirectory('flutter_tools') + .childFile('.packages'); packagesFile.createSync(recursive: true); // Attempting to run pub in a test throws. - await expectLater(Template.fromName('app', fileSystem: fileSystem, templateManifest: null), - throwsUnsupportedError); - })); + await expectLater( + Template.fromName( + 'app', + fileSystem: fileSystem, + templateManifest: null, + logger: BufferLogger.test(), + pub: pub, + templateRenderer: FakeTemplateRenderer(), + ), + throwsUnsupportedError, + ); + }); - test('Template.fromName runs pub get if flutter_template_images directory is missing', () => testbed.run(() async { + testWithoutContext('Template.fromName runs pub get if flutter_template_images directory is missing', () async { final MemoryFileSystem fileSystem = MemoryFileSystem(); + final MockPub pub = MockPub(); Cache.flutterRoot = '/flutter'; final File packagesFile = fileSystem.directory(Cache.flutterRoot) - .childDirectory('packages') - .childDirectory('flutter_tools') - .childFile('.packages'); + .childDirectory('packages') + .childDirectory('flutter_tools') + .childFile('.packages'); packagesFile.createSync(recursive: true); packagesFile.writeAsStringSync('\n'); @@ -101,11 +156,35 @@ packagesFile.writeAsStringSync('flutter_template_images:file:///flutter_template_images'); }); - await Template.fromName('app', fileSystem: fileSystem, templateManifest: null); - }, overrides: <Type, Generator>{ - Pub: () => MockPub(), - })); + await Template.fromName( + 'app', + fileSystem: fileSystem, + templateManifest: null, + logger: BufferLogger.test(), + templateRenderer: FakeTemplateRenderer(), + pub: pub, + ); + }); } class MockPub extends Mock implements Pub {} class MockDirectory extends Mock implements Directory {} + +class FakeFile extends Fake implements File { + FakeFile(this.path); + + @override + final String path; + + @override + int lengthSync() { + throw const FileSystemException('', '', OSError('', 5)); + } +} + +class FakeTemplateRenderer extends TemplateRenderer { + @override + String renderString(String template, dynamic context, {bool htmlEscapeValues = false}) { + return ''; + } +}