Skip to content
This repository has been archived by the owner on Sep 26, 2022. It is now read-only.

buildResponse doesn't catch IOException #3

Closed
d4mn opened this issue May 29, 2020 · 8 comments
Closed

buildResponse doesn't catch IOException #3

d4mn opened this issue May 29, 2020 · 8 comments

Comments

@d4mn
Copy link

d4mn commented May 29, 2020

When response is GZIP I get exception:

E/Capacitor/Plugin: Error org.json.JSONException: Value ������^��� of type java.lang.String cannot be converted to JSONArray at org.json.JSON.typeMismatch(JSON.java:111) at org.json.JSONArray.<init>(JSONArray.java:96) at org.json.JSONArray.<init>(JSONArray.java:108) at com.getcapacitor.JSArray.<init>(JSArray.java:17) at com.getcapacitor.plugin.http.Http.buildResponse(Http.java:394) at com.getcapacitor.plugin.http.Http.get(Http.java:83) at com.getcapacitor.plugin.http.Http.request(Http.java:64) at java.lang.reflect.Method.invoke(Native Method) at com.getcapacitor.PluginHandle.invoke(PluginHandle.java:99) at com.getcapacitor.Bridge$1.run(Bridge.java:517) at android.os.Handler.handleCallback(Handler.java:790) at android.os.Handler.dispatchMessage(Handler.java:99) at android.os.Looper.loop(Looper.java:164) at android.os.HandlerThread.run(HandlerThread.java:65)

Describe the bug
If response is gzip HttpURLConnection doesn't decompress data automaticaly

To Reproduce
Steps to reproduce the behavior:

  1. Try to fetch gzip compressed url
  2. See error

Smartphone (please complete the following information):

  • Device: [Nexus 5x]
  • OS: [Android 8]

Expected behavior
Should decode GZIP response with GZIPInputStream

Additional context
String encoding = conn.getContentEncoding(); if (encoding != null && encoding.contains("gzip")) { stream = new GZIPInputStream(stream); }

@mlynch
Copy link
Contributor

mlynch commented May 29, 2020

Good catch, looking into it

@mlynch
Copy link
Contributor

mlynch commented May 29, 2020

Hmm just tested this with a gzip response and it works fine for me. The docs for HTTPURLConnection even say this automatically handles gzip:

image

Any other specifics about your setup that might be causing this to fail?

@d4mn
Copy link
Author

d4mn commented May 30, 2020

Hm i wasn't sure also about that when I read docs and what other users talk in the stackoverfow and when I added this fix the data had been decoded correctly. I need more tests to do this. One thing I also changed is my "Accept-Encoding" header changed from "gzip, br, deflate" to "gzip" could be that multiple accept parameters caused this bug. I will try to test it as soon as I can reach my code and will let you know!

@mlynch
Copy link
Contributor

mlynch commented May 30, 2020

Will test this with custom headers, maybe they are overwriting somehow

@d4mn
Copy link
Author

d4mn commented Jun 1, 2020

Hi, i tested it with multiple headers and when you put headers: {"Accept-Encoding": "gzip"} in the headers list and server is configured to return gzip compressed response then the lib doesn't decode data automatically. You have to decode stream your self. The problem disappears if you remove this header and server doesn't compress data. It's kind of not like a bug but more like a feature if user explicitly wants compressed data for example if returned data is a very big json response then yes compressing and decompressing is needed otherwise you can just scrip this header and you're good to go. Thanx for looking into this!

P.S IF it's not hard you can put simple check for this that other users don't get lost about it. It does nothing harm. Here is what i used: in buildResponse function
InputStream stream = (errorStream != null ? errorStream : conn.getInputStream()); String encoding = conn.getContentEncoding(); if (encoding != null && encoding.contains("gzip")) { stream = new GZIPInputStream(stream); } BufferedReader in = new BufferedReader(new InputStreamReader(stream));

@mlynch
Copy link
Contributor

mlynch commented Jun 2, 2020

Thanks! Just so I understand, if you remove this header and the server does compress the data, it works for you, yes?

Because that worked fine for me.

@d4mn
Copy link
Author

d4mn commented Jun 5, 2020

If your remove this header server won't compress the data that's how nginx or apache works. Unless some custom addon used or something. Data is compressed only then when asked.

@thomasvidas
Copy link
Contributor

BuildResponse now does catch IOException 😄. Closing this as the issue appears to be fixed in the capacitor-v3 branch

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants