From a9e5e104b636fc8b82558de914c79442749a6ecf Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Sat, 19 Sep 2020 17:02:15 -0700 Subject: [PATCH 1/3] [path_provider] Move Windows FFI behind a conditional import Moves the real implementation of path_provider_windows behind a conditional export, instead exporting a stub on platforms that don't support dart:ffi. This avoids build breakage in web projects that have transitive dependencies on path_provider (and thus path_provider_windows due to manual endorsement). This will no longer be necessary once https://github.com/flutter/flutter/issues/52267 is fixed, since only Windows builds will ever need to have code-level dependency on path_provider_windows. --- .../path_provider_windows/CHANGELOG.md | 6 + .../lib/path_provider_windows.dart | 223 +----------------- .../lib/{ => src}/folders.dart | 0 .../lib/src/path_provider_windows_real.dart | 223 ++++++++++++++++++ .../lib/src/path_provider_windows_stub.dart | 30 +++ .../path_provider_windows/pubspec.yaml | 2 +- 6 files changed, 265 insertions(+), 219 deletions(-) rename packages/path_provider/path_provider_windows/lib/{ => src}/folders.dart (100%) create mode 100644 packages/path_provider/path_provider_windows/lib/src/path_provider_windows_real.dart create mode 100644 packages/path_provider/path_provider_windows/lib/src/path_provider_windows_stub.dart diff --git a/packages/path_provider/path_provider_windows/CHANGELOG.md b/packages/path_provider/path_provider_windows/CHANGELOG.md index 0b7328631f24..a7cfe1dbb283 100644 --- a/packages/path_provider/path_provider_windows/CHANGELOG.md +++ b/packages/path_provider/path_provider_windows/CHANGELOG.md @@ -1,3 +1,9 @@ +## 0.0.4 + +* Move the actual implementation behind a conditional import, exporting + a stub for platforms that don't support FFI. Fixes web builds in + projects with transitive dependencies on path_provider. + ## 0.0.3 * Add missing `pluginClass: none` for compatibilty with stable channel. diff --git a/packages/path_provider/path_provider_windows/lib/path_provider_windows.dart b/packages/path_provider/path_provider_windows/lib/path_provider_windows.dart index e29b70a714d9..ed96698c9589 100644 --- a/packages/path_provider/path_provider_windows/lib/path_provider_windows.dart +++ b/packages/path_provider/path_provider_windows/lib/path_provider_windows.dart @@ -2,221 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import 'dart:async'; -import 'dart:io'; -import 'dart:ffi'; - -import 'package:ffi/ffi.dart'; -import 'package:meta/meta.dart'; -import 'package:path/path.dart' as path; -import 'package:path_provider_platform_interface/path_provider_platform_interface.dart'; -import 'package:path_provider_windows/folders.dart'; -import 'package:win32/win32.dart'; - -/// Wraps the Win32 VerQueryValue API call. -/// -/// This class exists to allow injecting alternate metadata in tests without -/// building multiple custom test binaries. -@visibleForTesting -class VersionInfoQuerier { - /// Returns the value for [key] in [versionInfo]s English strings section, or - /// null if there is no such entry, or if versionInfo is null. - getStringValue(Pointer versionInfo, key) { - if (versionInfo == null) { - return null; - } - const kEnUsLanguageCode = '040904e4'; - final keyPath = TEXT('\\StringFileInfo\\$kEnUsLanguageCode\\$key'); - final length = allocate(); - final valueAddress = allocate(); - try { - if (VerQueryValue(versionInfo, keyPath, valueAddress, length) == 0) { - return null; - } - return Pointer.fromAddress(valueAddress.value) - .unpackString(length.value); - } finally { - free(keyPath); - free(length); - free(valueAddress); - } - } -} - -/// The Windows implementation of [PathProviderPlatform] -/// -/// This class implements the `package:path_provider` functionality for Windows. -class PathProviderWindows extends PathProviderPlatform { - /// The object to use for performing VerQueryValue calls. - @visibleForTesting - VersionInfoQuerier versionInfoQuerier = VersionInfoQuerier(); - - /// This is typically the same as the TMP environment variable. - @override - Future getTemporaryPath() async { - final buffer = allocate(count: MAX_PATH + 1).cast(); - String path; - - try { - final length = GetTempPath(MAX_PATH, buffer); - - if (length == 0) { - final error = GetLastError(); - throw WindowsException(error); - } else { - path = buffer.unpackString(length); - - // GetTempPath adds a trailing backslash, but SHGetKnownFolderPath does - // not. Strip off trailing backslash for consistency with other methods - // here. - if (path.endsWith('\\')) { - path = path.substring(0, path.length - 1); - } - } - - // Ensure that the directory exists, since GetTempPath doesn't. - final directory = Directory(path); - if (!directory.existsSync()) { - await directory.create(recursive: true); - } - - return Future.value(path); - } finally { - free(buffer); - } - } - - @override - Future getApplicationSupportPath() async { - final appDataRoot = await getPath(WindowsKnownFolder.RoamingAppData); - final directory = Directory( - path.join(appDataRoot, _getApplicationSpecificSubdirectory())); - // Ensure that the directory exists if possible, since it will on other - // platforms. If the name is longer than MAXPATH, creating will fail, so - // skip that step; it's up to the client to decide what to do with the path - // in that case (e.g., using a short path). - if (directory.path.length <= MAX_PATH) { - if (!directory.existsSync()) { - await directory.create(recursive: true); - } - } - return directory.path; - } - - @override - Future getApplicationDocumentsPath() => - getPath(WindowsKnownFolder.Documents); - - @override - Future getDownloadsPath() => getPath(WindowsKnownFolder.Downloads); - - /// Retrieve any known folder from Windows. - /// - /// folderID is a GUID that represents a specific known folder ID, drawn from - /// [WindowsKnownFolder]. - Future getPath(String folderID) { - final pathPtrPtr = allocate(); - Pointer pathPtr; - - try { - GUID knownFolderID = GUID.fromString(folderID); - - final hr = SHGetKnownFolderPath( - knownFolderID.addressOf, KF_FLAG_DEFAULT, NULL, pathPtrPtr); - - if (FAILED(hr)) { - if (hr == E_INVALIDARG || hr == E_FAIL) { - throw WindowsException(hr); - } - } - - pathPtr = Pointer.fromAddress(pathPtrPtr.value); - final path = pathPtr.unpackString(MAX_PATH); - return Future.value(path); - } finally { - CoTaskMemFree(pathPtr.cast()); - free(pathPtrPtr); - } - } - - /// Returns the relative path string to append to the root directory returned - /// by Win32 APIs for application storage (such as RoamingAppDir) to get a - /// directory that is unique to the application. - /// - /// The convention is to use company-name\product-name\. This will use that if - /// possible, using the data in the VERSIONINFO resource, with the following - /// fallbacks: - /// - If the company name isn't there, that component will be dropped. - /// - If the product name isn't there, it will use the exe's filename (without - /// extension). - String _getApplicationSpecificSubdirectory() { - String companyName; - String productName; - - final Pointer moduleNameBuffer = - allocate(count: MAX_PATH + 1).cast(); - final Pointer unused = allocate(); - Pointer infoBuffer; - try { - // Get the module name. - final moduleNameLength = GetModuleFileName(0, moduleNameBuffer, MAX_PATH); - if (moduleNameLength == 0) { - final error = GetLastError(); - throw WindowsException(error); - } - - // From that, load the VERSIONINFO resource - int infoSize = GetFileVersionInfoSize(moduleNameBuffer, unused); - if (infoSize != 0) { - infoBuffer = allocate(count: infoSize); - if (GetFileVersionInfo(moduleNameBuffer, 0, infoSize, infoBuffer) == - 0) { - free(infoBuffer); - infoBuffer = null; - } - } - companyName = _sanitizedDirectoryName( - versionInfoQuerier.getStringValue(infoBuffer, 'CompanyName')); - productName = _sanitizedDirectoryName( - versionInfoQuerier.getStringValue(infoBuffer, 'ProductName')); - - // If there was no product name, use the executable name. - if (productName == null) { - productName = path.basenameWithoutExtension( - moduleNameBuffer.unpackString(moduleNameLength)); - } - - return companyName != null - ? path.join(companyName, productName) - : productName; - } finally { - free(moduleNameBuffer); - free(unused); - if (infoBuffer != null) { - free(infoBuffer); - } - } - } - - /// Makes [rawString] safe as a directory component. See - /// https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file#naming-conventions - /// - /// If after sanitizing the string is empty, returns null. - String _sanitizedDirectoryName(String rawString) { - if (rawString == null) { - return null; - } - String sanitized = rawString - // Replace banned characters. - .replaceAll(RegExp(r'[<>:"/\\|?*]'), '_') - // Remove trailing whitespace. - .trimRight() - // Ensure that it does not end with a '.'. - .replaceAll(RegExp(r'[.]+$'), ''); - const kMaxComponentLength = 255; - if (sanitized.length > kMaxComponentLength) { - sanitized = sanitized.substring(0, kMaxComponentLength); - } - return sanitized.isEmpty ? null : sanitized; - } -} +// path_provider_windows is implemented using FFI; export a stub for platforms +// that don't support FFI (e.g., web) to avoid having transitive dependencies +// break web compilation. +export 'src/path_provider_windows_stub.dart' + if (dart.library.ffi) 'src/path_provider_windows_real.dart'; diff --git a/packages/path_provider/path_provider_windows/lib/folders.dart b/packages/path_provider/path_provider_windows/lib/src/folders.dart similarity index 100% rename from packages/path_provider/path_provider_windows/lib/folders.dart rename to packages/path_provider/path_provider_windows/lib/src/folders.dart diff --git a/packages/path_provider/path_provider_windows/lib/src/path_provider_windows_real.dart b/packages/path_provider/path_provider_windows/lib/src/path_provider_windows_real.dart new file mode 100644 index 000000000000..7ff448abf020 --- /dev/null +++ b/packages/path_provider/path_provider_windows/lib/src/path_provider_windows_real.dart @@ -0,0 +1,223 @@ +// Copyright 2020 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'dart:async'; +import 'dart:io'; +import 'dart:ffi'; + +import 'package:ffi/ffi.dart'; +import 'package:meta/meta.dart'; +import 'package:path/path.dart' as path; +import 'package:path_provider_platform_interface/path_provider_platform_interface.dart'; +import 'package:win32/win32.dart'; + +import 'folders.dart'; + +/// Wraps the Win32 VerQueryValue API call. +/// +/// This class exists to allow injecting alternate metadata in tests without +/// building multiple custom test binaries. +@visibleForTesting +class VersionInfoQuerier { + /// Returns the value for [key] in [versionInfo]s English strings section, or + /// null if there is no such entry, or if versionInfo is null. + getStringValue(Pointer versionInfo, key) { + if (versionInfo == null) { + return null; + } + const kEnUsLanguageCode = '040904e4'; + final keyPath = TEXT('\\StringFileInfo\\$kEnUsLanguageCode\\$key'); + final length = allocate(); + final valueAddress = allocate(); + try { + if (VerQueryValue(versionInfo, keyPath, valueAddress, length) == 0) { + return null; + } + return Pointer.fromAddress(valueAddress.value) + .unpackString(length.value); + } finally { + free(keyPath); + free(length); + free(valueAddress); + } + } +} + +/// The Windows implementation of [PathProviderPlatform] +/// +/// This class implements the `package:path_provider` functionality for Windows. +class PathProviderWindows extends PathProviderPlatform { + /// The object to use for performing VerQueryValue calls. + @visibleForTesting + VersionInfoQuerier versionInfoQuerier = VersionInfoQuerier(); + + /// This is typically the same as the TMP environment variable. + @override + Future getTemporaryPath() async { + final buffer = allocate(count: MAX_PATH + 1).cast(); + String path; + + try { + final length = GetTempPath(MAX_PATH, buffer); + + if (length == 0) { + final error = GetLastError(); + throw WindowsException(error); + } else { + path = buffer.unpackString(length); + + // GetTempPath adds a trailing backslash, but SHGetKnownFolderPath does + // not. Strip off trailing backslash for consistency with other methods + // here. + if (path.endsWith('\\')) { + path = path.substring(0, path.length - 1); + } + } + + // Ensure that the directory exists, since GetTempPath doesn't. + final directory = Directory(path); + if (!directory.existsSync()) { + await directory.create(recursive: true); + } + + return Future.value(path); + } finally { + free(buffer); + } + } + + @override + Future getApplicationSupportPath() async { + final appDataRoot = await getPath(WindowsKnownFolder.RoamingAppData); + final directory = Directory( + path.join(appDataRoot, _getApplicationSpecificSubdirectory())); + // Ensure that the directory exists if possible, since it will on other + // platforms. If the name is longer than MAXPATH, creating will fail, so + // skip that step; it's up to the client to decide what to do with the path + // in that case (e.g., using a short path). + if (directory.path.length <= MAX_PATH) { + if (!directory.existsSync()) { + await directory.create(recursive: true); + } + } + return directory.path; + } + + @override + Future getApplicationDocumentsPath() => + getPath(WindowsKnownFolder.Documents); + + @override + Future getDownloadsPath() => getPath(WindowsKnownFolder.Downloads); + + /// Retrieve any known folder from Windows. + /// + /// folderID is a GUID that represents a specific known folder ID, drawn from + /// [WindowsKnownFolder]. + Future getPath(String folderID) { + final pathPtrPtr = allocate(); + Pointer pathPtr; + + try { + GUID knownFolderID = GUID.fromString(folderID); + + final hr = SHGetKnownFolderPath( + knownFolderID.addressOf, KF_FLAG_DEFAULT, NULL, pathPtrPtr); + + if (FAILED(hr)) { + if (hr == E_INVALIDARG || hr == E_FAIL) { + throw WindowsException(hr); + } + } + + pathPtr = Pointer.fromAddress(pathPtrPtr.value); + final path = pathPtr.unpackString(MAX_PATH); + return Future.value(path); + } finally { + CoTaskMemFree(pathPtr.cast()); + free(pathPtrPtr); + } + } + + /// Returns the relative path string to append to the root directory returned + /// by Win32 APIs for application storage (such as RoamingAppDir) to get a + /// directory that is unique to the application. + /// + /// The convention is to use company-name\product-name\. This will use that if + /// possible, using the data in the VERSIONINFO resource, with the following + /// fallbacks: + /// - If the company name isn't there, that component will be dropped. + /// - If the product name isn't there, it will use the exe's filename (without + /// extension). + String _getApplicationSpecificSubdirectory() { + String companyName; + String productName; + + final Pointer moduleNameBuffer = + allocate(count: MAX_PATH + 1).cast(); + final Pointer unused = allocate(); + Pointer infoBuffer; + try { + // Get the module name. + final moduleNameLength = GetModuleFileName(0, moduleNameBuffer, MAX_PATH); + if (moduleNameLength == 0) { + final error = GetLastError(); + throw WindowsException(error); + } + + // From that, load the VERSIONINFO resource + int infoSize = GetFileVersionInfoSize(moduleNameBuffer, unused); + if (infoSize != 0) { + infoBuffer = allocate(count: infoSize); + if (GetFileVersionInfo(moduleNameBuffer, 0, infoSize, infoBuffer) == + 0) { + free(infoBuffer); + infoBuffer = null; + } + } + companyName = _sanitizedDirectoryName( + versionInfoQuerier.getStringValue(infoBuffer, 'CompanyName')); + productName = _sanitizedDirectoryName( + versionInfoQuerier.getStringValue(infoBuffer, 'ProductName')); + + // If there was no product name, use the executable name. + if (productName == null) { + productName = path.basenameWithoutExtension( + moduleNameBuffer.unpackString(moduleNameLength)); + } + + return companyName != null + ? path.join(companyName, productName) + : productName; + } finally { + free(moduleNameBuffer); + free(unused); + if (infoBuffer != null) { + free(infoBuffer); + } + } + } + + /// Makes [rawString] safe as a directory component. See + /// https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file#naming-conventions + /// + /// If after sanitizing the string is empty, returns null. + String _sanitizedDirectoryName(String rawString) { + if (rawString == null) { + return null; + } + String sanitized = rawString + // Replace banned characters. + .replaceAll(RegExp(r'[<>:"/\\|?*]'), '_') + // Remove trailing whitespace. + .trimRight() + // Ensure that it does not end with a '.'. + .replaceAll(RegExp(r'[.]+$'), ''); + const kMaxComponentLength = 255; + if (sanitized.length > kMaxComponentLength) { + sanitized = sanitized.substring(0, kMaxComponentLength); + } + return sanitized.isEmpty ? null : sanitized; + } +} diff --git a/packages/path_provider/path_provider_windows/lib/src/path_provider_windows_stub.dart b/packages/path_provider/path_provider_windows/lib/src/path_provider_windows_stub.dart new file mode 100644 index 000000000000..db79ee3ab411 --- /dev/null +++ b/packages/path_provider/path_provider_windows/lib/src/path_provider_windows_stub.dart @@ -0,0 +1,30 @@ +// Copyright 2020 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:path_provider_platform_interface/path_provider_platform_interface.dart'; + +import '../path_provider_windows.dart'; + +/// A stub implementation to satisfy compilation of multi-platform packages that +/// depend on path_provider_windows. This should never actually be created. +/// +/// Notably, because path_provider needs to manually register +/// path_provider_windows, anything with a transitive dependency on +/// path_provider will also depend on path_provider_windows, not just at the +/// pubspec level but the code level. +class PathProviderWindows extends PathProviderPlatform { + /// Errors on attempted instantiation of the stub. It exists only to satisfy + /// compile-time dependencies, and should never actually be created. + PathProviderWindows() { + assert(false); + } + + /// Stub; see comment on VersionInfoQuerier. + VersionInfoQuerier versionInfoQuerier; +} + +/// Stub to satisfy the analyzer, which doesn't seem to handle conditional +/// exports correctly. +class VersionInfoQuerier { +} diff --git a/packages/path_provider/path_provider_windows/pubspec.yaml b/packages/path_provider/path_provider_windows/pubspec.yaml index 998f4e2c616b..38f6316abef8 100644 --- a/packages/path_provider/path_provider_windows/pubspec.yaml +++ b/packages/path_provider/path_provider_windows/pubspec.yaml @@ -1,7 +1,7 @@ name: path_provider_windows description: Windows implementation of the path_provider plugin homepage: https://github.com/flutter/plugins/tree/master/packages/path_provider/path_provider_windows -version: 0.0.3 +version: 0.0.4 flutter: plugin: From 79f7797e862a064ef3accc25b99a2fcb0b5fb1c6 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Sat, 19 Sep 2020 17:14:31 -0700 Subject: [PATCH 2/3] format --- .../lib/src/path_provider_windows_stub.dart | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/path_provider/path_provider_windows/lib/src/path_provider_windows_stub.dart b/packages/path_provider/path_provider_windows/lib/src/path_provider_windows_stub.dart index db79ee3ab411..a6ed436513d7 100644 --- a/packages/path_provider/path_provider_windows/lib/src/path_provider_windows_stub.dart +++ b/packages/path_provider/path_provider_windows/lib/src/path_provider_windows_stub.dart @@ -26,5 +26,4 @@ class PathProviderWindows extends PathProviderPlatform { /// Stub to satisfy the analyzer, which doesn't seem to handle conditional /// exports correctly. -class VersionInfoQuerier { -} +class VersionInfoQuerier {} From 7c7e623bfa2d208d3ddb5d5ca48de200627a84f1 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Sun, 20 Sep 2020 06:12:23 -0700 Subject: [PATCH 3/3] Remove accidental import --- .../lib/src/path_provider_windows_stub.dart | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/path_provider/path_provider_windows/lib/src/path_provider_windows_stub.dart b/packages/path_provider/path_provider_windows/lib/src/path_provider_windows_stub.dart index a6ed436513d7..fedb53735fb1 100644 --- a/packages/path_provider/path_provider_windows/lib/src/path_provider_windows_stub.dart +++ b/packages/path_provider/path_provider_windows/lib/src/path_provider_windows_stub.dart @@ -4,8 +4,6 @@ import 'package:path_provider_platform_interface/path_provider_platform_interface.dart'; -import '../path_provider_windows.dart'; - /// A stub implementation to satisfy compilation of multi-platform packages that /// depend on path_provider_windows. This should never actually be created. ///