Merge pull request #1829 from ataylorme/patch-1

Add double quotes around the ETag header value; recognize multiple quoted ETags in If-None-Match header
ampproject · Jan 18, 2019 · 5d771d7 · 5d771d7
2 parents f893a8f + 2ad6aa3
commit 5d771d7
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 4 deletions.
diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php
@@ -1652,10 +1652,23 @@ public static function prepare_response( $response, $args = array() ) {
 		 * not have been set yet during the WordPress template generation is
 		 * the ETag. The AMP plugin sends a Vary header at amp_init.
 		 */
-		AMP_HTTP::send_header( 'ETag', $response_cache_key );
+		AMP_HTTP::send_header( 'ETag', '"' . $response_cache_key . '"' );
 
-		// Handle responses that are cached by the browser.
-		if ( isset( $_SERVER['HTTP_IF_NONE_MATCH'] ) && $_SERVER['HTTP_IF_NONE_MATCH'] === $response_cache_key ) {
+		/*
+		 * Handle responses that are cached by the browser, returning 304 response if the response cache key
+		 * matches any ETags mentioned in If-None-Match request header. Note that if the client request indicates a
+		 * weak validator (prefixed by W/) then this will be ignored. The MD5 strings will be extracted from the
+		 * If-None-Match request header and if any of them match the $response_cache_key then a 304 Not Modified
+		 * response is returned.
+		 */
+		$has_matching_etag = (
+			isset( $_SERVER['HTTP_IF_NONE_MATCH'] )
+			&&
+			preg_match_all( '#\b[0-9a-f]{32}\b#', wp_unslash( $_SERVER['HTTP_IF_NONE_MATCH'] ), $etag_match_candidates )
+			&&
+			in_array( $response_cache_key, $etag_match_candidates[0], true )
+		);
+		if ( $has_matching_etag ) {
 			status_header( 304 );
 			return '';
 		}

diff --git a/tests/test-class-amp-theme-support.php b/tests/test-class-amp-theme-support.php
@@ -1355,7 +1355,21 @@ public function test_prepare_response() {
 		// Test that ETag allows response to short-circuit via If-None-Match request header.
 		$_SERVER['HTTP_IF_NONE_MATCH'] = $etag;
 		$this->assertEmpty( '', $call_prepare_response() );
-		$_SERVER['HTTP_IF_NONE_MATCH'] = $etag . '-v2';
+
+		$_SERVER['HTTP_IF_NONE_MATCH'] = sprintf( '"%s"', $etag );
+		$this->assertEmpty( '', $call_prepare_response() );
+
+		$_SERVER['HTTP_IF_NONE_MATCH'] = sprintf( 'W/"%s"', $etag );
+		$this->assertEmpty( '', $call_prepare_response() );
+
+		$_SERVER['HTTP_IF_NONE_MATCH'] = sprintf( '"%s", W/"%s"', md5( 'foo' ), $etag );
+		$this->assertEmpty( '', $call_prepare_response() );
+
+		$_SERVER['HTTP_IF_NONE_MATCH'] = sprintf( '"%s", "%s"', $etag, md5( 'bar' ) );
+		$this->assertEmpty( '', $call_prepare_response() );
+
+		// Test not match.
+		$_SERVER['HTTP_IF_NONE_MATCH'] = strrev( $etag );
 		$this->assertNotEmpty( $call_prepare_response() );
 		$this->assertNotEmpty( $this->get_etag_header_value( AMP_HTTP::$headers_sent ) );
 		unset( $_SERVER['HTTP_IF_NONE_MATCH'] );