Skip to content

Commit f74a562

Browse files
committed
Fix Host header for proxy client requests
When using `ban` requests with Varnish proxy client, requests are queued and sent with `flush()`. Queued requests are registered without host, making Guzzle create an empty `Host` header. When flushing, Guzzle request is re-created starting with the original one (so without host), with all registered headers. The problem is that the empty `Host` header is also copied, and thus not re-created by Guzzle with the server URL. In the backend, cURL sees it and takes it into account, overriding passed URL silently. This causes requests with empty host being fired by Guzzle, which is kind of annoying 😃.
1 parent 1e0a515 commit f74a562

File tree

4 files changed

+38
-7
lines changed

4 files changed

+38
-7
lines changed

src/ProxyClient/AbstractProxyClient.php

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,11 +161,16 @@ private function sendRequests(array $requests)
161161
$allRequests = array();
162162

163163
foreach ($requests as $request) {
164+
$headers = $request->getHeaders()->toArray();
165+
// Force to re-create Host header if empty.
166+
if (empty($headers['Host'])) {
167+
unset( $headers['Host'] );
168+
}
164169
foreach ($this->servers as $server) {
165170
$proxyRequest = $this->client->createRequest(
166171
$request->getMethod(),
167172
$server . $request->getResource(),
168-
$request->getHeaders()
173+
$headers
169174
);
170175
$allRequests[] = $proxyRequest;
171176
}

src/Test/VarnishTestCase.php

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,15 +148,24 @@ protected function getVarnishVersion()
148148
*
149149
* @return Varnish
150150
*/
151-
protected function getProxyClient()
151+
protected function getProxyClient($baseUrl = null)
152152
{
153153
if (null === $this->proxyClient) {
154154
$this->proxyClient = new Varnish(
155155
array('http://127.0.0.1:' . $this->getProxy()->getPort()),
156-
$this->getHostName() . ':' . $this->getProxy()->getPort()
156+
$baseUrl
157157
);
158158
}
159159

160160
return $this->proxyClient;
161161
}
162+
163+
/**
164+
* @return string
165+
* @throws \Exception
166+
*/
167+
protected function getBaseUrl()
168+
{
169+
return $this->getHostName() . ':' . $this->getProxy()->getPort();
170+
}
162171
}

tests/Functional/Varnish/VarnishProxyClientTest.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ public function testPurge()
7777
$this->assertMiss($this->getResponse('/cache.php'));
7878
$this->assertHit($this->getResponse('/cache.php'));
7979

80-
$this->getProxyClient()->purge('/cache.php')->flush();
80+
$this->getProxyClient($this->getBaseUrl())->purge('/cache.php')->flush();
8181
$this->assertMiss($this->getResponse('/cache.php'));
8282
}
8383

@@ -97,7 +97,7 @@ public function testPurgeContentType()
9797
$this->assertHit($this->getResponse('/negotation.php', $html));
9898

9999
$this->getResponse('/negotation.php');
100-
$this->getProxyClient()->purge('/negotation.php')->flush();
100+
$this->getProxyClient($this->getBaseUrl())->purge('/negotation.php')->flush();
101101
$this->assertMiss($this->getResponse('/negotation.php', $json));
102102
$this->assertMiss($this->getResponse('/negotation.php', $html));
103103
}
@@ -118,7 +118,7 @@ public function testRefresh()
118118
$response = $this->getResponse('/cache.php');
119119
$this->assertHit($response);
120120

121-
$this->getProxyClient()->refresh('/cache.php')->flush();
121+
$this->getProxyClient($this->getBaseUrl())->refresh('/cache.php')->flush();
122122
usleep(1000);
123123
$refreshed = $this->getResponse('/cache.php');
124124
$this->assertGreaterThan((float) $response->getBody(true), (float) $refreshed->getBody(true));
@@ -129,7 +129,7 @@ public function testRefreshContentType()
129129
$json = array('Accept' => 'application/json');
130130
$html = array('Accept' => 'text/html');
131131

132-
$this->getProxyClient()->refresh('/negotation.php', $json)->flush();
132+
$this->getProxyClient($this->getBaseUrl())->refresh('/negotation.php', $json)->flush();
133133

134134
$this->assertHit($this->getResponse('/negotation.php', $json));
135135
$this->assertMiss($this->getResponse('/negotation.php', $html));

tests/Unit/ProxyClient/VarnishTest.php

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,23 @@ public function testBanEverything()
4747
$this->assertEquals('fos.lo', $headers->get('Host'));
4848
}
4949

50+
public function testBanEverythingNoBaseUrl()
51+
{
52+
$varnish = new Varnish(array('http://127.0.0.1:123'), null, $this->client);
53+
$varnish->ban(array())->flush();
54+
55+
$requests = $this->getRequests();
56+
$this->assertCount(1, $requests);
57+
$this->assertEquals('BAN', $requests[0]->getMethod());
58+
59+
$headers = $requests[0]->getHeaders();
60+
$this->assertEquals('.*', $headers->get('X-Host'));
61+
$this->assertEquals('.*', $headers->get('X-Url'));
62+
$this->assertEquals('.*', $headers->get('X-Content-Type'));
63+
// Ensure host header doesn't exist, as we don't have a base URL.
64+
$this->assertNull($headers->get('Host'));
65+
}
66+
5067
public function testBanHeaders()
5168
{
5269
$varnish = new Varnish(array('http://127.0.0.1:123'), 'fos.lo', $this->client);

0 commit comments

Comments
 (0)