Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed an issue where sharing attachments could lose filename and extension #343

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Session/Conversations/ConversationVC+Interaction.swift
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ extension ConversationVC:
}

let fileName = urlResourceValues.name ?? "attachment".localized()
guard let dataSource = DataSourcePath(fileUrl: url, shouldDeleteOnDeinit: false) else {
guard let dataSource = DataSourcePath(fileUrl: url, sourceFilename: urlResourceValues.name, shouldDeleteOnDeinit: false) else {
DispatchQueue.main.async { [weak self] in
self?.viewModel.showToast(text: "attachmentsErrorLoad".localized())
}
Expand Down Expand Up @@ -412,7 +412,7 @@ extension ConversationVC:

func showAttachmentApprovalDialogAfterProcessingVideo(at url: URL, with fileName: String) {
ModalActivityIndicatorViewController.present(fromViewController: self, canCancel: true, message: nil) { [weak self, dependencies = viewModel.dependencies] modalActivityIndicator in
let dataSource = DataSourcePath(fileUrl: url, shouldDeleteOnDeinit: false)!
let dataSource = DataSourcePath(fileUrl: url, sourceFilename: fileName, shouldDeleteOnDeinit: false)!
dataSource.sourceFilename = fileName

SignalAttachment
Expand Down Expand Up @@ -2594,7 +2594,7 @@ extension ConversationVC:
}

// Get data
let dataSourceOrNil = DataSourcePath(fileUrl: audioRecorder.url, shouldDeleteOnDeinit: true)
let dataSourceOrNil = DataSourcePath(fileUrl: audioRecorder.url, sourceFilename: nil, shouldDeleteOnDeinit: true)
self.audioRecorder = nil

guard let dataSource = dataSourceOrNil else { return SNLog("Couldn't load recorded data.") }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ class GifPickerViewController: OWSViewController, UISearchBarDelegate, UICollect
return
}

let dataSource = DataSourcePath(filePath: asset.filePath, shouldDeleteOnDeinit: false)
let dataSource = DataSourcePath(filePath: asset.filePath, sourceFilename: URL(fileURLWithPath: asset.filePath).pathExtension, shouldDeleteOnDeinit: false)
let attachment = SignalAttachment.attachment(dataSource: dataSource, type: rendition.type, imageQuality: .medium)

self?.dismiss(animated: true) {
Expand Down
2 changes: 1 addition & 1 deletion Session/Media Viewing & Editing/PhotoCapture.swift
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ extension PhotoCapture: CaptureOutputDelegate {
Log.debug("[PhotoCapture] Ignoring error, since capture succeeded.")
}

let dataSource = DataSourcePath(fileUrl: outputFileURL, shouldDeleteOnDeinit: true)
let dataSource = DataSourcePath(fileUrl: outputFileURL, sourceFilename: nil, shouldDeleteOnDeinit: true)
let attachment = SignalAttachment.attachment(dataSource: dataSource, type: .mpeg4Movie)
delegate?.photoCapture(self, didFinishProcessingAttachment: attachment)
}
Expand Down
2 changes: 1 addition & 1 deletion Session/Media Viewing & Editing/PhotoLibrary.swift
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ class PhotoCollectionContents {

guard
exportSession?.status == .completed,
let dataSource = DataSourcePath(fileUrl: exportURL, shouldDeleteOnDeinit: true)
let dataSource = DataSourcePath(fileUrl: exportURL, sourceFilename: nil, shouldDeleteOnDeinit: true)
else {
resolver(Result.failure(PhotoLibraryError.assertionError(description: "Failed to build data source for exported video URL")))
return
Expand Down
16 changes: 9 additions & 7 deletions Session/Settings/HelpViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -169,13 +169,15 @@ class HelpViewModel: SessionTableViewModel, NavigatableStateHolder, ObservableTa
cancelTitle: "Share",
cancelStyle: .alert_text,
onConfirm: { _ in UIPasteboard.general.string = latestLogFilePath },
onCancel: { _ in
HelpViewModel.shareLogsInternal(
viewControllerToDismiss: viewControllerToDismiss,
targetView: targetView,
animated: animated,
onShareComplete: onShareComplete
)
onCancel: { modal in
modal.dismiss(animated: true) {
HelpViewModel.shareLogsInternal(
viewControllerToDismiss: viewControllerToDismiss,
targetView: targetView,
animated: animated,
onShareComplete: onShareComplete
)
}
}
)
)
Expand Down
7 changes: 5 additions & 2 deletions SessionMessagingKit/Database/Models/Attachment.swift
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,10 @@ extension Attachment {
// If the filename has not file extension, deduce one
// from the MIME type.
if targetFileExtension.isEmpty {
targetFileExtension = (UTType(sessionMimeType: mimeType)?.sessionFileExtension ?? UTType.fileExtensionDefault)
targetFileExtension = (
UTType(sessionMimeType: mimeType)?.sessionFileExtension(sourceFilename: sourceFilename) ??
UTType.fileExtensionDefault
)
}

targetFileExtension = targetFileExtension.lowercased()
Expand All @@ -657,7 +660,7 @@ extension Attachment {
}

let targetFileExtension: String = (
UTType(sessionMimeType: mimeType)?.sessionFileExtension ??
UTType(sessionMimeType: mimeType)?.sessionFileExtension(sourceFilename: sourceFilename) ??
UTType.fileExtensionDefault
).lowercased()

Expand Down
8 changes: 6 additions & 2 deletions SessionMessagingKit/Database/Models/LinkPreview.swift
Original file line number Diff line number Diff line change
Expand Up @@ -132,12 +132,16 @@ public extension LinkPreview {

static func generateAttachmentIfPossible(imageData: Data?, type: UTType) throws -> Attachment? {
guard let imageData: Data = imageData, !imageData.isEmpty else { return nil }
guard let fileExtension: String = type.sessionFileExtension else { return nil }
guard let fileExtension: String = type.sessionFileExtension(sourceFilename: nil) else { return nil }
guard let mimeType: String = type.preferredMIMEType else { return nil }

let filePath = FileSystem.temporaryFilePath(fileExtension: fileExtension)
try imageData.write(to: NSURL.fileURL(withPath: filePath), options: .atomicWrite)
let dataSource: DataSourcePath = DataSourcePath(filePath: filePath, shouldDeleteOnDeinit: true)
let dataSource: DataSourcePath = DataSourcePath(
filePath: filePath,
sourceFilename: nil,
shouldDeleteOnDeinit: true
)

return Attachment(contentType: mimeType, dataSource: dataSource)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ public class SignalAttachment: Equatable {
// can be identified.
public var mimeType: String {
guard
let fileExtension: String = sourceFilename.map({ $0 as NSString })?.pathExtension,
let fileExtension: String = sourceFilename.map({ URL(fileURLWithPath: $0) })?.pathExtension,
!fileExtension.isEmpty,
let fileExtensionMimeType: String = UTType(sessionFileExtension: fileExtension)?.preferredMIMEType
else { return (dataType.preferredMIMEType ?? UTType.mimeTypeDefault) }
Expand Down Expand Up @@ -306,9 +306,9 @@ public class SignalAttachment: Equatable {
// can be identified.
public var fileExtension: String? {
guard
let fileExtension: String = sourceFilename.map({ $0 as NSString })?.pathExtension,
let fileExtension: String = sourceFilename.map({ URL(fileURLWithPath: $0) })?.pathExtension,
!fileExtension.isEmpty
else { return dataType.sessionFileExtension }
else { return dataType.sessionFileExtension(sourceFilename: sourceFilename) }

return fileExtension.filteredFilename
}
Expand Down Expand Up @@ -803,7 +803,7 @@ public class SignalAttachment: Equatable {
let baseFilename = dataSource.sourceFilename
let mp4Filename = baseFilename?.filenameWithoutExtension.appendingFileExtension("mp4")

guard let dataSource = DataSourcePath(fileUrl: exportURL, shouldDeleteOnDeinit: true) else {
guard let dataSource = DataSourcePath(fileUrl: exportURL, sourceFilename: baseFilename, shouldDeleteOnDeinit: true) else {
let attachment = SignalAttachment(dataSource: DataSourceValue.empty, dataType: type)
attachment.error = .couldNotConvertToMpeg4
resolver(Result.success(attachment))
Expand Down
10 changes: 4 additions & 6 deletions SessionShareExtension/ShareNavController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -274,16 +274,14 @@ final class ShareNavController: UINavigationController, ShareViewDelegate {
//
// NOTE: SharingThreadPickerViewController will try to unpack them
// and send them as normal text messages if possible.
case (_, true): return DataSourcePath(fileUrl: url, shouldDeleteOnDeinit: false)
case (_, true):
return DataSourcePath(fileUrl: url, sourceFilename: customFileName, shouldDeleteOnDeinit: false)

default:
guard let dataSource = DataSourcePath(fileUrl: url, shouldDeleteOnDeinit: false) else {
guard let dataSource = DataSourcePath(fileUrl: url, sourceFilename: customFileName, shouldDeleteOnDeinit: false) else {
return nil
}

// Fallback to the last part of the URL
dataSource.sourceFilename = (customFileName ?? url.lastPathComponent)

return dataSource
}
}
Expand Down Expand Up @@ -429,7 +427,7 @@ final class ShareNavController: UINavigationController, ShareViewDelegate {
switch value {
case let data as Data:
let customFileName = "Contact.vcf" // stringlint:ignore
let customFileExtension: String? = srcType.sessionFileExtension
let customFileExtension: String? = srcType.sessionFileExtension(sourceFilename: nil)

guard let tempFilePath = try? FileSystem.write(data: data, toTemporaryFileWithExtension: customFileExtension) else {
resolver(
Expand Down
21 changes: 17 additions & 4 deletions SessionUtilitiesKit/Media/DataSource.swift
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public class DataSourceValue: DataSource {
}

public convenience init?(data: Data?, dataType: UTType) {
guard let fileExtension: String = dataType.sessionFileExtension else { return nil }
guard let fileExtension: String = dataType.sessionFileExtension(sourceFilename: nil) else { return nil }

self.init(data: data, fileExtension: fileExtension)
}
Expand Down Expand Up @@ -210,15 +210,28 @@ public class DataSourcePath: DataSource {

// MARK: - Initialization

public init(filePath: String, shouldDeleteOnDeinit: Bool) {
public init(
filePath: String,
sourceFilename: String?,
shouldDeleteOnDeinit: Bool
) {
self.filePath = filePath
self.sourceFilename = sourceFilename
self.shouldDeleteOnDeinit = shouldDeleteOnDeinit
}

public convenience init?(fileUrl: URL?, shouldDeleteOnDeinit: Bool) {
public convenience init?(
fileUrl: URL?,
sourceFilename: String?,
shouldDeleteOnDeinit: Bool
) {
guard let fileUrl: URL = fileUrl, fileUrl.isFileURL else { return nil }

self.init(filePath: fileUrl.path, shouldDeleteOnDeinit: shouldDeleteOnDeinit)
self.init(
filePath: fileUrl.path,
sourceFilename: (sourceFilename ?? fileUrl.lastPathComponent),
shouldDeleteOnDeinit: shouldDeleteOnDeinit
)
}

deinit {
Expand Down
52 changes: 32 additions & 20 deletions SessionUtilitiesKit/Media/UTType+Utilities.swift
Original file line number Diff line number Diff line change
Expand Up @@ -107,26 +107,6 @@ public extension UTType {
var isMicrosoftDoc: Bool { UTType.supportedMicrosoftDocTypes.contains(self) }
var isVisualMedia: Bool { isImage || isVideo || isAnimated }

var sessionFileExtension: String? {
// Special-case the "aac" filetype we use for voice messages (for legacy reasons)
// to use a .m4a file extension, not .aac, since AVAudioPlayer can't handle .aac
// properly. Doesn't affect file contents.
guard identifier != "public.aac-audio" else { return "m4a" }

// Try to deduce the file extension by using a lookup table.
//
// This should be more accurate than deducing the file extension by
// converting to a UTI type. For example, .m4a files will have a
// UTI type of kUTTypeMPEG4Audio which incorrectly yields the file
// extension .mp4 instead of .m4a.
guard
let mimeType: String = preferredMIMEType,
let fileExtension: String = UTType.genericMimeTypesToExtensionTypes[mimeType]
else { return preferredFilenameExtension }

return fileExtension
}

// MARK: - Initialization

init?(sessionFileExtension: String) {
Expand Down Expand Up @@ -188,6 +168,38 @@ public extension UTType {
return (UTType(sessionMimeType: mimeType) ?? .invalid).isVisualMedia
}

func sessionFileExtension(sourceFilename: String?) -> String? {
// First try to check if the file extension on `sourceFilename` is valid
// for the `preferredMIMEType` as we want to keep that if so (eg. use `.log`
// instead of `.txt`)
if
let sourceFileExtension: String = sourceFilename.map({ URL(fileURLWithPath: $0) })?.pathExtension,
let mimeType: String = preferredMIMEType,
let sourceExtensionMimeType: String = UTType.genericMimeTypesToExtensionTypes[sourceFileExtension],
UTType(mimeType: sourceExtensionMimeType)?.preferredMIMEType == mimeType
{
return sourceFileExtension
}

// Special-case the "aac" filetype we use for voice messages (for legacy reasons)
// to use a .m4a file extension, not .aac, since AVAudioPlayer can't handle .aac
// properly. Doesn't affect file contents.
guard identifier != "public.aac-audio" else { return "m4a" }

// Try to deduce the file extension by using a lookup table.
//
// This should be more accurate than deducing the file extension by
// converting to a UTI type. For example, .m4a files will have a
// UTI type of kUTTypeMPEG4Audio which incorrectly yields the file
// extension .mp4 instead of .m4a.
guard
let mimeType: String = preferredMIMEType,
let fileExtension: String = UTType.genericMimeTypesToExtensionTypes[mimeType]
else { return preferredFilenameExtension }

return fileExtension
}

// MARK: - Lookup Table

static func sessionMimeType(for fileExtension: String) -> String? {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,7 @@ public class AttachmentApprovalViewController: UIPageViewController, UIPageViewC
// Rewrite the filename's extension to reflect the output file format.
var filename: String? = attachmentItem.attachment.sourceFilename
if let sourceFilename = attachmentItem.attachment.sourceFilename {
if let fileExtension: String = dataType.sessionFileExtension {
if let fileExtension: String = dataType.sessionFileExtension(sourceFilename: sourceFilename) {
filename = (sourceFilename as NSString).deletingPathExtension.appendingFileExtension(fileExtension)
}
}
Expand Down