From 28ff59513bef4ae2f250a337d3c1d1dd57961d1a Mon Sep 17 00:00:00 2001 From: Andrew Kolos Date: Fri, 28 Jun 2024 15:01:04 -0700 Subject: [PATCH] [tool] when writing to openssl as a part of macOS/iOS code-signing, flush the stdin stream before closing it (#150120) Fixes https://github.com/flutter/flutter/issues/100584. Might help https://github.com/flutter/flutter/issues/137184. --- .../flutter_tools/lib/src/base/process.dart | 64 +++++++++++++++++-- .../lib/src/ios/code_signing.dart | 10 ++- .../test/general.shard/base/process_test.dart | 35 ++++++++++ .../general.shard/ios/code_signing_test.dart | 54 ++++++++++++++++ packages/flutter_tools/test/src/fakes.dart | 10 +++ 5 files changed, 166 insertions(+), 7 deletions(-) diff --git a/packages/flutter_tools/lib/src/base/process.dart b/packages/flutter_tools/lib/src/base/process.dart index d28770dc79..c55cd76a85 100644 --- a/packages/flutter_tools/lib/src/base/process.dart +++ b/packages/flutter_tools/lib/src/base/process.dart @@ -244,31 +244,83 @@ abstract class ProcessUtils { /// ``` /// /// However it did not catch a [SocketException] on Linux. + /// + /// As part of making sure errors are caught, this function will call [flush] + /// on [stdin] to ensure that [line] is written to the pipe before this + /// function returns. This means completion will be blocked if the kernel + /// buffer of the pipe is full. static Future writelnToStdinGuarded({ required IOSink stdin, required String line, required void Function(Object, StackTrace) onError, + }) async { + await _writeToStdinGuarded( + stdin: stdin, + content: line, + onError: onError, + isLine: true, + ); + } + + /// Please see [writelnToStdinGuarded]. + /// + /// This calls `stdin.write` instead of `stdin.writeln`. + static Future writeToStdinGuarded({ + required IOSink stdin, + required String content, + required void Function(Object, StackTrace) onError, + }) async { + await _writeToStdinGuarded( + stdin: stdin, + content: content, + onError: onError, + isLine: false, + ); + } + + static Future _writeToStdinGuarded({ + required IOSink stdin, + required String content, + required void Function(Object, StackTrace) onError, + required bool isLine, }) async { final Completer completer = Completer(); + void handleError(Object error, StackTrace stackTrace) { + try { + onError(error, stackTrace); + completer.complete(); + } on Exception catch (e) { + completer.completeError(e); + } + } + void writeFlushAndComplete() { - stdin.writeln(line); - stdin.flush().whenComplete(() { - if (!completer.isCompleted) { + if (isLine) { + stdin.writeln(content); + } else { + stdin.write(content); + } + stdin.flush().then( + (_) { completer.complete(); - } - }); + }, + onError: handleError, + ); } runZonedGuarded( writeFlushAndComplete, (Object error, StackTrace stackTrace) { - onError(error, stackTrace); + handleError(error, stackTrace); + + // We may have already completed with an error in `handleError`. if (!completer.isCompleted) { completer.complete(); } }, ); + return completer.future; } } diff --git a/packages/flutter_tools/lib/src/ios/code_signing.dart b/packages/flutter_tools/lib/src/ios/code_signing.dart index c819f55fa4..3358455a2c 100644 --- a/packages/flutter_tools/lib/src/ios/code_signing.dart +++ b/packages/flutter_tools/lib/src/ios/code_signing.dart @@ -231,7 +231,15 @@ Future _getCodeSigningIdentityDevelopmentTeam({ final Process opensslProcess = await processUtils.start( const ['openssl', 'x509', '-subject']); - await (opensslProcess.stdin..write(signingCertificateStdout)).close(); + + await ProcessUtils.writeToStdinGuarded( + stdin: opensslProcess.stdin, + content: signingCertificateStdout, + onError: (Object? error, _) { + throw Exception('Unexpected error when writing to openssl: $error'); + }, + ); + await opensslProcess.stdin.close(); final String opensslOutput = await utf8.decodeStream(opensslProcess.stdout); // Fire and forget discard of the stderr stream so we don't hold onto resources. diff --git a/packages/flutter_tools/test/general.shard/base/process_test.dart b/packages/flutter_tools/test/general.shard/base/process_test.dart index aa13e307c1..ad36e418da 100644 --- a/packages/flutter_tools/test/general.shard/base/process_test.dart +++ b/packages/flutter_tools/test/general.shard/base/process_test.dart @@ -2,6 +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 'package:flutter_tools/src/base/io.dart'; import 'package:flutter_tools/src/base/logger.dart'; import 'package:flutter_tools/src/base/platform.dart'; @@ -444,4 +446,37 @@ void main() { ); }); }); + + group('writeToStdinGuarded', () { + testWithoutContext('handles any error thrown by stdin.flush', () async { + final _ThrowsOnFlushIOSink stdin = _ThrowsOnFlushIOSink(); + Object? errorPassedToCallback; + + await ProcessUtils.writeToStdinGuarded( + stdin: stdin, + content: 'message to stdin', + onError: (Object error, StackTrace stackTrace) { + errorPassedToCallback = error; + }, + ); + + expect( + errorPassedToCallback, + isNotNull, + reason: 'onError callback should have been invoked.', + ); + + expect(errorPassedToCallback, const TypeMatcher()); + }); + }); +} + +class _ThrowsOnFlushIOSink extends MemoryIOSink { + @override + Future flush() async { + throw const SocketException( + 'Write failed', + osError: OSError('Broken pipe', 32), + ); + } } diff --git a/packages/flutter_tools/test/general.shard/ios/code_signing_test.dart b/packages/flutter_tools/test/general.shard/ios/code_signing_test.dart index c19cd0428d..385e84f9fe 100644 --- a/packages/flutter_tools/test/general.shard/ios/code_signing_test.dart +++ b/packages/flutter_tools/test/general.shard/ios/code_signing_test.dart @@ -15,6 +15,7 @@ import 'package:flutter_tools/src/ios/code_signing.dart'; import '../../src/common.dart'; import '../../src/fake_process_manager.dart'; +import '../../src/fakes.dart'; const String kCertificates = ''' 1) 86f7e437faa5a7fce15d1ddcb9eaeaea377667b8 "iPhone Developer: Profile 1 (1111AAAA11)" @@ -581,6 +582,59 @@ void main() { expect(developmentTeam, isNull); expect(processManager, hasNoRemainingExpectations); }); + + testWithoutContext('handles stdin pipe breaking on openssl process', () async { + final StreamSink> stdinSink = ClosedStdinController(); + + final Completer completer = Completer(); + const String certificates = ''' +1) 86f7e437faa5a7fce15d1ddcb9eaeaea377667b8 "iPhone Developer: Profile 1 (1111AAAA11)" + 1 valid identities found'''; + final FakeProcessManager processManager = FakeProcessManager.list([ + const FakeCommand( + command: ['which', 'security'], + ), + const FakeCommand( + command: ['which', 'openssl'], + ), + const FakeCommand( + command: ['security', 'find-identity', '-p', 'codesigning', '-v'], + stdout: certificates, + ), + const FakeCommand( + command: ['security', 'find-certificate', '-c', '1111AAAA11', '-p'], + stdout: 'This is a fake certificate', + ), + FakeCommand( + command: const ['openssl', 'x509', '-subject'], + stdin: IOSink(stdinSink), + stdout: 'subject= /CN=iPhone Developer: Profile 1 (1111AAAA11)/OU=3333CCCC33/O=My Team/C=US', + completer: completer, + ), + ]); + + Future?> getCodeSigningIdentities() => getCodeSigningIdentityDevelopmentTeamBuildSetting( + buildSettings: const { + 'bogus': 'bogus', + }, + platform: macosPlatform, + processManager: processManager, + logger: logger, + config: testConfig, + terminal: testTerminal, + ); + + await expectLater( + () => getCodeSigningIdentities(), + throwsA( + const TypeMatcher().having( + (Exception e) => e.toString(), + 'message', + equals('Exception: Unexpected error when writing to openssl: SocketException: Bad pipe'), + ), + ), + ); + }); }); } diff --git a/packages/flutter_tools/test/src/fakes.dart b/packages/flutter_tools/test/src/fakes.dart index b812679f98..d473e5b57c 100644 --- a/packages/flutter_tools/test/src/fakes.dart +++ b/packages/flutter_tools/test/src/fakes.dart @@ -737,3 +737,13 @@ class FakeDevtoolsLauncher extends Fake implements DevtoolsLauncher { closed = true; } } + +class ClosedStdinController extends Fake implements StreamSink> { + @override + Future addStream(Stream> stream) async => throw const SocketException('Bad pipe'); + + @override + Future close() async { + return null; + } +}