fix: detect native changes in Windows patches (#2758)

cspell.config.yaml

@@ -80,6 +80,7 @@ words:
   - propertylistserialization
   - pubspec
   - pwsh
+  - rdata
   - reactivecircus # From .github dir, doesn't show up in "**" check?
   - readlink
   - reinit

packages/shorebird_cli/lib/src/archive_analysis/byte_utils.dart

@@ -0,0 +1,14 @@
+import 'dart:typed_data';
+
+/// Reads a 16-bit integer as a little-endian value from the provided bytes.
+int readInt16(Uint8List bytes, int offset) {
+  return bytes[offset + 1] << 8 | bytes[offset];
+}
+
+/// Reads a 32-bit integer as a little-endian value from the provided bytes.
+int readInt32(Uint8List bytes, int offset) {
+  return bytes[offset + 3] << 24 |
+      bytes[offset + 2] << 16 |
+      bytes[offset + 1] << 8 |
+      bytes[offset];
+}

packages/shorebird_cli/lib/src/archive_analysis/macho.dart

@@ -1,24 +1,18 @@
 import 'dart:io';
 import 'dart:typed_data';
 
+import 'package:shorebird_cli/src/archive_analysis/byte_utils.dart';
+
 const _machOHeaderSize = 32;
 const _uuidLoadCommandType = 0x1b;
 
 /// Utilities for interacting with Mach-O files.
 /// See https://en.wikipedia.org/wiki/Mach-O.
 class MachO {
-  /// Reads a 32-bit integer as a little-endian value from the provided bytes.
-  static int _readInt32(Uint8List bytes, int offset) {
-    return bytes[offset + 3] << 24 |
-        bytes[offset + 2] << 16 |
-        bytes[offset + 1] << 8 |
-        bytes[offset];
-  }
-
   /// Returns `true` if the provided file is a Mach-O file.
   static bool isMachOFile(File file) {
     final bytes = file.readAsBytesSync();
-    final magic = _readInt32(bytes, 0);
+    final magic = readInt32(bytes, 0);
 
     // These are the magic numbers for Mach-O files.
     // See https://en.wikipedia.org/wiki/Mach-O#Mach-O_header
@@ -35,13 +29,13 @@ class MachO {
     // The number of load commands is a 32-bit int at offset 16. We could
     // probably write a more robust MachO header parser, but this is all we need
     // for now.
-    final numberOfLoadCommands = _readInt32(bytes, 16);
+    final numberOfLoadCommands = readInt32(bytes, 16);
 
     // The load commands are immediately after the header.
     var offset = _machOHeaderSize;
     for (var i = 0; i < numberOfLoadCommands; i++) {
-      final commandType = _readInt32(bytes, offset);
+      final commandType = readInt32(bytes, offset);
-      final commandLength = _readInt32(bytes, offset + 4);
+      final commandLength = readInt32(bytes, offset + 4);
 
       if (commandType == _uuidLoadCommandType) {
         // Zero out the UUID bytes.

packages/shorebird_cli/lib/src/archive_analysis/portable_executable.dart

@@ -1,22 +1,93 @@
 import 'dart:io';
 import 'dart:typed_data';
 
+import 'package:shorebird_cli/src/archive_analysis/byte_utils.dart';
+
 /// Utilities for interacting with Windows Portable Executable files.
 class PortableExecutable {
   /// Zeroes out the timestamps in the provided PE file to enable comparison of
   /// binaries with different build times.
   ///
+  /// Flutter app EXEs include the build time as a timestamp twice in the file.
+  /// We need to zero these out so we can check for actual binary differences.
+  ///
   /// Timestamps are DWORD (4-byte) values at:
-  ///   1. offset 0x110 in the PE header.
+  ///   1. In the PE header. The offset of this header is always at 0x3c in the
-  ///   2. offset 0x6e14 (seems to be in section 1, need to figure out a robust
+  ///      file, and the timestamp is at offset 0x8 from the start of the PE.
-  ///     way to find this).
+  ///   2. In the .rdata section. I have not yet found a precise way to
+  ///      determine the timestamp's offset in this section, so we read through
+  ///      the section in 4-byte increments and zero out any DWORDs that match
+  ///      the timestamp from the PE header.
   ///
   /// https://learn.microsoft.com/en-us/windows/win32/debug/pe-format#coff-file-header-object-and-image
   static Uint8List bytesWithZeroedTimestamps(File file) {
     final bytes = file.readAsBytesSync();
-    final timestampLocations = [0x110, 0x6e14];
+
-    for (final location in timestampLocations) {
+    // https://learn.microsoft.com/en-us/windows/win32/debug/pe-format#signature-image-only
-      bytes.setRange(location, location + 4, List.filled(4, 0));
+    //
+    // PE files have a 4-byte "signature" that identifies them as PE files. This
+    // signature is at the offset specified at 0x3c, and the PE header starts
+    // after this.
+    //
+    // The PE header is 20 bytes long and contains the following fields:
+    //   0x0: 2 bytes for machine
+    //   0x2: 2 bytes for number of sections
+    //   0x4: 4 bytes for time date stamp
+    //   0x8: 4 bytes for pointer to symbol table
+    //   0xc: 4 bytes for number of symbols
+    //   0x10: 2 bytes for size of optional header
+    const peHeaderSize = 0x14;
+    final signatureOffset = readInt32(bytes, 0x3c);
+    final peHeaderOffset = signatureOffset + 0x4;
+    final numSections = readInt16(bytes, peHeaderOffset + 0x2);
+    final peHeaderTimestampOffset = peHeaderOffset + 0x4;
+    final optionalHeaderSize = readInt16(bytes, peHeaderOffset + 0x10);
+    final peHeaderTimestamp = readInt32(bytes, peHeaderTimestampOffset);
+    // Zero out the first timestamp
+    bytes.setRange(
+      peHeaderTimestampOffset,
+      peHeaderTimestampOffset + 4,
+      List.filled(4, 0),
+    );
+
+    // After the PE header is the optional header, which is of variable size.
+    // It does not contain any information we currently care about, so we skip
+    // that and proceed to the section table, which follows the optional header.
+    //
+    // Each section header is 40 (0x28) bytes long.
+    //
+    // https://learn.microsoft.com/en-us/windows/win32/debug/pe-format#section-table-section-headers
+    const sectionHeaderSize = 0x28;
+    final sectionHeadersOffset =
+        peHeaderOffset + peHeaderSize + optionalHeaderSize;
+
+    int? rawDataOffset;
+    int? rawDataSize;
+    for (var i = 0; i < numSections; i++) {
+      final sectionOffset = sectionHeadersOffset + i * sectionHeaderSize;
+      final sectionName = String.fromCharCodes(
+        bytes.sublist(sectionOffset, sectionOffset + 8),
+      );
+      if (sectionName.startsWith('.rdata')) {
+        rawDataSize = readInt32(bytes, sectionOffset + 0x10);
+        rawDataOffset = readInt32(bytes, sectionOffset + 0x14);
+        break;
+      }
+    }
+
+    // If we could not find an .rdata section, that means that this PE file is
+    // likely malformed.
+    if (rawDataOffset == null || rawDataSize == null) {
+      throw Exception('Could not find .rdata section');
+    }
+
+    // Iterate through the .rdata section and zero out any instances of the
+    // timestamp from the PE header that we find.
+    for (var i = 0; i < rawDataSize; i += 4) {
+      final currentOffset = rawDataOffset + i;
+      if (readInt32(bytes, currentOffset) == peHeaderTimestamp) {
+        bytes.setRange(currentOffset, currentOffset + 4, List.filled(4, 0));
+      }
     }
 
     return bytes;

packages/shorebird_cli/lib/src/archive_analysis/windows_archive_differ.dart

@@ -36,11 +36,8 @@ class WindowsArchiveDiffer extends ArchiveDiffer {
 
   @override
   bool isNativeFilePath(String filePath) {
-    // TODO(bryanoltman): reenable this check once we can reliably report
+    const nativeFileExtensions = ['.dll', '.exe'];
-    // native diffs
+    return nativeFileExtensions.contains(p.extension(filePath));
-    // const nativeFileExtensions = ['.dll', '.exe'];
-    // return nativeFileExtensions.contains(p.extension(filePath));
-    return false;
   }
 
   @override

packages/shorebird_cli/lib/src/platform/windows.dart

@@ -14,7 +14,7 @@ final minimumSupportedWindowsFlutterVersion = Version(3, 27, 2);
 
 /// Revisions of Flutter 3.27.1 that support windows.
 const windowsFlutterGitHashesBelowMinVersion = {
-  '56228c343d6c7fd3e1e548dbb290f9713bb22aa9'
+  '56228c343d6c7fd3e1e548dbb290f9713bb22aa9',
 };
 
 /// A warning message printed at the start of `shorebird release windows` and

packages/shorebird_cli/test/src/archive_analysis/portable_executable_test.dart

@@ -0,0 +1,75 @@
+import 'dart:io';
+
+import 'package:archive/archive_io.dart';
+import 'package:path/path.dart' as p;
+import 'package:shorebird_cli/src/archive_analysis/portable_executable.dart';
+import 'package:test/test.dart';
+
+void main() {
+  group(PortableExecutable, () {
+    group('when no .rdata section exists', () {
+      late File file;
+
+      setUp(() {
+        final tempDir = Directory.systemTemp.createTempSync();
+        file = File(p.join(tempDir.path, 'my.exe'))
+          ..writeAsBytesSync(List.filled(1000, 0));
+      });
+
+      test('throws exception', () {
+        expect(
+          () => PortableExecutable.bytesWithZeroedTimestamps(file),
+          throwsA(
+            isA<Exception>().having(
+              (e) => e.toString(),
+              'string value',
+              'Exception: Could not find .rdata section',
+            ),
+          ),
+        );
+      });
+    });
+
+    group('when given a valid exe', () {
+      late File file;
+      final winArchivesFixturesBasePath =
+          p.join('test', 'fixtures', 'win_archives');
+      final releasePath = p.join(
+        winArchivesFixturesBasePath,
+        'release.zip',
+      );
+
+      setUp(() async {
+        final tempDir = Directory.systemTemp.createTempSync();
+        final inputStream = InputFileStream(releasePath);
+        final archive = ZipDecoder().decodeStream(inputStream);
+        await extractArchiveToDisk(archive, tempDir.path);
+        file = File(p.join(tempDir.path, 'hello_windows.exe'));
+      });
+
+      test('zeroes out timestamps', () {
+        // Known locations of timestamps in this executable
+        const timestampLocations = [0x110, 0x6e14];
+
+        final beforeBytes = file.readAsBytesSync();
+        final afterBytes = PortableExecutable.bytesWithZeroedTimestamps(file);
+        expect(
+          beforeBytes.sublist(timestampLocations[0], timestampLocations[0] + 4),
+          isNot(equals([0, 0, 0, 0])),
+        );
+        expect(
+          beforeBytes.sublist(timestampLocations[1], timestampLocations[1] + 4),
+          isNot(equals([0, 0, 0, 0])),
+        );
+        expect(
+          afterBytes.sublist(timestampLocations[0], timestampLocations[0] + 4),
+          equals([0, 0, 0, 0]),
+        );
+        expect(
+          afterBytes.sublist(timestampLocations[1], timestampLocations[1] + 4),
+          equals([0, 0, 0, 0]),
+        );
+      });
+    });
+  });
+}

packages/shorebird_cli/test/src/archive_analysis/windows_archive_differ_test.dart

@@ -42,32 +42,28 @@ void main() {
       });
     });
 
-    group(
+    group('isNativeFilePath', () {
-      'isNativeFilePath',
+      group('when file extension is .dll', () {
-      () {
+        test('returns true', () {
-        group('when file extension is .dll', () {
+          final result = differ.isNativeFilePath('foo.dll');
-          test('returns true', () {
+          expect(result, isTrue);
-            final result = differ.isNativeFilePath('foo.dll');
-            expect(result, isTrue);
-          });
         });
+      });
 
-        group('when file extension is .exe', () {
+      group('when file extension is .exe', () {
-          test('returns true', () {
+        test('returns true', () {
-            final result = differ.isNativeFilePath('foo.exe');
+          final result = differ.isNativeFilePath('foo.exe');
-            expect(result, isTrue);
+          expect(result, isTrue);
-          });
         });
+      });
 
-        group('when file extension is not .dll or .exe', () {
+      group('when file extension is not .dll or .exe', () {
-          test('returns false', () {
+        test('returns false', () {
-            final result = differ.isNativeFilePath('foo.so');
+          final result = differ.isNativeFilePath('foo.so');
-            expect(result, isFalse);
+          expect(result, isFalse);
-          });
         });
-      },
+      });
-      skip: 'Disabled until we can reliably report native diffs',
+    });
-    );
 
     group('changedFiles', () {
       final winArchivesFixturesBasePath =

packages/shorebird_cli/test/src/commands/release/release_command_test.dart

@@ -12,7 +12,6 @@ import 'package:shorebird_cli/src/common_arguments.dart';
 import 'package:shorebird_cli/src/config/config.dart';
 import 'package:shorebird_cli/src/logging/logging.dart';
 import 'package:shorebird_cli/src/metadata/metadata.dart';
-import 'package:shorebird_cli/src/platform/macos.dart';
 import 'package:shorebird_cli/src/platform/platform.dart';
 import 'package:shorebird_cli/src/release_type.dart';
 import 'package:shorebird_cli/src/shorebird_env.dart';

packages/shorebird_cli/test/src/commands/release/windows_releaser_test.dart

@@ -250,7 +250,8 @@ For more information see: ${supportedFlutterVersionsUrl.toLink()}''',
             when(
               () => shorebirdFlutter.getRevisionForVersion(any()),
             ).thenAnswer(
-                (_) async => windowsFlutterGitHashesBelowMinVersion.first);
+              (_) async => windowsFlutterGitHashesBelowMinVersion.first,
+            );
           });
 
           test('completes normally', () async {