Skip to content

Commit f78526c

Browse files
arthuraleefacebook-github-bot
authored andcommitted
Avoid re-encoding images when uploading local files (#31457)
Summary: Fixes #27099 When you upload a local file using XHR + the `FormData` API, RN uses `RCTNetworkTask` to retrieve the image file data from the local filesystem (request URL is a file:// URL) ([code pointer](https://github.com/facebook/react-native/blob/master/Libraries/Network/RCTNetworking.mm#L398)). As a result, if you are uploading a local image file that is in the app's directory `RCTNetworkTask` will end up using `RCTLocalAssetImageLoader` to load the image, which reads the image into a `UIImage` and then re-encodes it using `UIImageJPEGRepresentation` with a compression quality of 1.0, which is the higest ([code pointer](https://github.com/facebook/react-native/blob/4c5182c1cc8bafb15490adf602c87cb5bf289ffd/Libraries/Image/RCTImageLoader.mm#L1114)). Not only is this unnecessary, it ends up inflating the size of the jpg if it had been previously compressed to a lower quality. With this PR, this issue is fixed by forcing the `RCTFileRequestHandler` to be used when retrieving local files for upload, regardless of whether they are images or not. As a result, any file to be uploaded gets read into `NSData` which is the format needed when appending to the multipart body. I considered fixing this by modifying the behavior of how the handlers were chosen, but this felt like a safer fix since it will be scoped to just uploads and wont affect image fetching. ## Changelog [iOS] [Fixed] - Avoid re-encoding images when uploading local files Pull Request resolved: #31457 Test Plan: The repro for this is a bit troublesome, especially because this issue doesn't repro in RNTester. There is [some code](https://github.com/facebook/react-native/blob/master/packages/rn-tester/RNTester/AppDelegate.mm#L220) that is to be overriding the handlers that will be used, excluding the `RCTImageLoader`. I had to repro this in a fresh new RN app. 1. Create a blank RN app 2. Put an image in the folder of the app's install location. This would be similar to where files might be placed after an app downloads or captures an image. 3. Set up a quick express server that accepts multipart form uploads and stores the files 4. Trigger an upload via react native ``` const data = new FormData(); data.append('image', { uri: '/Users/arthur.lee/Library/Developer/CoreSimulator/Devices/46CDD981 (d0c8cb12f21604fd9730e275a52816d7fd00a826)-9164-4925-9025-1A76C0D9 (1946aee3d9696384d38890269ea705cafd472827)F0F5/data/Containers/Bundle/Application/B1E8A764-6221-4EA9-BE9A-2CB1699FD218 (1c92b1cff623ea3f3b78238b146ab001626ef305)/test.app/test.bundle/compressed.jpg', type: 'image/jpeg', name: 'image.jpeg', }); fetch(`http://localhost:3000/upload`, { method: 'POST', headers: {'Content-Type': 'multipart/form-data'}, body: data, }).then(console.log); ``` 5. Trigger the upload with and without this patch Original file: ``` $ ls -lh total 448 -rw-r--r-- 1 arthur.lee staff 223K Apr 29 17:08 compressed.jpg ``` Uploaded file (with and without patch): ``` $ ls -lh total 1624 -rw-r--r--@ 1 arthur.lee staff 584K Apr 29 17:11 image-nopatch.jpeg -rw-r--r--@ 1 arthur.lee staff 223K Apr 29 17:20 image-withpatch.jpeg ``` Would appreciate pointers on whether this needs to be tested more extensively Reviewed By: yungsters Differential Revision: D28630805 Pulled By: PeteTheHeat fbshipit-source-id: 606a6091fa3e817966548c5eb84b19cb8b9abb1c
1 parent f12f0e6 commit f78526c

File tree

2 files changed

+27
-2
lines changed

2 files changed

+27
-2
lines changed

Libraries/Network/RCTNetworking.h

+2
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@
4545
- (RCTNetworkTask *)networkTaskWithRequest:(NSURLRequest *)request
4646
completionBlock:(RCTURLRequestCompletionBlock)completionBlock;
4747

48+
- (RCTNetworkTask *)networkTaskWithRequest:(NSURLRequest *)request handler:(id<RCTURLRequestHandler>)handler completionBlock:(RCTURLRequestCompletionBlock)completionBlock;
49+
4850
- (void)addRequestHandler:(id<RCTNetworkingRequestHandler>)handler;
4951

5052
- (void)addResponseHandler:(id<RCTNetworkingResponseHandler>)handler;

Libraries/Network/RCTNetworking.mm

+25-2
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#import <React/RCTUtils.h>
1818

1919
#import <React/RCTHTTPRequestHandler.h>
20+
#import <React/RCTFileRequestHandler.h>
2021

2122
#import "RCTNetworkPlugins.h"
2223

@@ -154,6 +155,7 @@ @implementation RCTNetworking
154155
}
155156

156157
@synthesize methodQueue = _methodQueue;
158+
@synthesize moduleRegistry = _moduleRegistry;
157159

158160
RCT_EXPORT_MODULE()
159161

@@ -188,6 +190,14 @@ - (void)invalidate
188190
_responseHandlers = nil;
189191
}
190192

193+
// TODO (T93136931) - Investigate why this is needed. This setter shouldn't be
194+
// necessary, since moduleRegistry is a property on RCTEventEmitter (which this
195+
// class inherits from).
196+
- (void)setModuleRegistry:(RCTModuleRegistry *)moduleRegistry
197+
{
198+
_moduleRegistry = moduleRegistry;
199+
}
200+
191201
- (NSArray<NSString *> *)supportedEvents
192202
{
193203
return @[@"didCompleteNetworkResponse",
@@ -393,9 +403,9 @@ - (RCTURLRequestCancellationBlock)processDataForHTTPQuery:(nullable NSDictionary
393403
}
394404
NSURLRequest *request = [RCTConvert NSURLRequest:query[@"uri"]];
395405
if (request) {
396-
397406
__block RCTURLRequestCancellationBlock cancellationBlock = nil;
398-
RCTNetworkTask *task = [self networkTaskWithRequest:request completionBlock:^(NSURLResponse *response, NSData *data, NSError *error) {
407+
id<RCTURLRequestHandler> handler = [self.moduleRegistry moduleForName:"FileRequestHandler"];
408+
RCTNetworkTask *task = [self networkTaskWithRequest:request handler:handler completionBlock:^(NSURLResponse *response, NSData *data, NSError *error) {
399409
dispatch_async(self->_methodQueue, ^{
400410
cancellationBlock = callback(error, data ? @{@"body": data, @"contentType": RCTNullIfNil(response.MIMEType)} : nil);
401411
});
@@ -676,6 +686,19 @@ - (RCTNetworkTask *)networkTaskWithRequest:(NSURLRequest *)request completionBlo
676686
return task;
677687
}
678688

689+
- (RCTNetworkTask *)networkTaskWithRequest:(NSURLRequest *)request handler:(id<RCTURLRequestHandler>)handler completionBlock:(RCTURLRequestCompletionBlock)completionBlock
690+
{
691+
if (!handler) {
692+
// specified handler is nil, fall back to generic method
693+
return [self networkTaskWithRequest:request completionBlock:completionBlock];
694+
}
695+
RCTNetworkTask *task = [[RCTNetworkTask alloc] initWithRequest:request
696+
handler:handler
697+
callbackQueue:_methodQueue];
698+
task.completionBlock = completionBlock;
699+
return task;
700+
}
701+
679702
#pragma mark - JS API
680703

681704
RCT_EXPORT_METHOD(sendRequest:(JS::NativeNetworkingIOS::SpecSendRequestQuery &)query

0 commit comments

Comments
 (0)