search

lox / httpcache

An RFC7234 compliant golang http.Handler for caching HTTP responses
MIT License
264 stars 28 forks source link

Transfer-Encoding: Chunked returns only the first chunk #32

Open jfesler opened 8 years ago

jfesler commented 8 years ago

Thanks for your work with httpcache - no small effort!

I have found that the Transfer-Encoding: chunked behavior is broken. When sending a request through httpcache.NewHandler(httpcche.NewMemoryCache()) with httputil.NewSingleHostReverseProxy, that the cache returns only the first chunk.

Without the cache, httputil.NewSingleHostReverseProxy properly fetches chunked content fine.

An example of chunked output from the web server I'm testing with:

Jasons-MacBook-Pro:gateway-test jfesler$ telnet localhost 4000
Trying ::1...
telnet: connect to address ::1: Connection refused
Trying 127.0.0.1...
Connected to localhost.
Escape character is '^]'.
GET /cached/hello/ HTTP/1.1
Host: localhost:3000

HTTP/1.1 200 OK
Content-Type: text/plain
Date: Tue, 13 Sep 2016 23:45:55 GMT
Transfer-Encoding: chunked

2f
Counter(i=0); time is now 2016-09-13- 16:45:55

2f
Counter(i=1); time is now 2016-09-13- 16:45:56

2f
Counter(i=2); time is now 2016-09-13- 16:45:57

2f
Counter(i=3); time is now 2016-09-13- 16:45:58

2f
Counter(i=4); time is now 2016-09-13- 16:45:59

2f
Counter(i=5); time is now 2016-09-13- 16:46:00

2f
Counter(i=6); time is now 2016-09-13- 16:46:01

2f
Counter(i=7); time is now 2016-09-13- 16:46:02

2f
Counter(i=8); time is now 2016-09-13- 16:46:03

2f
Counter(i=9); time is now 2016-09-13- 16:46:04

0

And with curl, hitting that proxy directly, it works fine as well:

Jasons-MacBook-Pro:gateway-test jfesler$ curl http://localhost:4000/counter/
Counter(i=0); time is now 2016-09-13- 16:52:56
Counter(i=1); time is now 2016-09-13- 16:52:57
Counter(i=2); time is now 2016-09-13- 16:52:58
Counter(i=3); time is now 2016-09-13- 16:52:59
Counter(i=4); time is now 2016-09-13- 16:53:00
Counter(i=5); time is now 2016-09-13- 16:53:01
Counter(i=6); time is now 2016-09-13- 16:53:02
Counter(i=7); time is now 2016-09-13- 16:53:03
Counter(i=8); time is now 2016-09-13- 16:53:04
Counter(i=9); time is now 2016-09-13- 16:53:05

On the bright side, the cache at least doesn't serve me the same chunk#1 as a cache hit; it instead calls it a miss and goes to the back end again on the subsequent query.

Unfortunately, I can't provide code samples, due to my current choice of employer. Which means I can't offer a PR either :-( For that I truly apologize.

lox commented 8 years ago

Thanks for the report, I'll investigate this. I've been trying to get a redesign of the internals of httpcache going, which should address some of these issues.

jfesler commented 8 years ago

"On the bright side, the cache at least doesn't serve me the same chunk#1 as a cache hit; it instead calls it a miss and goes to the back end again on the subsequent query."

Ignore that bit; not relevant. Digging deeper into your code, my response is not giving enough of a reason to be isCacheable.

The chunking issue remains :-)

jfesler commented 8 years ago

Narrowed down a bit more.

Chunked works fine if isCacheable(). Chunked fails if not isCacheable(); instead returns the first chunk, as a miss.