From 52a0f80980510a0c0459468c5870dd5d33858744 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 22 Jul 2024 23:21:26 -0700 Subject: [PATCH] compose: Include content-type in image-upload requests Fixes: #829 --- lib/widgets/compose_box.dart | 34 +++++++++++++++++++++++++----- test/widgets/compose_box_test.dart | 6 ++++++ 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/lib/widgets/compose_box.dart b/lib/widgets/compose_box.dart index abd634dda4..35c54936bd 100644 --- a/lib/widgets/compose_box.dart +++ b/lib/widgets/compose_box.dart @@ -4,6 +4,7 @@ import 'package:flutter/material.dart'; import 'package:flutter/services.dart'; import 'package:flutter_gen/gen_l10n/zulip_localizations.dart'; import 'package:image_picker/image_picker.dart'; +import 'package:mime/mime.dart'; import '../api/exception.dart'; import '../api/model/model.dart'; @@ -423,11 +424,17 @@ class _FixedDestinationContentInput extends StatelessWidget { /// A convenience class to represent data from the generic file picker, /// the media library, and the camera, in a single form. class _File { - _File({required this.content, required this.length, required this.filename}); + _File({ + required this.content, + required this.length, + required this.filename, + required this.mimeType, + }); final Stream> content; final int length; final String filename; + final String? mimeType; } Future _uploadFiles({ @@ -474,14 +481,14 @@ Future _uploadFiles({ } for (final (tag, file) in uploadsInProgress) { - final _File(:content, :length, :filename) = file; + final _File(:content, :length, :filename, :mimeType) = file; Uri? url; try { final result = await uploadFile(store.connection, content: content, length: length, filename: filename, - mimeType: null, // TODO(#829) + mimeType: mimeType, ); url = Uri.parse(result.uri); } catch (e) { @@ -579,7 +586,17 @@ Future> _getFilePickerFiles(BuildContext context, FileType type) return result.files.map((f) { assert(f.readStream != null); // We passed `withReadStream: true` to pickFiles. - return _File(content: f.readStream!, length: f.size, filename: f.name); + String? mimeType; + if (f.path != null) { + mimeType = lookupMimeType(f.path!, + headerBytes: f.bytes?.take(defaultMagicNumbersMaxLength).toList()); + } + return _File( + content: f.readStream!, + length: f.size, + filename: f.name, + mimeType: mimeType, + ); }); } @@ -664,7 +681,14 @@ class _AttachFromCameraButton extends _AttachUploadsButton { } final length = await result.length(); - return [_File(content: result.openRead(), length: length, filename: result.name)]; + return [_File( + content: result.openRead(), + length: length, + filename: result.name, + mimeType: result.mimeType + // TODO pass headerBytes (what's a good way to get that?) + ?? lookupMimeType(result.path), + )]; } } diff --git a/test/widgets/compose_box_test.dart b/test/widgets/compose_box_test.dart index 53c8c86559..01b608b1c8 100644 --- a/test/widgets/compose_box_test.dart +++ b/test/widgets/compose_box_test.dart @@ -281,6 +281,8 @@ void main() { testBinding.pickFilesResult = FilePickerResult([PlatformFile( readStream: Stream.fromIterable(['asdf'.codeUnits]), + // TODO test inference of mimeType from initial bytes, when + // it can't be inferred from path path: '/private/var/mobile/Containers/Data/Application/foo/tmp/image.jpg', name: 'image.jpg', size: 12345, @@ -304,6 +306,7 @@ void main() { ..field.equals('file') ..length.equals(12345) ..filename.equals('image.jpg') + ..contentType.asString.equals('image/jpeg') ..has>>((f) => f.finalize().toBytes(), 'contents') .completes((it) => it.deepEquals(['asdf'.codeUnits].expand((l) => l))) ); @@ -328,6 +331,8 @@ void main() { composeBoxController.topicController!.value = const TextEditingValue(text: 'some topic'); testBinding.pickImageResult = XFile.fromData( + // TODO test inference of mimeType when it's missing here + mimeType: 'image/jpeg', utf8.encode('asdf'), name: 'image.jpg', length: 12345, @@ -352,6 +357,7 @@ void main() { ..field.equals('file') ..length.equals(12345) ..filename.equals('image.jpg') + ..contentType.asString.equals('image/jpeg') ..has>>((f) => f.finalize().toBytes(), 'contents') .completes((it) => it.deepEquals(['asdf'.codeUnits].expand((l) => l))) );