Author: jtarrio Date: Thu Aug 25 22:55:22 2011 New Revision: 1161788 URL: http://svn.apache.org/viewvc?rev=1161788&view=rev Log: Generate and check ETags using the proper syntax.
According to the RFC, ETags have to be in quote signs. Also, the If-None-Matches request header can specify several ETags, separated with commas. Review: https://reviews.apache.org/r/1648/ Modified: shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ETaggingHttpResponse.java shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ETagFilterTest.java shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ETaggingHttpResponseTest.java Modified: shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ETaggingHttpResponse.java URL: http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ETaggingHttpResponse.java?rev=1161788&r1=1161787&r2=1161788&view=diff ============================================================================== --- shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ETaggingHttpResponse.java (original) +++ shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ETaggingHttpResponse.java Thu Aug 25 22:55:22 2011 @@ -18,6 +18,10 @@ package org.apache.shindig.gadgets.servlet; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.CharMatcher; +import com.google.common.base.Splitter; +import com.google.common.base.Strings; +import com.google.common.collect.ImmutableList; import org.apache.http.util.ByteArrayBuffer; import org.apache.shindig.common.util.HashUtil; @@ -48,6 +52,9 @@ public class ETaggingHttpResponse extend public static final String RESPONSE_HEADER = "ETag"; public static final String REQUEST_HEADER = "If-None-Match"; + private static final Splitter IF_NONE_MATCH_SPLITTER = + Splitter.on(',').trimResults().trimResults(CharMatcher.is('"')); + protected final HttpServletRequest request; protected final BufferServletOutputStream stream; protected ServletOutputStream originalStream; @@ -145,16 +152,15 @@ public class ETaggingHttpResponse extend * * @throws IOException If there was a problem writing to the output. */ - void writeToOutput() throws IOException { + protected void writeToOutput() throws IOException { if (writer != null) { writer.flush(); } byte[] bytes = stream.getBuffer().toByteArray(); if (batching) { String etag = stream.getContentHash(); - String reqEtag = request.getHeader(REQUEST_HEADER); - ((HttpServletResponse) getResponse()).setHeader(RESPONSE_HEADER, etag); - if (etag.equals(reqEtag)) { + ((HttpServletResponse) getResponse()).setHeader(RESPONSE_HEADER, '"' + etag + '"'); + if (etagMatches(etag)) { emitETagMatchedResult(); } else { emitFullResponseBody(bytes); @@ -165,6 +171,14 @@ public class ETaggingHttpResponse extend } } + protected boolean etagMatches(String etag) { + String ifNoneMatches = request.getHeader(REQUEST_HEADER); + if (Strings.isNullOrEmpty(ifNoneMatches)) { + return false; + } + return ImmutableList.copyOf(IF_NONE_MATCH_SPLITTER.split(ifNoneMatches)).contains(etag); + } + protected void emitETagMatchedResult() { ((HttpServletResponse) getResponse()).setStatus(HttpServletResponse.SC_NOT_MODIFIED); getResponse().setContentLength(0); Modified: shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ETagFilterTest.java URL: http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ETagFilterTest.java?rev=1161788&r1=1161787&r2=1161788&view=diff ============================================================================== --- shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ETagFilterTest.java (original) +++ shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ETagFilterTest.java Thu Aug 25 22:55:22 2011 @@ -69,7 +69,7 @@ public class ETagFilterTest { EasyMock.expect(response.getOutputStream()).andReturn(stream).anyTimes(); EasyMock.expect(response.getCharacterEncoding()).andReturn(ENCODING).anyTimes(); EasyMock.expect(request.getHeader(ETaggingHttpResponse.REQUEST_HEADER)).andReturn(null); - response.setHeader(ETaggingHttpResponse.RESPONSE_HEADER, GOOD_ETAG); + response.setHeader(ETaggingHttpResponse.RESPONSE_HEADER, '"' + GOOD_ETAG + '"'); response.setContentLength(RESPONSE_BODY_LENGTH); } Modified: shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ETaggingHttpResponseTest.java URL: http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ETaggingHttpResponseTest.java?rev=1161788&r1=1161787&r2=1161788&view=diff ============================================================================== --- shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ETaggingHttpResponseTest.java (original) +++ shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ETaggingHttpResponseTest.java Thu Aug 25 22:55:22 2011 @@ -19,6 +19,10 @@ package org.apache.shindig.gadgets.servl import static org.junit.Assert.*; +import com.google.common.base.Function; +import com.google.common.base.Joiner; +import com.google.common.collect.Lists; + import org.apache.http.util.ByteArrayBuffer; import org.apache.shindig.gadgets.servlet.ETaggingHttpResponse.BufferServletOutputStream; import org.easymock.EasyMock; @@ -26,6 +30,8 @@ import org.easymock.IMocksControl; import org.junit.Before; import org.junit.Test; +import java.util.Arrays; + import javax.servlet.ServletOutputStream; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -49,6 +55,12 @@ public class ETaggingHttpResponseTest { private static final String SECOND_ETAG = "b6e56fb0129c3530f23dbb795daa3200"; private static final String BAD_ETAG = "some bogus etag"; private static final String EMPTY_CONTENT_ETAG = "d41d8cd98f00b204e9800998ecf8427e"; + + private static final Function<String, String> ETAG_QUOTER = new Function<String, String>() { + public String apply(String input) { + return '"' + input + '"'; + } + }; private IMocksControl control; private HttpServletRequest request; @@ -73,7 +85,7 @@ public class ETaggingHttpResponseTest { @Test public void testTagContentWithPrint() throws Exception { - expectRequestETag(null); + expectRequestETag(); expectFullResponse(); control.replay(); @@ -83,7 +95,7 @@ public class ETaggingHttpResponseTest { assertResponseHasBody(); control.verify(); } - + @Test public void testNotModifiedWithPrint() throws Exception { expectRequestETag(GOOD_ETAG); @@ -96,6 +108,19 @@ public class ETaggingHttpResponseTest { assertResponseBodyIsEmpty(); control.verify(); } + + @Test + public void testNotModifiedWithManyETagsInRequest() throws Exception { + expectRequestETag(SECOND_ETAG, GOOD_ETAG, BAD_ETAG); + expectNotModifiedResponse(GOOD_ETAG); + control.replay(); + + response.getWriter().print(RESPONSE_BODY); + response.flushBuffer(); + + assertResponseBodyIsEmpty(); + control.verify(); + } @Test public void testNonMatchingETagWithPrint() throws Exception { @@ -109,10 +134,23 @@ public class ETaggingHttpResponseTest { assertResponseHasBody(); control.verify(); } + + @Test + public void testNonMatchingETagWithManyETagsInRequest() throws Exception { + expectRequestETag(BAD_ETAG, SECOND_ETAG, EMPTY_CONTENT_ETAG); + expectFullResponse(); + control.replay(); + + response.getWriter().print(RESPONSE_BODY); + response.flushBuffer(); + + assertResponseHasBody(); + control.verify(); + } @Test public void testTagContentWithWrite() throws Exception { - expectRequestETag(null); + expectRequestETag(); expectFullResponse(); control.replay(); @@ -151,8 +189,8 @@ public class ETaggingHttpResponseTest { @Test public void testTagEmptyContent() throws Exception { - expectRequestETag(null); - origResponse.setHeader(ETaggingHttpResponse.RESPONSE_HEADER, EMPTY_CONTENT_ETAG); + expectRequestETag(); + origResponse.setHeader(ETaggingHttpResponse.RESPONSE_HEADER, '"' + EMPTY_CONTENT_ETAG + '"'); origResponse.setContentLength(0); control.replay(); @@ -165,7 +203,7 @@ public class ETaggingHttpResponseTest { @Test public void testStreamingMode() throws Exception { - expectRequestETag(null); + expectRequestETag(); control.replay(); response.getWriter().print(RESPONSE_BODY); @@ -213,17 +251,21 @@ public class ETaggingHttpResponseTest { control.verify(); } - private void expectRequestETag(String eTag) { - EasyMock.expect(request.getHeader(ETaggingHttpResponse.REQUEST_HEADER)).andReturn(eTag); + private void expectRequestETag(String... eTag) { + String eTags = null; + if (eTag.length > 0) { + eTags = Joiner.on(',').join(Lists.transform(Arrays.asList(eTag), ETAG_QUOTER)); + } + EasyMock.expect(request.getHeader(ETaggingHttpResponse.REQUEST_HEADER)).andReturn(eTags); } private void expectFullResponse() { - origResponse.setHeader(ETaggingHttpResponse.RESPONSE_HEADER, GOOD_ETAG); + origResponse.setHeader(ETaggingHttpResponse.RESPONSE_HEADER, '"' + GOOD_ETAG + '"'); origResponse.setContentLength(RESPONSE_BODY_LENGTH); } private void expectNotModifiedResponse(String eTag) { - origResponse.setHeader(ETaggingHttpResponse.RESPONSE_HEADER, eTag); + origResponse.setHeader(ETaggingHttpResponse.RESPONSE_HEADER, '"' + eTag + '"'); origResponse.setStatus(HttpServletResponse.SC_NOT_MODIFIED); origResponse.setContentLength(0); }
