echocat/caddy-filter

Empty body responses (like 301) not handled correctly

blaubaer opened this issue · 9 comments

blaubaer commented

If an empty body responses with different response codes than 200 are not handled correctly. The header are not set at the target response.

Specially in case if rule proxy or fastcgi is used.

blaubaer commented

@mwpastore this will handle your filed problem from #4.

blaubaer commented

This issue is now fixed but remains open until I can verify that download page of caddy serves this fix.

mwpastore commented

Looks good so far! Thanks again!

mwpastore commented

This is still not quite right...

HTTP/1.1 200 OK
Accept-Ranges: bytes
Age: 0
Content-Type: text/html; charset=utf-8
Date: Tue, 31 Jan 2017 20:32:34 GMT
Location: /wp-admin/
Server: Caddy
Vary: Accept-Encoding
Via: 1.1 varnish-v4
X-Varnish: 18448601
Content-Length: 45

<a href="/wp-admin/">Moved Permanently</a>.

It's presenting a Location header, without the weird Status header, but the status code is 200 instead of 301.

blaubaer commented

What do you exactly expect? I assume the one you mention before is the actual result, right?

mwpastore commented

I expect to see an HTTP redirect:

HTTP/1.1 301 Moved Permanently
Location: /wp-admin/
blaubaer commented

Hey @mwpastore! I tried to reproduce this issue. I write an integration test for it and also tried to reproduce this with a PHP installation and some PHP script. But it didn't happend in my cases.

Do you can provide me a complete environment in a ZIP file etc. with it? If it is possible not a whole WordPress installation. 😉

mwpastore commented

Hi @blaubaer! Thank you for keeping at it. You can use my runnable example from #4, but with the following PHP instead:

<?php
error_log("This error will break caddy with caddy-filter", 0);

header('Location: https://www.google.com', true, 301);
mwpastore commented

Hmm, I see what you mean. There must be some other variable that this test case doesn't factor in. I'll try to figure out what it is.