Merge pull request #134 from FriendsOfSymfony/fix-accept-header-handling

fix accept header handling when client sends no accept header
FriendsOfSymfony · Nov 21, 2014 · 4e227b4 · 4e227b4
2 parents 480615b + e894523
commit 4e227b4
Show file tree

Hide file tree

Showing 6 changed files with 63 additions and 16 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -0,0 +1,8 @@
+Changelog
+=========
+
+1.1.2
+-----
+
+* **2014-11-17** Fixed documentation for user context varnish configuration to also work when
+  client omits the `Accept` HTTP header.
diff --git a/tests/.htaccess b/tests/.htaccess
@@ -0,0 +1,2 @@
+deny from all
+
diff --git a/tests/Functional/Fixtures/varnish-3/user_context.vcl b/tests/Functional/Fixtures/varnish-3/user_context.vcl
@@ -14,10 +14,12 @@ sub vcl_recv {
         && (req.http.cookie || req.http.authorization)
         && (req.request == "GET" || req.request == "HEAD")
     ) {
-        set req.http.x-fos-original-url    = req.url;
-        set req.http.x-fos-original-accept = req.http.accept;
-
-        set req.http.accept            = "application/vnd.fos.user-context-hash";
+        set req.http.x-fos-original-url = req.url;
+        # Backup accept header, if set
+        if (req.http.accept) {
+            set req.http.x-fos-original-accept = req.http.accept;
+        }
+        set req.http.accept = "application/vnd.fos.user-context-hash";
 
         # A little hack for testing all scenarios. Choose one for your application.
         if ("failure" == req.http.x-cache-hash) {
@@ -38,11 +40,15 @@ sub vcl_recv {
     if (req.restarts > 0
         && req.http.accept == "application/vnd.fos.user-context-hash"
     ) {
-        set req.url         = req.http.x-fos-original-url;
-        set req.http.accept = req.http.x-fos-original-accept;
-
+        set req.url = req.http.x-fos-original-url;
         unset req.http.x-fos-original-url;
-        unset req.http.x-fos-original-accept;
+        if (req.http.x-fos-original-accept) {
+            set req.http.accept = req.http.x-fos-original-accept;
+            unset req.http.x-fos-original-accept;
+        } else {
+            # If accept header was not set in original request, remove the header here.
+            unset req.http.accept;
+        }
 
         # Force the lookup, the backend must tell not to cache or vary on the
         # user hash to properly separate cached data.

diff --git a/tests/Functional/Fixtures/varnish-4/user_context.vcl b/tests/Functional/Fixtures/varnish-4/user_context.vcl
@@ -16,10 +16,12 @@ sub vcl_recv {
         && (req.http.cookie || req.http.authorization)
         && (req.method == "GET" || req.method == "HEAD")
     ) {
-        set req.http.x-fos-original-url    = req.url;
-        set req.http.x-fos-original-accept = req.http.accept;
-
-        set req.http.accept            = "application/vnd.fos.user-context-hash";
+        set req.http.x-fos-original-url = req.url;
+        # Backup accept header, if set
+        if (req.http.accept) {
+            set req.http.x-fos-original-accept = req.http.accept;
+        }
+        set req.http.accept = "application/vnd.fos.user-context-hash";
 
         # A little hack for testing all scenarios. Choose one for your application.
         if ("failure" == req.http.x-cache-hash) {
@@ -40,11 +42,15 @@ sub vcl_recv {
     if (req.restarts > 0
         && req.http.accept == "application/vnd.fos.user-context-hash"
     ) {
-        set req.url         = req.http.x-fos-original-url;
-        set req.http.accept = req.http.x-fos-original-accept;
-
+        set req.url = req.http.x-fos-original-url;
         unset req.http.x-fos-original-url;
-        unset req.http.x-fos-original-accept;
+        if (req.http.x-fos-original-accept) {
+            set req.http.accept = req.http.x-fos-original-accept;
+            unset req.http.x-fos-original-accept;
+        } else {
+            # If accept header was not set in original request, remove the header here.
+            unset req.http.accept;
+        }
 
         # Force the lookup, the backend must tell not to cache or vary on the
         # user hash to properly separate cached data.

diff --git a/tests/Functional/Fixtures/web/user_context.php b/tests/Functional/Fixtures/web/user_context.php
@@ -11,6 +11,16 @@
 
 header('X-Cache-Debug: 1');
 
+if (isset($_GET['accept'])) {
+    if ($_GET['accept'] != $_SERVER['HTTP_ACCEPT']) {
+        header('HTTP/1.1 500 Wrong accept header "' . $_SERVER['HTTP_ACCEPT'] . '", expected "' . $_GET['accept'] . '"');
+        exit;
+    }
+} elseif (isset($_SERVER['HTTP_ACCEPT'])) {
+    header('HTTP/1.1 500 Expected no accept header ' . $_SERVER['HTTP_ACCEPT']);
+    exit;
+}
+
 if ('POST' == strtoupper($_SERVER['REQUEST_METHOD'])) {
     echo "POST";
     exit;

diff --git a/tests/Functional/Varnish/UserContextTestCase.php b/tests/Functional/Varnish/UserContextTestCase.php
@@ -27,6 +27,10 @@ abstract class UserContextTestCase extends VarnishTestCase
      */
     abstract protected function assertContextCache($hashCache);
 
+    /**
+     * Sending requests without an Accept: header so none should arrive at the
+     * backend for the actual request.
+     */
     public function testUserContextHash()
     {
         $response1 = $this->getResponse('/user_context.php', array(), array('cookies' => array('foo')));
@@ -64,6 +68,17 @@ public function testUserContextHash()
         $this->assertHit($headResponse2);
     }
 
+    public function testAcceptHeader()
+    {
+        $response1 = $this->getResponse(
+            '/user_context.php?accept=text/plain',
+            array('Accept' => 'text/plain'),
+            array('cookies' => array('foo'))
+        );
+        $this->assertEquals('foo', $response1->getBody(true));
+
+    }
+
     public function testUserContextUnauthorized()
     {
         try {