Skip to content

Commit

Permalink
[HttpKernel] fixed Content-Length header when using ESI tags (closes #…
Browse files Browse the repository at this point in the history 
…2623)
  • Loading branch information
@fabpot
fabpot committed Nov 14, 2011
1 parent d67fbe9 commit f7c5bf1
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 0 deletions.
3 changes: 3 additions & 0 deletions src/Symfony/Component/HttpKernel/HttpCache/HttpCache.php
View file
Expand Up @@ -585,6 +585,9 @@ private function restoreResponseBody(Request $request, Response $response)

$response->setContent(ob_get_clean());
$response->headers->remove('X-Body-Eval');
if (!$response->headers->has('Transfer-Encoding')) {
$response->headers->set('Content-Length', strlen($response->getContent()));
}
} elseif ($response->headers->has('X-Body-File')) {
$response->setContent(file_get_contents($response->headers->get('X-Body-File')));
} else {
Expand Down
26 changes: 26 additions & 0 deletions tests/Symfony/Tests/Component/HttpKernel/HttpCache/HttpCacheTest.php
View file
Expand Up @@ -978,4 +978,30 @@ public function testEsiCacheForceValidation()
$this->assertTrue($this->response->headers->hasCacheControlDirective('private'));
$this->assertTrue($this->response->headers->hasCacheControlDirective('no-cache'));
}

public function testEsiRecalculateContentLengthHeader()
{
$responses = array(
array(
'status' => 200,
'body' => '<esi:include src="/foo" />',
'headers' => array(
'Content-Length' => 26,
'Cache-Control' => 's-maxage=300',
'Surrogate-Control' => 'content="ESI/1.0"',
),
),
array(
'status' => 200,
'body' => 'Hello World!',
'headers' => array(),
),
);

$this->setNextResponses($responses);

$this->request('GET', '/', array(), array(), true);
$this->assertEquals('Hello World!', $this->response->getContent());
$this->assertEquals(12, $this->response->headers->get('Content-Length'));
}
}

6 comments on commit f7c5bf1

@lsmith77
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and we are sure that for this code we will not again run into trouble?

@fabpot
Copy link
Member Author

@fabpot fabpot commented on f7c5bf1 Nov 14, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean?

@stof
Copy link
Member

@stof stof commented on f7c5bf1 Nov 14, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does Varnish handle a response containing a Content-Length header ? Does it update it after resolving the ESI tags ?

@lsmith77
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well we ran into a world of hurt when we added the content-length before and dropped this feature because we found its better to rely on the web server adding the content length.

@innocead
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. This code work when esi tags convert into html without Varnish and esi is on in config. No problems should be.
  2. Content-Length header is also adding before that code. In Store->write metod. And that is why symfony send wrong Content-Length header when using ESI tags without Varnish.
  3. Varnish update header after resolving the ESI

@fabpot
Copy link
Member Author

@fabpot fabpot commented on f7c5bf1 Nov 14, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lsmith77: The code is different from the one we have removed some time ago. Here, I've just fixed a bug because the Store set a wrong Content-Length header.

Please sign in to comment.