[flutter_tools] implement safe file copy with multiple fallbacks (#69000)

The tool observes a large number of unhandled exceptions during the file copy portion of flutter create. it is difficult to tell whether the permission issue is caused by the source/destination, or whether it is due to a bug in dart:io.

To work around this, implement a permission check for both the source and dest files. If either fails, the tool can exit with a more specific message.

If these checks pass, then perform the actual copy. If the copy fails, fallback to manually copying the bytes

packages/flutter_tools/lib/src/base/error_handling_io.dart

@@ -4,12 +4,14 @@
 
 import 'dart:convert';
 import 'dart:io' as io show Directory, File, Link, ProcessException, ProcessResult, ProcessSignal, systemEncoding, Process, ProcessStartMode;
+import 'dart:typed_data';
 
 import 'package:file/file.dart';
 import 'package:meta/meta.dart';
 import 'package:path/path.dart' as p; // ignore: package_path_import
 import 'package:process/process.dart';
 
+import '../reporting/reporting.dart';
 import 'common.dart' show throwToolExit;
 import 'platform.dart';
 
@@ -272,6 +274,66 @@ class ErrorHandlingFile
     );
   }
 
+  /// This copy method attempts to handle file system errors from both reading
+  /// and writing the copied file.
+  @override
+  File copySync(String newPath) {
+    final File resultFile = fileSystem.file(newPath);
+    // First check if the source file can be read. If not, bail through error
+    // handling.
+    _runSync<void>(
+      () => delegate.openSync(mode: FileMode.read).closeSync(),
+      platform: _platform,
+      failureMessage: 'Flutter failed to copy $path to $newPath due to source location error'
+    );
+    // Next check if the destination file can be written. If not, bail through
+    // error handling.
+    _runSync<void>(
+      ()  {
+        resultFile.createSync(recursive: true);
+        resultFile.openSync(mode: FileMode.writeOnly).closeSync();
+      },
+      platform: _platform,
+      failureMessage: 'Flutter faiuled to copy $path to $newPath due to destination location error'
+    );
+    // If both of the above checks passed, attempt to copy the file and catch
+    // any thrown errors.
+    try {
+      return wrapFile(delegate.copySync(newPath));
+    } on FileSystemException {
+      // Proceed below
+    }
+    // If the copy failed but both of the above checks passed, copy the bytes
+    // directly.
+    _runSync(() {
+      RandomAccessFile source;
+      RandomAccessFile sink;
+      try {
+        source = delegate.openSync(mode: FileMode.read);
+        sink = resultFile.openSync(mode: FileMode.writeOnly);
+        // 64k is the same sized buffer used by dart:io for `File.openRead`.
+        final Uint8List buffer = Uint8List(64 * 1024);
+        final int totalBytes = source.lengthSync();
+        int bytes = 0;
+        while (bytes < totalBytes) {
+          final int chunkLength = source.readIntoSync(buffer);
+          sink.writeFromSync(buffer, 0, chunkLength);
+          bytes += chunkLength;
+        }
+      } catch (err) { // ignore: avoid_catches_without_on_clauses
+        ErrorHandlingFileSystem.deleteIfExists(resultFile, recursive: true);
+        rethrow;
+      } finally {
+        source?.closeSync();
+        sink?.closeSync();
+      }
+    }, platform: _platform, failureMessage: 'Flutter failed to copy $path to $newPath due to unknown error');
+    // The original copy failed, but the manual copy worked. Report an analytics event to
+    // track this to determine if this code path is actually hit.
+    ErrorHandlingEvent('copy-fallback').send();
+    return wrapFile(resultFile);
+  }
+
   @override
   String toString() => delegate.toString();
 }

packages/flutter_tools/lib/src/commands/create.dart

@@ -796,7 +796,6 @@ https://flutter.dev/docs/development/packages-and-plugins/developing-packages#pl
       logger: globals.logger,
       templateRenderer: globals.templateRenderer,
       templateManifest: templateManifest,
-      pub: pub,
     );
     return template.render(directory, context, overwriteExisting: overwrite);
   }

packages/flutter_tools/lib/src/project.dart

@@ -14,7 +14,6 @@ import 'base/file_system.dart';
 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;
@@ -702,7 +701,6 @@ class IosProject extends FlutterProjectPlatform implements XcodeBasedProject {
       templateManifest: null,
       logger: globals.logger,
       templateRenderer: globals.templateRenderer,
-      pub: pub,
     );
     template.render(
       target,
@@ -862,7 +860,6 @@ to migrate your project.
       templateManifest: null,
       logger: globals.logger,
       templateRenderer: globals.templateRenderer,
-      pub: pub,
     );
     template.render(
       target,

packages/flutter_tools/lib/src/reporting/events.dart

@@ -248,3 +248,8 @@ class CodeSizeEvent extends UsageEvent {
     flutterUsage: flutterUsage ?? globals.flutterUsage,
   );
 }
+
+/// An event for tracking the usage of specific error handling fallbacks.
+class ErrorHandlingEvent extends UsageEvent {
+  ErrorHandlingEvent(String parameter) : super('error-handling', parameter, flutterUsage: globals.flutterUsage);
+}

packages/flutter_tools/lib/src/template.dart

@@ -12,7 +12,6 @@ import 'base/logger.dart';
 import 'base/template.dart';
 import 'cache.dart';
 import 'dart/package_map.dart';
-import 'dart/pub.dart';
 
 /// Expands templates in a directory to a destination. All files that must
 /// undergo template expansion should end with the '.tmpl' extension. All files
@@ -70,11 +69,10 @@ class Template {
     @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, logger, pub);
+    final Directory imageDir = await _templateImageDirectory(name, fileSystem, logger);
     return Template(
       templateDir,
       templateDir, imageDir,
@@ -235,7 +233,6 @@ class Template {
       //         not need mustache rendering but needs to be directly copied.
 
       if (sourceFile.path.endsWith(copyTemplateExtension)) {
-        _validateReadPermissions(sourceFile);
         sourceFile.copySync(finalDestinationFile.path);
 
         return;
@@ -247,7 +244,6 @@ class Template {
       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;
@@ -257,7 +253,6 @@ class Template {
       //         rendering via mustache.
 
       if (sourceFile.path.endsWith(templateExtension)) {
-         _validateReadPermissions(sourceFile);
         final String templateContents = sourceFile.readAsStringSync();
         final String renderedContents = _templateRenderer.renderString(templateContents, context);
 
@@ -268,20 +263,11 @@ class Template {
 
       // 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) {
@@ -292,7 +278,7 @@ Directory _templateDirectoryInPackage(String name, FileSystem fileSystem) {
 
 // Returns the directory containing the 'name' template directory in
 // flutter_template_images, to resolve image placeholder against.
-Future<Directory> _templateImageDirectory(String name, FileSystem fileSystem, Logger logger, Pub pub) async {
+Future<Directory> _templateImageDirectory(String name, FileSystem fileSystem, Logger logger) async {
   final String toolPackagePath = fileSystem.path.join(
       Cache.flutterRoot, 'packages', 'flutter_tools');
   final String packageFilePath = fileSystem.path.join(toolPackagePath, kPackagesFileName);

packages/flutter_tools/test/general.shard/base/error_handling_io_test.dart

@@ -3,11 +3,14 @@
 // found in the LICENSE file.
 
 import 'package:file/file.dart';
+import 'package:file/memory.dart';
 import 'package:flutter_tools/src/base/common.dart';
 import 'package:flutter_tools/src/base/error_handling_io.dart';
 import 'package:flutter_tools/src/base/file_system.dart';
 import 'package:flutter_tools/src/base/io.dart';
 import 'package:flutter_tools/src/base/platform.dart';
+import 'package:flutter_tools/src/globals.dart' as globals show flutterUsage;
+import 'package:flutter_tools/src/reporting/reporting.dart';
 import 'package:mockito/mockito.dart';
 import 'package:path/path.dart' as path; // ignore: package_path_import
 
@@ -18,6 +21,9 @@ class MockFile extends Mock implements File {}
 class MockFileSystem extends Mock implements FileSystem {}
 class MockPathContext extends Mock implements path.Context {}
 class MockDirectory extends Mock implements Directory {}
+class MockRandomAccessFile extends Mock implements RandomAccessFile {}
+class MockProcessManager extends Mock implements ProcessManager {}
+class MockUsage extends Mock implements Usage {}
 
 final Platform windowsPlatform = FakePlatform(
   operatingSystem: 'windows',
@@ -787,6 +793,122 @@ void main() {
              throwsToolExit(message: expectedMessage));
     });
   });
+
+  group('CopySync' , () {
+    const int eaccess = 13;
+    MockFileSystem mockFileSystem;
+    ErrorHandlingFileSystem fileSystem;
+
+    setUp(() {
+      mockFileSystem = MockFileSystem();
+      fileSystem = ErrorHandlingFileSystem(
+        delegate: mockFileSystem,
+        platform: linuxPlatform,
+      );
+      when(mockFileSystem.path).thenReturn(MockPathContext());
+    });
+
+    testWithoutContext('copySync handles error if openSync on source file fails', () {
+      final MockFile source = MockFile();
+      when(source.openSync(mode: anyNamed('mode')))
+        .thenThrow(const FileSystemException('', '', OSError('', eaccess)));
+      when(mockFileSystem.file('source')).thenReturn(source);
+
+      expect(() => fileSystem.file('source').copySync('dest'), throwsToolExit());
+    });
+
+    testWithoutContext('copySync handles error if openSync on destination file fails', () {
+      final MockFile source = MockFile();
+      final MockFile dest = MockFile();
+      when(source.openSync(mode: anyNamed('mode')))
+        .thenReturn(MockRandomAccessFile());
+      when(dest.openSync(mode: anyNamed('mode')))
+        .thenThrow(const FileSystemException('', '', OSError('', eaccess)));
+      when(mockFileSystem.file('source')).thenReturn(source);
+      when(mockFileSystem.file('dest')).thenReturn(dest);
+
+      expect(() => fileSystem.file('source').copySync('dest'), throwsToolExit());
+    });
+
+    testWithoutContext('copySync will copySync if there are no exceptions', () {
+      final MockFile source = MockFile();
+      final MockFile dest = MockFile();
+
+      when(source.copySync(any)).thenReturn(dest);
+      when(mockFileSystem.file('source')).thenReturn(source);
+      when(source.openSync(mode: anyNamed('mode')))
+        .thenReturn(MockRandomAccessFile());
+      when(mockFileSystem.file('dest')).thenReturn(dest);
+      when(dest.openSync(mode: anyNamed('mode')))
+        .thenReturn(MockRandomAccessFile());
+
+      fileSystem.file('source').copySync('dest');
+
+      verify(source.copySync('dest')).called(1);
+    });
+
+    // Uses context for analytics.
+    testUsingContext('copySync can directly copy bytes if both files can be opened but copySync fails', () {
+      final MemoryFileSystem memoryFileSystem = MemoryFileSystem.test();
+      final MockFile source = MockFile();
+      final MockFile dest = MockFile();
+      final List<int> expectedBytes = List<int>.generate(64 * 1024 + 3, (int i) => i.isEven ? 0 : 1);
+      final File memorySource = memoryFileSystem.file('source')
+        ..writeAsBytesSync(expectedBytes);
+      final File memoryDest = memoryFileSystem.file('dest')
+        ..createSync();
+
+      when(source.copySync(any))
+        .thenThrow(const FileSystemException('', '', OSError('', eaccess)));
+      when(source.openSync(mode: anyNamed('mode')))
+        .thenAnswer((Invocation invocation) => memorySource.openSync(mode: invocation.namedArguments[#mode] as FileMode));
+      when(dest.openSync(mode: anyNamed('mode')))
+        .thenAnswer((Invocation invocation) => memoryDest.openSync(mode: invocation.namedArguments[#mode] as FileMode));
+      when(mockFileSystem.file('source')).thenReturn(source);
+      when(mockFileSystem.file('dest')).thenReturn(dest);
+
+      fileSystem.file('source').copySync('dest');
+
+      expect(memoryDest.readAsBytesSync(), expectedBytes);
+      verify(globals.flutterUsage.sendEvent('error-handling', 'copy-fallback')).called(1);
+    }, overrides: <Type, Generator>{
+      Usage: () => MockUsage(),
+    });
+
+    // Uses context for analytics.
+    testUsingContext('copySync deletes the result file if the fallback fails', () {
+      final MemoryFileSystem memoryFileSystem = MemoryFileSystem.test();
+      final MockFile source = MockFile();
+      final MockFile dest = MockFile();
+      final File memorySource = memoryFileSystem.file('source')
+        ..createSync();
+      final File memoryDest = memoryFileSystem.file('dest')
+        ..createSync();
+      int calledCount = 0;
+
+      when(dest.existsSync()).thenReturn(true);
+      when(source.copySync(any))
+        .thenThrow(const FileSystemException('', '', OSError('', eaccess)));
+      when(source.openSync(mode: anyNamed('mode')))
+        .thenAnswer((Invocation invocation) {
+          if (calledCount == 1) {
+            throw const FileSystemException('', '', OSError('', eaccess));
+          }
+          calledCount +=  1;
+          return memorySource.openSync(mode: invocation.namedArguments[#mode] as FileMode);
+        });
+      when(dest.openSync(mode: anyNamed('mode')))
+        .thenAnswer((Invocation invocation) => memoryDest.openSync(mode: invocation.namedArguments[#mode] as FileMode));
+      when(mockFileSystem.file('source')).thenReturn(source);
+      when(mockFileSystem.file('dest')).thenReturn(dest);
+
+      expect(() => fileSystem.file('source').copySync('dest'), throwsToolExit());
+
+      verify(dest.deleteSync(recursive: true)).called(1);
+    }, overrides: <Type, Generator>{
+      Usage: () => MockUsage(),
+    });
+  });
 }
 
 void setupProcessManagerMocks(
@@ -823,5 +945,3 @@ void setupProcessManagerMocks(
     workingDirectory: anyNamed('workingDirectory'),
   )).thenThrow(ProcessException('', <String>[], '', errorCode));
 }
-
-class MockProcessManager extends Mock implements ProcessManager {}

packages/flutter_tools/test/template_test.dart

@@ -7,8 +7,6 @@ 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:mockito/mockito.dart';
 import 'src/common.dart';
@@ -32,30 +30,6 @@ void main() {
         throwsToolExit());
   });
 
-  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.test();
     final Directory templateDir = fileSystem.directory('templates');
@@ -82,106 +56,10 @@ void main() {
     expect(destinationImage, exists);
     expect(destinationImage.readAsBytesSync(), equals(sourceImage.readAsBytesSync()));
   });
-
-  testWithoutContext('Template.fromName runs pub get if .packages is missing', () async {
-    final MemoryFileSystem fileSystem = MemoryFileSystem.test();
-    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,
-        logger: BufferLogger.test(),
-        pub: pub,
-        templateRenderer: FakeTemplateRenderer(),
-      ),
-      throwsUnsupportedError,
-    );
-  });
-
-  testWithoutContext('Template.fromName runs pub get if .packages is missing flutter_template_images', () async {
-    final MemoryFileSystem fileSystem = MemoryFileSystem.test();
-    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');
-    packagesFile.createSync(recursive: true);
-
-    // Attempting to run pub in a test throws.
-    await expectLater(
-      Template.fromName(
-        'app',
-        fileSystem: fileSystem,
-        templateManifest: null,
-        logger: BufferLogger.test(),
-        pub: pub,
-        templateRenderer: FakeTemplateRenderer(),
-      ),
-      throwsUnsupportedError,
-    );
-  });
-
-  testWithoutContext('Template.fromName runs pub get if flutter_template_images directory is missing', () async {
-    final MemoryFileSystem fileSystem = MemoryFileSystem.test();
-    final MockPub pub = MockPub();
-    Cache.flutterRoot = '/flutter';
-    final File packagesFile = fileSystem.directory(Cache.flutterRoot)
-      .childDirectory('packages')
-      .childDirectory('flutter_tools')
-      .childFile('.packages');
-    packagesFile.createSync(recursive: true);
-    packagesFile.writeAsStringSync('\n');
-
-    when(pub.get(
-      context: PubContext.pubGet,
-      directory: anyNamed('directory'),
-      generateSyntheticPackage: false,
-    )).thenAnswer((Invocation invocation) async {
-      // Create valid package entry.
-      packagesFile.writeAsStringSync('flutter_template_images:file:///flutter_template_images');
-    });
-
-    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}) {