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

Check filesize before uploading file to server and display alert if file is too big #7778

Closed
wants to merge 3 commits into from
Closed
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
2 changes: 2 additions & 0 deletions Riot/Assets/en.lproj/Vector.strings
Original file line number Diff line number Diff line change
Expand Up @@ -1715,6 +1715,8 @@ Tap the + to start adding people.";
// MARK: File upload
"file_upload_error_title" = "File upload";
"file_upload_error_unsupported_file_type_message" = "File type not supported.";
"file_upload_error_too_large_title" = "File too large";
"file_upload_error_too_large_message" = "Maximum supported file size is %@MB";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you checked what this looks like? Now that there's an NSByteCountFormatter feeding the alert, it should already contain the appropriate unit formatted appropriately in the string.


// MARK: Emoji picker
"emoji_picker_title" = "Reactions";
Expand Down
3 changes: 3 additions & 0 deletions Riot/Assets/fr.lproj/Vector.strings
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't make changes to the translations directly in the project - these should be added on https://translate.element.io once the PR is merged, otherwise we run the risk of handling conflicts.

Original file line number Diff line number Diff line change
Expand Up @@ -778,6 +778,9 @@
// MARK: File upload
"file_upload_error_title" = "Envoi de fichier";
"file_upload_error_unsupported_file_type_message" = "Type de fichier non pris en charge.";
"file_upload_error_too_large_title" = "Fichier trop lourd";
"file_upload_error_too_large_message" = "La taille maximum autorisée est %@Mo";

"auth_softlogout_signed_out" = "Vous êtes déconnecté";
"auth_softlogout_sign_in" = "Se connecter";
"auth_softlogout_reason" = "L’administrateur de votre serveur d’accueil (%1$@) vous a déconnecté de votre compte %2$@ (%3$@).";
Expand Down
7 changes: 7 additions & 0 deletions Riot/Modules/MatrixKit/Models/Room/MXKRoomDataSource.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,13 @@ typedef enum : NSUInteger

} MXKRoomDataSourceBubblesPagination;

// Check filesize before sending: make RoomDataSource errors public
typedef NS_ENUM (NSUInteger, MXKRoomDataSourceError) {
MXKRoomDataSourceErrorResendGeneric = 10001,
MXKRoomDataSourceErrorResendInvalidMessageType = 10002,
MXKRoomDataSourceErrorResendInvalidLocalFilePath = 10003,
MXKRoomDataSourceErrorCantSendFileToBig = 10004, // Check filesize before sending: file to big error
};

#pragma mark - Cells identifiers

Expand Down
93 changes: 88 additions & 5 deletions Riot/Modules/MatrixKit/Models/Room/MXKRoomDataSource.m
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,12 @@

NSString * const MXKRoomDataSourceErrorDomain = @"kMXKRoomDataSourceErrorDomain";

typedef NS_ENUM (NSUInteger, MXKRoomDataSourceError) {
MXKRoomDataSourceErrorResendGeneric = 10001,
MXKRoomDataSourceErrorResendInvalidMessageType = 10002,
MXKRoomDataSourceErrorResendInvalidLocalFilePath = 10003,
};
// Check filesize before sending: make RoomDataSource errors public
//typedef NS_ENUM (NSUInteger, MXKRoomDataSourceError) {
// MXKRoomDataSourceErrorResendGeneric = 10001,
// MXKRoomDataSourceErrorResendInvalidMessageType = 10002,
// MXKRoomDataSourceErrorResendInvalidLocalFilePath = 10003,
//};


@interface MXKRoomDataSource ()
Expand Down Expand Up @@ -1923,6 +1924,67 @@ - (BOOL)canReplyToEventWithId:(NSString*)eventIdToReply
return [self.room canReplyToEvent:eventToReply];
}

// Check filesize before sending: check file size before sending
- (BOOL)_isFilesizeOkToBeSent:(NSUInteger)filesize
{
// Check maxUploadSize accepted by the home server before trying to upload.
NSUInteger maxUploadFileSize = self.mxSession.maxUploadSize;
if (filesize > maxUploadFileSize)
{
return NO;
}
Comment on lines +1932 to +1935
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question again here:

Have you checked if this takes any encryption overhead into account? Might be worth building a bit of leniency into here.

else
{
return YES;
}
}

- (BOOL)isFilesizeOkToBeSentForData:(NSData *)fileData
{
return [self _isFilesizeOkToBeSent:fileData.length];
}

- (BOOL)isFilesizeOkToBeSentForLocalFileUrl:(NSURL *)localFileUrl
{
NSDictionary *fileAttributes = [NSFileManager.defaultManager attributesOfItemAtPath:localFileUrl.path error:nil];
if (fileAttributes)
{
return [self _isFilesizeOkToBeSent:fileAttributes.fileSize];
}
else
{
return NO;
}
}

- (BOOL)isFilesizeOkToBeSentForLocalAVAsset:(AVAsset *)asset
{
// Check if asset points to a local file
if( ![asset isKindOfClass:AVURLAsset.class] )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our objective-c style should look like so:

Suggested change
if( ![asset isKindOfClass:AVURLAsset.class] )
if (![asset isKindOfClass:AVURLAsset.class])

There's a few of these in here.

{
// If asset doesn't point to a local asset, we can't check size.
// Return YES to let the upload happens and get the result of the backend.
return YES;
}

AVURLAsset *urlAsset = (AVURLAsset *)asset;
NSNumber *assetFilesize;
NSError *error;

// Try to get asset filesize.
[urlAsset.URL getResourceValue:&assetFilesize forKey:NSURLFileSizeKey error:&error];

// If we can't check size,
if( error != NULL || assetFilesize == NULL )
{
// return YES to let the upload happens and get the result of the backend.
return YES;
}

return [self _isFilesizeOkToBeSent:assetFilesize.unsignedLongValue];
}


- (void)sendImage:(NSData *)imageData mimeType:(NSString *)mimetype success:(void (^)(NSString *))success failure:(void (^)(NSError *))failure
{
UIImage *image = [UIImage imageWithData:imageData];
Expand All @@ -1944,6 +2006,13 @@ - (void)sendImage:(NSData *)imageData mimeType:(NSString *)mimetype success:(voi

- (void)sendImageData:(NSData*)imageData withImageSize:(CGSize)imageSize mimeType:(NSString*)mimetype andThumbnail:(UIImage*)thumbnail success:(void (^)(NSString *eventId))success failure:(void (^)(NSError *error))failure
{
// Check filesize before sending: check fielsize before trying to send file
if( ![self isFilesizeOkToBeSentForData:imageData] )
{
failure([NSError errorWithDomain:MXKRoomDataSourceErrorDomain code:MXKRoomDataSourceErrorCantSendFileToBig userInfo:nil]);
return;
}

__block MXEvent *localEchoEvent = nil;

[_room sendImage:imageData withImageSize:imageSize mimeType:mimetype andThumbnail:thumbnail threadId:self.threadId localEcho:&localEchoEvent success:success failure:failure];
Expand All @@ -1964,6 +2033,13 @@ - (void)sendVideo:(NSURL *)videoLocalURL withThumbnail:(UIImage *)videoThumbnail

- (void)sendVideoAsset:(AVAsset *)videoAsset withThumbnail:(UIImage *)videoThumbnail success:(void (^)(NSString *))success failure:(void (^)(NSError *))failure
{
// Check filesize before sending: check fielsize before trying to send file
if( ![self isFilesizeOkToBeSentForLocalAVAsset:videoAsset] )
{
failure([NSError errorWithDomain:MXKRoomDataSourceErrorDomain code:MXKRoomDataSourceErrorCantSendFileToBig userInfo:nil]);
return;
}

__block MXEvent *localEchoEvent = nil;

[_room sendVideoAsset:videoAsset withThumbnail:videoThumbnail threadId:self.threadId localEcho:&localEchoEvent success:success failure:failure];
Comment on lines +2037 to 2045
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @NicolasBuquet, thanks for the update. To my eyes this one still appears to be too early: MXRoom.sendVideoAsset will convert the video meaning that the check above would block some files from sending that would have been fine.

Expand Down Expand Up @@ -2013,6 +2089,13 @@ - (void)sendVoiceMessage:(NSURL *)audioFileLocalURL

- (void)sendFile:(NSURL *)fileLocalURL mimeType:(NSString*)mimeType success:(void (^)(NSString *))success failure:(void (^)(NSError *))failure
{
// Check filesize before sending: check fielsize before trying to send file
if( ![self isFilesizeOkToBeSentForLocalFileUrl:fileLocalURL] )
{
failure([NSError errorWithDomain:MXKRoomDataSourceErrorDomain code:MXKRoomDataSourceErrorCantSendFileToBig userInfo:nil]);
return;
}

__block MXEvent *localEchoEvent = nil;

[_room sendFile:fileLocalURL mimeType:mimeType threadId:self.threadId localEcho:&localEchoEvent success:success failure:failure];
Expand Down
19 changes: 18 additions & 1 deletion Riot/Modules/Room/RoomViewController.m
Original file line number Diff line number Diff line change
Expand Up @@ -7747,6 +7747,17 @@ - (void)documentPickerPresenter:(MXKDocumentPickerPresenter *)presenter didPickD
}
}

// Check filesize before sending: if send file error is "File too big", display alert box to user
- (void)displayAlertIfErrorIsFileIsTooBig:(NSError *)error
{
if( error.code == MXKRoomDataSourceErrorCantSendFileToBig )
{
NSUInteger maxUploadFileSize = self.roomDataSource.mxSession.maxUploadSize;
[self showAlertWithTitle:[VectorL10n fileUploadErrorTooLargeTitle]
message:[VectorL10n fileUploadErrorTooLargeMessage:[NSByteCountFormatter stringFromByteCount:maxUploadFileSize countStyle:NSByteCountFormatterCountStyleFile]]];
}
}

- (void)sendImage:(NSData *)imageData mimeType:(NSString *)mimeType {
// Create before sending the message in case of a discussion (direct chat)
MXWeakify(self);
Expand All @@ -7758,7 +7769,9 @@ - (void)sendImage:(NSData *)imageData mimeType:(NSString *)mimeType {
[self.roomDataSource sendImage:imageData mimeType:mimeType success:nil failure:^(NSError *error) {
// Nothing to do. The image is marked as unsent in the room history by the datasource
MXLogDebug(@"[MXKRoomViewController] sendImage failed.");
}];
// Check filesize before sending: if error is "FileTooBig", display alert box.
[self displayAlertIfErrorIsFileIsTooBig:error];
}];
}
// Errors are handled at the request level. This should be improved in case of code rewriting.
}];
Expand All @@ -7775,6 +7788,8 @@ - (void)sendVideo:(NSURL * _Nonnull)url {
[(RoomDataSource*)self.roomDataSource sendVideo:url success:nil failure:^(NSError *error) {
// Nothing to do. The video is marked as unsent in the room history by the datasource
MXLogDebug(@"[MXKRoomViewController] sendVideo failed.");
// Check filesize before sending: if error is "FileTooBig", display alert box.
[self displayAlertIfErrorIsFileIsTooBig:error];
}];
}
// Errors are handled at the request level. This should be improved in case of code rewriting.
Expand All @@ -7792,6 +7807,8 @@ - (void)sendFile:(NSURL * _Nonnull)url mimeType:(NSString *)mimeType {
[self.roomDataSource sendFile:url mimeType:mimeType success:nil failure:^(NSError *error) {
// Nothing to do. The file is marked as unsent in the room history by the datasource
MXLogDebug(@"[MXKRoomViewController] sendFile failed.");
// Check filesize before sending: if error is "FileTooBig", display alert box.
[self displayAlertIfErrorIsFileIsTooBig:error];
}];
}
// Errors are handled at the request level. This should be improved in case of code rewriting.
Expand Down
1 change: 1 addition & 0 deletions changelog.d/pr-7778.change
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Check filesize before uploading file to server and display alert if file is too big.
Loading