PE-9210: A share dialog that hangs, keys shown in the clear, and reads that never time out

lib/blocs/drive_attach/drive_attach_cubit.dart

@@ -90,6 +90,22 @@ class DriveAttachCubit extends Cubit<DriveAttachState> {
             );
           }
 
+          // A private share link no longer carries the drive's name - it is a
+          // secret, and it is recoverable from the drive's own record once the
+          // key is in hand. Resolve it here so the auto-attach below still
+          // has one.
+          //
+          // Deliberately resolved *before* `submit()` rather than inside it:
+          // `submit()` reads the name controller up front so that a name the
+          // user typed by hand is the one the drive is attached under, and
+          // moving that read later would silently discard it.
+          if (driveNameController.text.isEmpty &&
+              driveKeyController.text.isNotEmpty) {
+            await driveNameLoader();
+
+            if (isClosed) return;
+          }
+
           if (driveNameController.text.isNotEmpty &&
               driveKeyController.text.isNotEmpty) {
             submit();

lib/blocs/drive_share/drive_share_cubit.dart

@@ -2,6 +2,9 @@ import 'package:ardrive/blocs/blocs.dart';
 import 'package:ardrive/core/crypto/crypto.dart';
 import 'package:ardrive/models/models.dart';
 import 'package:ardrive/utils/link_generators.dart';
+import 'package:ardrive/utils/logger.dart';
+import 'package:ardrive_utils/ardrive_utils.dart';
+import 'package:arweave/utils.dart';
 import 'package:equatable/equatable.dart';
 import 'package:flutter/foundation.dart';
 import 'package:flutter_bloc/flutter_bloc.dart';
@@ -12,11 +15,21 @@ part 'drive_share_state.dart';
 class DriveShareCubit extends Cubit<DriveShareState> {
   final Drive drive;
 
+  /// The folder being shared, or `null` when the whole drive is.
+  final FolderID? folderId;
+
   final ProfileCubit _profileCubit;
   final DriveDao _driveDao;
 
+  /// Whether the built link embeds the drive key.
+  ///
+  /// Starts off. See [generatePrivateDriveShareLink] for why a drive key is
+  /// handed over separately by default.
+  bool _keyIsInLink = false;
+
   DriveShareCubit({
     required this.drive,
+    this.folderId,
     required DriveDao driveDao,
     required ProfileCubit profileCubit,
   })  : _driveDao = driveDao,
@@ -25,40 +38,94 @@ class DriveShareCubit extends Cubit<DriveShareState> {
     loadDriveShareDetails();
   }
 
+  /// Rebuilds the link with or without the key embedded in it.
+  Future<void> setKeyIsInLink(bool value) async {
+    if (value == _keyIsInLink) {
+      return;
+    }
+
+    _keyIsInLink = value;
+
+    await loadDriveShareDetails();
+  }
+
+  /// Builds the share link for [drive], or fails in a way the dialog can show.
+  ///
+  /// Everything here runs inside the guard on purpose. This method is called
+  /// from the constructor body and its future is never awaited, so anything it
+  /// throws becomes an unhandled asynchronous error: the cubit would stay in
+  /// [DriveShareLoadInProgress] and the dialog would spin forever, which is
+  /// exactly what a missing drive key used to do.
   Future<void> loadDriveShareDetails() async {
-    late Uri driveShareLink;
     emit(DriveShareLoadInProgress());
 
-    if (drive.isPrivate) {
-      DriveKey? driveKey;
-      if (_profileCubit.state is ProfileLoggedIn) {
-        final profileKey =
-            (_profileCubit.state as ProfileLoggedIn).user.cipherKey;
-        driveKey = await _driveDao.getDriveKey(drive.id, profileKey);
-      } else {
-        driveKey = await _driveDao.getDriveKeyFromMemory(drive.id);
-      }
-      if (driveKey != null) {
-        driveShareLink = await generatePrivateDriveShareLink(
-          driveId: drive.id,
-          driveName: drive.name,
-          driveKey: driveKey.key,
-        );
-      } else {
-        throw StateError('Drive key not found');
+    try {
+      final driveKey = drive.isPrivate ? await _driveKey() : null;
+
+      final driveShareLink = driveKey == null
+          ? generatePublicDriveShareLink(
+              driveId: drive.id,
+              driveName: drive.name,
+              folderId: folderId,
+            )
+          : await generatePrivateDriveShareLink(
+              driveId: drive.id,
+              driveKey: driveKey.key,
+              folderId: folderId,
+              includeKey: _keyIsInLink,
+            );
+
+      if (isClosed) {
+        return;
       }
-    } else {
-      driveShareLink = generatePublicDriveShareLink(
-        driveId: drive.id,
-        driveName: drive.name,
+
+      emit(
+        DriveShareLoadSuccess(
+          drive: drive,
+          driveShareLink: driveShareLink,
+          isFolder: folderId != null,
+          keyIsInLink: _keyIsInLink,
+          // The key is offered as its own artifact so the sharer can send it
+          // through a different channel than the link.
+          driveKeyBase64: driveKey == null
+              ? null
+              : encodeBytesToBase64(await driveKey.key.extractBytes()),
+        ),
+      );
+    } catch (e, stacktrace) {
+      // The drive id is safe to log; the key never is, and nothing here puts
+      // one in the message.
+      logger.e(
+        'Failed to build the share link for drive ${drive.id}',
+        e,
+        stacktrace,
       );
+
+      if (isClosed) {
+        return;
+      }
+
+      emit(const DriveShareLoadFail());
+    }
+  }
+
+  /// The private drive's key, which the link and the handover both need.
+  ///
+  /// It comes from the profile when one is signed in, and from the in-memory
+  /// store otherwise - a drive attached in this session but never persisted.
+  /// Neither is guaranteed, and a [StateError] here is a real outcome rather
+  /// than a should-never-happen: it lands on the failure state above.
+  Future<DriveKey> _driveKey() async {
+    final profileState = _profileCubit.state;
+
+    final driveKey = profileState is ProfileLoggedIn
+        ? await _driveDao.getDriveKey(drive.id, profileState.user.cipherKey)
+        : await _driveDao.getDriveKeyFromMemory(drive.id);
+
+    if (driveKey == null) {
+      throw StateError('Drive key not found');
     }
 
-    emit(
-      DriveShareLoadSuccess(
-        drive: drive,
-        driveShareLink: driveShareLink,
-      ),
-    );
+    return driveKey;
   }
 }

lib/blocs/drive_share/drive_share_state.dart

@@ -5,7 +5,7 @@ abstract class DriveShareState extends Equatable {
   const DriveShareState();
 
   @override
-  List<Object> get props => [];
+  List<Object?> get props => [];
 }
 
 /// [DriveShareLoadInProgress] means that the drive share details are being loaded.
@@ -18,23 +18,43 @@ class DriveShareLoadSuccess extends DriveShareState {
   /// The link to share access of this drive with.
   final Uri driveShareLink;
 
+  /// Whether the link points at one folder rather than the whole drive.
+  final bool isFolder;
+
+  /// Whether the drive key is embedded in [driveShareLink].
+  final bool keyIsInLink;
+
+  /// The drive key, for the sharer to hand over separately.
+  ///
+  /// `null` for a public drive, which has none.
+  final String? driveKeyBase64;
+
   const DriveShareLoadSuccess({
     required this.drive,
     required this.driveShareLink,
+    this.isFolder = false,
+    this.keyIsInLink = false,
+    this.driveKeyBase64,
   });
 
+  /// Whether the key travels as its own artifact rather than inside the link.
+  bool get hasSeparateKeyArtifact => driveKeyBase64 != null && !keyIsInLink;
+
   @override
-  List<Object> get props => [drive, driveShareLink];
+  List<Object?> get props => [
+        drive,
+        driveShareLink,
+        isFolder,
+        keyIsInLink,
+        driveKeyBase64,
+      ];
 }
 
 /// [DriveShareLoadFail] shows failiure states in the UI.
+///
+/// Carries no message: the dialog owns the copy so that it can be localized,
+/// which a cubit with no [BuildContext] cannot do. This is the same shape the
+/// file share dialog's failure states use.
 class DriveShareLoadFail extends DriveShareState {
-  final String message;
-
-  const DriveShareLoadFail({
-    required this.message,
-  });
-
-  @override
-  List<Object> get props => [message];
+  const DriveShareLoadFail();
 }

lib/blocs/shared_file/shared_file_cubit.dart

@@ -100,6 +100,35 @@ class SharedFileCubit extends Cubit<SharedFileState> {
 
   Future<void>? _backgroundWork;
 
+  /// The longest a single network read here may take before it is abandoned.
+  ///
+  /// The data path has been bounded for a long time - [DataGatewayFallback]
+  /// gives every fetch a request timeout, a total timeout and a hedge. The
+  /// GraphQL reads that run *in front of* it had nothing: [GraphQLRetry]
+  /// retries a call that fails, but sets no timeout, so a connection that
+  /// errors is retried and a connection that simply hangs is not. This page
+  /// would sit on its skeleton forever.
+  ///
+  /// Sized well above a healthy read and well below a recipient's patience.
+  /// Anything that trips it lands in the load failure state, which already
+  /// offers Retry.
+  static const defaultReadTimeout = Duration(seconds: 15);
+
+  final Duration _readTimeout;
+
+  /// Bounds [future], naming [what] so a timeout is legible in the log.
+  ///
+  /// A [TimeoutException] is deliberately left to propagate: every caller on
+  /// the critical path already handles a failed read, either by degrading to
+  /// another resolution path or by emitting the failure state.
+  Future<T> _bounded<T>(Future<T> future, String what) => future.timeout(
+        _readTimeout,
+        onTimeout: () => throw TimeoutException(
+          'Timed out after ${_readTimeout.inSeconds}s while $what',
+          _readTimeout,
+        ),
+      );
+
   SharedFileCubit({
     required this.fileId,
     this.fileKey,
@@ -109,10 +138,12 @@ class SharedFileCubit extends Cubit<SharedFileState> {
     required licenseService,
     ArDriveCrypto? crypto,
     Duration propagationRetryDelay = const Duration(seconds: 3),
+    Duration readTimeout = defaultReadTimeout,
   })  : _arweave = arweave,
         _licenseService = licenseService,
         _crypto = crypto ?? ArDriveCrypto(),
         _propagationRetryDelay = propagationRetryDelay,
+        _readTimeout = readTimeout,
         // A v2 link can paint its skeleton with the real name and size before
         // a single byte has been fetched.
         super(SharedFileLoadInProgress(payload: linkPayload)) {
@@ -276,9 +307,9 @@ class SharedFileCubit extends Cubit<SharedFileState> {
     emit(current.copyWith(activityStatus: SharedFileActivityStatus.loading));
 
     try {
-      final entities = await _arweave.getAllFileEntitiesWithId(
-        fileId,
-        fileKey,
+      final entities = await _bounded(
+        _arweave.getAllFileEntitiesWithId(fileId, fileKey),
+        'reading the file\'s version history',
       );
 
       if (_isStale(resolution)) {
@@ -357,7 +388,10 @@ class SharedFileCubit extends Cubit<SharedFileState> {
     FileEntity? latest;
 
     try {
-      latest = await _arweave.getLatestFileEntityWithId(fileId, fileKey);
+      latest = await _bounded(
+        _arweave.getLatestFileEntityWithId(fileId, fileKey),
+        'checking for a newer revision',
+      );
     } catch (e, stacktrace) {
       logger.e(
         'Failed to load the newest revision of the shared file',
@@ -596,7 +630,10 @@ class SharedFileCubit extends Cubit<SharedFileState> {
     _SharedRevision? shared;
 
     try {
-      shared = await _fetchSharedRevision(metadataTxId, fileKey);
+      shared = await _bounded(
+        _fetchSharedRevision(metadataTxId, fileKey),
+        'reading the metadata transaction the link names',
+      );
     } on EntityTransactionParseException {
       if (_isStale(resolution)) {
         return true;
@@ -725,7 +762,10 @@ class SharedFileCubit extends Cubit<SharedFileState> {
         emit(SharedFileLoadInProgress(payload: linkPayload));
       }
 
-      final privacy = await _arweave.getFilePrivacyForId(fileId);
+      final privacy = await _bounded(
+        _arweave.getFilePrivacyForId(fileId),
+        'looking up whether the file is private',
+      );
 
       if (_isStale(resolution)) {
         return;
@@ -735,9 +775,9 @@ class SharedFileCubit extends Cubit<SharedFileState> {
         _emitLocked(linkPayload);
         return;
       }
-      final allEntities = await _arweave.getAllFileEntitiesWithId(
-        fileId,
-        fileKey,
+      final allEntities = await _bounded(
+        _arweave.getAllFileEntitiesWithId(fileId, fileKey),
+        'reading the file\'s revisions',
       );
 
       if (_isStale(resolution)) {
@@ -764,9 +804,9 @@ class SharedFileCubit extends Cubit<SharedFileState> {
         // revisions are in reverse chronological order, so first is most recent
         final target = _targetRevision(revisions);
         final latestLicense = target.licenseTxId != null
-            ? await fetchLicenseForRevision(
-                target,
-                owner: ownerAddress,
+            ? await _bounded(
+                fetchLicenseForRevision(target, owner: ownerAddress),
+                'reading the file\'s license',
               )
             : null;