ionic-team / capacitor

Build cross-platform Native Progressive Web Apps for iOS, Android, and the Web ⚑️
https://capacitorjs.com
MIT License
12.22k stars 1.01k forks source link

bug: CapacitorHttp - Requesting a Blob fails on native iOS and Android #6126

Closed KevinKelchen closed 9 months ago

KevinKelchen commented 1 year ago

Bug Report

Capacitor Version

πŸ’Š   Capacitor Doctor  πŸ’Š 

Latest Dependencies:

  @capacitor/cli: 4.5.0
  @capacitor/core: 4.5.0
  @capacitor/android: 4.5.0
  @capacitor/ios: 4.5.0

Installed Dependencies:

  @capacitor/cli: 4.5.0
  @capacitor/android: 4.5.0
  @capacitor/core: 4.5.0
  @capacitor/ios: 4.5.0

[success] iOS looking great! πŸ‘Œ
[success] Android looking great! πŸ‘Œ

Platform(s)

Native iOS Native Android

Current Behavior

Hello there! πŸ‘‹

Using the Angular HttpClient, which uses CapacitorHttp at a lower level, making a request to retrieve a Blob on native iOS and Android fails. Native iOS without CapacitorHttp, Native Android without CapacitorHttp, and Web work fine.

In our production app, we use a POST request to retrieve a Blob, but using a GET seems to fail as well and was easier to use in a repro.

Expected Behavior

Native iOS and native Android should successfully retrieve the requested Blob.

Also, if we want CapacitorHttp to work the same as XHR/fetch, this would be a difference.

Code Reproduction

https://github.com/KevinKelchen/capacitor-http-request-blob-issue#steps-to-reproduce

Other Technical Details

npm --version output: 8.15.0

node --version output: v16.17.0

pod --version output (iOS issues only): 1.11.3

Additional Context

Thanks so much! πŸ˜€

Kevin

silviogutierrez commented 1 year ago

I can confirm this is still an issue with Capacitor 4.6.1

silviogutierrez commented 1 year ago

@KevinKelchen : on further investigation, it's less about blob() and more about requesting anything that isn't a text/utf8 type file.

So if I fetch an JSON endpoint and call blob, it works. But if I fetch an image, to save it locally, and call blob (or text() or any other method) it fails.

beonde commented 1 year ago

Phew... I thought I was the only one going crazy with this bug! Can also confirm in capacitor/core 4.6.1.

I have even tried 'arraybuffer' for output and converting, but no dice.

markabrahams commented 1 year ago

Without analysing quite enough yet, it does seem I am striking a related or at least very similar issue (I'm using capacitor/core 4.7.0). With the CapacitorHttp plugin enabled (have only tested android at the moment), I'm fetching a PDF as an arraybuffer using the Angular HttpClient with code like this:

const headers = (new HttpHeaders()).set('Accept', 'application/pdf');
const myPdf = await lastValueFrom(this.httpClient.get<Buffer>(
    `http://localhost:3000/api/v1/pdf`,
     { headers: headers, responseType: 'arraybuffer' as 'json'}
));

And I get logs like this:

D/Capacitor/Console: File: http://localhost/ - Line 538 - Msg: CapacitorHttp XMLHttpRequest 1677857407746 http://localhost:3000/api/v1/pdf: 275.464111328125 ms
I/Capacitor/Console: File: http://localhost/main.d03a8937da65ac3f.js - Line 1 - Msg: Error: Response is not an ArrayBuffer.

and get thrown an empty error object, which to me suggests that the HTTP GET part has succeeded, but the subsequent return as an ArrayBuffer object has failed.

The same Angular HttpClient code works fine with the CapacitorHttp plugin disabled (and server CORS enabled), or in a browser.

Debugging the successful httpClient.get in a Chrome browser confirms that myPdf as returned by the Angular HttpClient is indeed an ArrayBuffer object.

(Sidenote: The Typescript stuff seems a bit screwy for this, hence the weird as 'json' casting and use of the Buffer generic instead of ArrayBuffer, but these are unrelated TypeScript issues, as proven by the working cases.)

mariusbolik commented 1 year ago

I have the same problem like @markabrahams! With capacitorHttp disabled, everything works fine! But with capacitorHttp enabled, I am not able to open a downloaded PDF on android!

This is the error shown in Android Studio:


E/Capacitor/Plugin: Failed to connect to localhost/127.0.0.1:80
    java.net.ConnectException: Failed to connect to localhost/127.0.0.1:80
        at com.android.okhttp.internal.io.RealConnection.connectSocket(RealConnection.java:147)
        at com.android.okhttp.internal.io.RealConnection.connect(RealConnection.java:116)
        at com.android.okhttp.internal.http.StreamAllocation.findConnection(StreamAllocation.java:186)
        at com.android.okhttp.internal.http.StreamAllocation.findHealthyConnection(StreamAllocation.java:128)
        at com.android.okhttp.internal.http.StreamAllocation.newStream(StreamAllocation.java:97)
        at com.android.okhttp.internal.http.HttpEngine.connect(HttpEngine.java:289)
        at com.android.okhttp.internal.http.HttpEngine.sendRequest(HttpEngine.java:232)
        at com.android.okhttp.internal.huc.HttpURLConnectionImpl.execute(HttpURLConnectionImpl.java:465)
        at com.android.okhttp.internal.huc.HttpURLConnectionImpl.connect(HttpURLConnectionImpl.java:131)
        at com.getcapacitor.plugin.util.CapacitorHttpUrlConnection.connect(CapacitorHttpUrlConnection.java:234)
        at com.getcapacitor.plugin.util.HttpRequestHandler.request(HttpRequestHandler.java:410)
        at com.getcapacitor.plugin.CapacitorHttp$1.run(CapacitorHttp.java:35)
        at java.lang.Thread.run(Thread.java:1012)
D/Capacitor: Sending plugin error: {"save":false,"callbackId":"109383765","pluginId":"CapacitorHttp","methodName":"request","success":false,"error":{"message":"Failed to connect to localhost\/127.0.0.1:80","code":"ConnectException"}}
D/Capacitor/Console: File: http://localhost/ - Line 407 - Msg: CapacitorHttp fetch 1678198976081 http://localhost/_capacitor_file_/data/user/0/de.btcecho.app/files/magazine/ausgabe-66-dezember-2022.pdf: 11.80615234375 ms
E/Capacitor/Console: File: http://localhost/polyfills.js - Line 1220 - Msg: Unhandled Promise rejection: Failed to connect to localhost/127.0.0.1:80 ; Zone: <root> ; Task: null ; Value: Error: Failed to connect to localhost/127.0.0.1:80 Error: Failed to connect to localhost/127.0.0.1:80
        at returnResult (http://localhost/:763:32)
        at win.androidBridge.onmessage (http://localhost/:738:21)

This is the Error shown in the Google Chrome console:

polyfills.js:1220 Unhandled Promise rejection: Failed to connect to localhost/127.0.0.1:80 ; Zone: <root> ; Task: null ; Value: Error: Failed to connect to localhost/127.0.0.1:80
    at returnResult (VM3:759:32)
    at win.androidBridge.onmessage (VM3:734:21) Error: Failed to connect to localhost/127.0.0.1:80
    at returnResult (http://localhost/:763:32)
    at win.androidBridge.onmessage (http://localhost/:738:21)

This is the function I am calling:

import * as pdfjsLib from 'pdfjs-dist';
...
pdfjsLib.getDocument({
    url,
    cMapUrl: C_MAP_URL,
    cMapPacked: true,
    enableXfa: true
});
mariusbolik commented 1 year ago

Seems like local requests through the native bridge using Capacitor.convertFileSrc(uri) are working on iOS because the URLs start with capacitor:// and aren't patched:

https://github.com/ionic-team/capacitor/blob/e486672731818d5c64c50956562aa4766f169d41/core/native-bridge.ts#L413-L416

So maybe we have 2 issues here:

  1. Blobs are not working with CpacitorHttp
  2. Requests for local files should possibly not be patched
MGX-CODING commented 1 year ago

Count me in too on this issue

I have to make HTTP GET requests to fetch images with authorization headers (so I can't use the src attribute of img tags)

When using Angular's HTTP client, everything works well

When Capacitor HTTP is used (for CORS reasons), then everything stops working

Related code :

    let url = 'some URL';
    return this.headers$.pipe(
      switchMap((headers) =>
        // Works
        this.http.get<any>(url, { headers, responseType: 'arraybuffer' })
        // Workn't
        /* from(
          CapacitorHttp.get({
            url,
            headers,
            responseType: 'arraybuffer',
          }).then((response) => response.data)
        ) */
      )
    );
MGX-CODING commented 1 year ago

As a workaround, one of my colleagues found this neat trick for images :

CapacitorHttp.get({
    url,
    headers,
    responseType: 'blob', // as any if needed
})
.then((response) =>  'data:image/png;base64,' + response.data))

Didn't tested for other MIME types, but this one seems to work for images. Use the return value as a img[src] value, should display the image correctly.

beonde commented 1 year ago

As a workaround, one of my colleagues found this neat trick for images :

CapacitorHttp.get({
    url,
    headers,
    responseType: 'blob', // as any if needed
})
.then((response) =>  'data:image/png;base64,' + response.data))

Didn't tested for other MIME types, but this one seems to work for images. Use the return value as a img[src] value, should display the image correctly.

Just something to watch out for with this solution. It actually may point to what is happening internally causing the error this issue is about. You are getting back a base64 encoded string of the image, when it should be a blob. If the Capacitor team ever fix this issue your implementation of the get method above will break.

kwolfy commented 1 year ago

The problem is here https://github.com/ionic-team/capacitor/blob/4b039f93d4c0b6b0665429cd573c073286734fc0/core/src/core-plugins.ts#L372-L376

@ItsChaceDI see that it was you who added that line. Can you remember please why you convert blob and arraybuffer to base64 ?

kwolfy commented 1 year ago

I can make a PR with correct behavior in case of blob and arrayBuffer, but it's important to understand if it won't break something internally.

And most importantly, I think it should be at least a minor update since users might have problems

markabrahams commented 1 year ago

Thanks @kwolfy - you're absolutely right!

I can confirm success in downloading PDFs as arraybuffers if the Base64 encoding is removed.

The native Android HttpRequestHandler.java is encoding arraybuffers and blobs as Base64 here: https://github.com/ionic-team/capacitor/blob/51a548b97129998de8c403b2a8eb0421135af812/android/capacitor/src/main/java/com/getcapacitor/plugin/util/HttpRequestHandler.java#L244-L248

And for iOS HttpRequestHandler.swift here: https://github.com/ionic-team/capacitor/blob/51a548b97129998de8c403b2a8eb0421135af812/ios/Capacitor/Capacitor/Plugins/HttpRequestHandler.swift#L134-L137

If I remove that Base64 encoding and just use UTF-8 string encoding to pass the arraybuffer/blob from the native handler to the JS CapacitorHttp plugin, I receive the arraybuffer fine. e.g. for Android in HttpRequestHandler.java replacing: return Base64.encodeToString(result, 0, result.length, Base64.DEFAULT); with just UTF-8 encoding of the byte array: return new String(result, StandardCharsets.UTF_8);

and on the CapacitorHTTP plugin side removing the Base64 handling in core-plugin.ts i.e. for arraybuffer and blob cases replace: data = await readBlobAsBase64(blob); with just: data = blob;

@ItsChaceD - what is your take on this? Would a PR for such a change be accepted?

I can't see this breaking anything, as I can't see that arraybuffer or blob requests would ever work (unless a client assumed they would receive Base64 encoded blobs/arraybuffers, but that would be a very odd assumption for mind!) Importantly, this would align the arraybuffer/blob result using the CapacitorHttp plugin with the result without the plugin enabled, which is the goal for the plugin.

nadavhalfon commented 1 year ago

this "fix" is making problems of requesting local data on native platforms.

For exmaple also loading - window.open(data:text/calendar;charset=utf8,${encodeURI(iCal)}, '_self'); or by using the Browser API / Capacitor http is causing for domain errors and block.

I think Capacitor should create a better solution maybe, I am not sure why this PR was made (the one I tagged)

chingham commented 1 year ago

No fix for this yet ?

suguruwataru commented 1 year ago

Found this issue where the original author explains this behavior: https://github.com/capacitor-community/http/issues/176 Given the technical difficulty I think it's acceptable that it keeps returning b64. It's not like I'm doing some heavy computing and this extra encoding/decoding creates a huge problem for me. Someone else might be doing that though. However this definitely should be documented. Currently the doc has no mentioning on usage of b64, and .data happens to be any. We ask for blob, there is a Blob type, of course we expect it to be Blob. Nonetheless it returns string. And now we are left to guess what the string is.

goforu commented 1 year ago

any updates?

stochmalm commented 1 year ago

Requesting blob/arraybuffer via http client should return blob/arraybuffer, not some kind of base64 string. If it's a technical problem, a conversion to proper type should be done on capacitor side. It's misleading and makes using external components like ngx-extended-pdf-viewer or ng-lazy-load-image surprisingly broken. I already lost a few hours when debugging why PDFs and images does not load on mobile.

louis123562 commented 1 year ago

idk how people can work with this for months...

WillooWisp commented 1 year ago

I can confirm the issue with this code as well. It works in the browser, but not on device, neither Android or iOS. One difference between browser and device is also that in the browser the blob in the response body already has the correct content type specified, but not in device, where it is always application/json and you have to manually first create the blob with the correct content type, but in browser it is possible to pass the response body directly to createObjectURL.

private loadImage(url: string): Observable<SafeUrl> {
        return this.http.get(url, { responseType: 'blob', observe: 'response' })
            .pipe(
                filter(response => response.status === 200),
                map(response => {
                    let contentType = response.headers.get('Content-Type');
                    const blob = new Blob([response.body], { type: contentType });
                    this.objectUrl = URL.createObjectURL(blob);
                    return this.domSanitizer.bypassSecurityTrustUrl(this.objectUrl);
                }));
    }
heyrex commented 10 months ago

Here's a git diff showing our workaround for Capacitor v4. A quick look at the latest code in the master branch makes me think this would work for newer versions too.

diff --git a/node_modules/@capacitor/ios/Capacitor/Capacitor/assets/native-bridge.js b/node_modules/@capacitor/ios/Capacitor/Capacitor/assets/native-bridge.js
index 4c08c8d0..a7784e74 100644
--- a/node_modules/@capacitor/ios/Capacitor/Capacitor/assets/native-bridge.js
+++ b/node_modules/@capacitor/ios/Capacitor/Capacitor/assets/native-bridge.js
@@ -577,6 +577,7 @@ var nativeBridge = (function (exports) {
                                     data: data !== null ? data : undefined,
                                     headers: Object.assign(Object.assign({}, headers), otherHeaders),
                                     dataType: type,
+                                    responseType: this.responseType, /* Added by Heyrex to un-break responseType == 'blob' */
                                 })
                                     .then((nativeResponse) => {
                                     // intercept & parse response before returning
@@ -585,10 +586,27 @@ var nativeBridge = (function (exports) {
                                         this._headers = nativeResponse.headers;
                                         this.status = nativeResponse.status;
                                         this.response = nativeResponse.data;
-                                        this.responseText =
-                                            typeof nativeResponse.data === 'string'
-                                                ? nativeResponse.data
-                                                : JSON.stringify(nativeResponse.data);
+
+                                        /* Added by Heyrex to un-break responseType == 'blob' */
+                                        if (this.responseType === '' ||
+                                            this.responseType === 'text') {
+                                            this.response =
+                                                typeof nativeResponse.data !== 'string'
+                                                    ? JSON.stringify(nativeResponse.data)
+                                                    : nativeResponse.data;
+                                            this.responseText = this.response;
+                                        }
+                                        else if(this.responseType === 'blob'){ //Added by Heyrex to unbreak responseType == 'blob'
+                                            this.response = atob(nativeResponse.data); //Base64 decode
+                                            this.response = Uint8Array.from(this.response, c => c.charCodeAt(0)); //Convert to an "itterable" type accepted by Blob constructor
+                                            this.response = new Blob([this.response], { type: "application/octet-stream" });
+                                            this.responseText = null;
+                                        } else {
+                                            this.response = nativeResponse.data;
+                                            this.responseText = null;
+                                        }
+                                        /* End of code added by Heyrex */
+
                                         this.responseURL = nativeResponse.url;
                                         this.readyState = 4;
                                         this.dispatchEvent(new Event('load'));
mrtnrs commented 9 months ago

This issue also occurs when using Mapbox gl js; when it tries to load the 3D layers as arrayBuffer, it fails

function loadGLTF(url ) { return fetch(url) .then(response => response.arrayBuffer()) .then(buffer => decodeGLTF(buffer, 0, url)); }

Error: Could not load model maple2-lod4 from https://api.mapbox.com/models/v1/mapbox/maple2-lod4.glb... byte length of Uint32Array should be a multiple of 4

ionitron-bot[bot] commented 8 months ago

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Capacitor, please create a new issue and ensure the template is fully filled out.