paulirotta / Tantalum

Tantalum Cross Platform Library
12 stars 6 forks source link

JsonGetter doesn't survive if Content-Length is missing #43

Closed vivainio closed 11 years ago

vivainio commented 11 years ago

Trying this url, there is no Content-Length header,

http://www.sodexo.fi//ruokalistat/output/daily_json/731/1/1/2013/en?mobileRedirect=false

In HttpGetter::exec, this leads to "length" being 0, and us going to this branch:

L.i(this.getClass().getName() + " exec", "No response. Stream is null, or length is 0");

Instead of:

            } else {
                bos = new ByteArrayOutputStream(16384);
                readBytesVariableLength(inputStream, bos);
                out = bos.toByteArray();
            }

Network capture below:

GET /ruokalistat/output/daily_json/731/1/1/2013/en?mobileRedirect=false HTTP/1.1
User-Agent: ***
Host: www.sodexo.fi
Connection: Keep-Alive
Accept-Encoding: gzip

HTTP/1.1 200 OK
Server: nginx/1.0.11
Date: Fri, 19 Apr 2013 06:35:54 GMT
Content-Type: application/json; charset=utf-8
Transfer-Encoding: chunked
Connection: keep-alive
X-Powered-By: PHP/5.3.8
X-Drupal-Cache: MISS
Expires: Sun, 19 Nov 1978 05:00:00 GMT
Last-Modified: Fri, 19 Apr 2013 06:35:54 +0000
Cache-Control: no-cache, must-revalidate, post-check=0, pre-check=0
ETag: "1366353354"
Set-Cookie: SESSee97dcf51c4e7f072056872f1ea10323=TNNdXst9Gv6iTXOWLbGzooI13tCzSLijWwl_ZewOzCc; expires=Sun, 12-May-2013 10:09:14 GMT; path=/; domain=.sodexo.fi; HttpOnly

90
{"meta":{"generated_timestamp":1366353354,"requested_timestamp":false,"ref_url":"http:\/\/www.sodexo.fi\/node\/","ref_title":null},"courses":[]}
0
vivainio commented 11 years ago

Also possible that chunked encoding screws it up

paulirotta commented 11 years ago

Some relevant mods after surgery in that area on the merged_rmsfastcache branch. I'm close to pulling that back into trunk with lots of goodness like faster caching, bug fixes and some API changes. Looking into adding a unit test.

vivainio commented 11 years ago

@paulirotta it seems your branch fixes this problem. Any objections still to merge it in?

paulirotta commented 11 years ago

All merged. I must check @adiksonline has some JSON changes. Merge of that is a separate task

kingsleyadio commented 11 years ago

Oh, I might have made some changes to that while investigating the BB issue. You can go through it if not, just use the original JSON package instead.