diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpField.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpField.java index 56928b316e9..78768705a16 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpField.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpField.java @@ -18,7 +18,6 @@ package org.eclipse.jetty.http; -import java.util.ArrayList; import java.util.Objects; import org.eclipse.jetty.util.StringUtil; @@ -85,113 +84,9 @@ public class HttpField { if (_value == null) return null; - - ArrayList list = new ArrayList<>(); - int state = 0; - int start=0; - int end=0; - StringBuilder builder = new StringBuilder(); - - for (int i=0;i<_value.length();i++) - { - char c = _value.charAt(i); - switch(state) - { - case 0: // initial white space - switch(c) - { - case '"': // open quote - state=2; - break; - - case ',': // ignore leading empty field - break; - - case ' ': // more white space - case '\t': - break; - - default: // character - start=i; - end=i; - state=1; - } - break; - - case 1: // In token - switch(c) - { - case ',': // next field - list.add(_value.substring(start,end+1)); - state=0; - break; - - case ' ': // more white space - case '\t': - break; - - default: - end=i; - } - break; - - case 2: // In Quoted - switch(c) - { - case '\\': // next field - state=3; - break; - - case '"': // end quote - list.add(builder.toString()); - builder.setLength(0); - state=4; - break; - - default: - builder.append(c); - } - break; - - case 3: // In Quoted Quoted - builder.append(c); - state=2; - break; - - case 4: // WS after end quote - switch(c) - { - case ' ': // white space - case '\t': // white space - break; - - case ',': // white space - state=0; - break; - - default: - throw new IllegalArgumentException("c="+(int)c); - - } - break; - } - } - - switch(state) - { - case 0: - break; - case 1: - list.add(_value.substring(start,end+1)); - break; - case 4: - break; - - default: - throw new IllegalArgumentException("state="+state); - } - - return list.toArray(new String[list.size()]); + + QuotedCSV list = new QuotedCSV(false,_value); + return list.getValues().toArray(new String[list.size()]); } /** diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpFields.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpFields.java index 67294d615ca..5da07cbbdf4 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpFields.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpFields.java @@ -282,6 +282,100 @@ public class HttpFields implements Iterable return list; } + + /** + * Add comma separated values, but only if not already + * present. + * @param header The header to add the value(s) to + * @param values The value(s) to add + * @return True if headers were modified + */ + public boolean addCSV(HttpHeader header,String... values) + { + QuotedCSV existing = null; + for (HttpField f : this) + { + if (f.getHeader()==header) + { + if (existing==null) + existing = new QuotedCSV(false); + existing.addValue(f.getValue()); + } + } + + String value = addCSV(existing,values); + if (value!=null) + { + add(header,value); + return true; + } + return false; + } + + /** + * Add comma separated values, but only if not already + * present. + * @param header The header to add the value(s) to + * @param values The value(s) to add + * @return True if headers were modified + */ + public boolean addCSV(String name,String... values) + { + QuotedCSV existing = null; + for (HttpField f : this) + { + if (f.getName().equalsIgnoreCase(name)) + { + if (existing==null) + existing = new QuotedCSV(false); + existing.addValue(f.getValue()); + } + } + String value = addCSV(existing,values); + if (value!=null) + { + add(name,value); + return true; + } + return false; + } + + protected String addCSV(QuotedCSV existing,String... values) + { + // remove any existing values from the new values + boolean add = true; + if (existing!=null && !existing.isEmpty()) + { + add = false; + + for (int i=values.length;i-->0;) + { + String unquoted = QuotedCSV.unquote(values[i]); + if (existing.getValues().contains(unquoted)) + values[i] = null; + else + add = true; + } + } + + if (add) + { + StringBuilder value = new StringBuilder(); + for (String v:values) + { + if (v==null) + continue; + if (value.length()>0) + value.append(", "); + value.append(v); + } + if (value.length()>0) + return value.toString(); + } + + return null; + } + /** * Get multiple field values of the same name, split * as a {@link QuotedCSV} @@ -292,11 +386,17 @@ public class HttpFields implements Iterable */ public List getCSV(HttpHeader header,boolean keepQuotes) { - QuotedCSV values = new QuotedCSV(keepQuotes); + QuotedCSV values = null; for (HttpField f : this) + { if (f.getHeader()==header) + { + if (values==null) + values = new QuotedCSV(keepQuotes); values.addValue(f.getValue()); - return values.getValues(); + } + } + return values==null?Collections.emptyList():values.getValues(); } /** @@ -309,11 +409,17 @@ public class HttpFields implements Iterable */ public List getCSV(String name,boolean keepQuotes) { - QuotedCSV values = new QuotedCSV(keepQuotes); + QuotedCSV values = null; for (HttpField f : this) + { if (f.getName().equalsIgnoreCase(name)) + { + if (values==null) + values = new QuotedCSV(keepQuotes); values.addValue(f.getValue()); - return values.getValues(); + } + } + return values==null?Collections.emptyList():values.getValues(); } /** @@ -325,11 +431,18 @@ public class HttpFields implements Iterable */ public List getQualityCSV(HttpHeader header) { - QuotedQualityCSV values = new QuotedQualityCSV(); + QuotedQualityCSV values = null; for (HttpField f : this) + { if (f.getHeader()==header) + { + if (values==null) + values = new QuotedQualityCSV(); values.addValue(f.getValue()); - return values.getValues(); + } + } + + return values==null?Collections.emptyList():values.getValues(); } /** @@ -341,11 +454,17 @@ public class HttpFields implements Iterable */ public List getQualityCSV(String name) { - QuotedQualityCSV values = new QuotedQualityCSV(); + QuotedQualityCSV values = null; for (HttpField f : this) + { if (f.getName().equalsIgnoreCase(name)) + { + if (values==null) + values = new QuotedQualityCSV(); values.addValue(f.getValue()); - return values.getValues(); + } + } + return values==null?Collections.emptyList():values.getValues(); } /** diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/QuotedCSV.java b/jetty-http/src/main/java/org/eclipse/jetty/http/QuotedCSV.java index fbeb153b917..46c9a098a6e 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/QuotedCSV.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/QuotedCSV.java @@ -22,6 +22,8 @@ import java.util.ArrayList; import java.util.Iterator; import java.util.List; +import org.eclipse.jetty.util.QuotedStringTokenizer; + /* ------------------------------------------------------------ */ /** * Implements a quoted comma separated list of values @@ -52,6 +54,9 @@ public class QuotedCSV implements Iterable } /* ------------------------------------------------------------ */ + /** Add and parse a value string(s) + * @param value A value that may contain one or more Quoted CSV items. + */ public void addValue(String value) { StringBuffer buffer = new StringBuffer(); @@ -241,6 +246,15 @@ public class QuotedCSV implements Iterable { } + public int size() + { + return _values.size(); + } + + public boolean isEmpty() + { + return _values.isEmpty(); + } public List getValues() { @@ -252,7 +266,7 @@ public class QuotedCSV implements Iterable { return _values.iterator(); } - + public static String unquote(String s) { // handle trivial cases diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpFieldsTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpFieldsTest.java index 75ccc054302..b5bf00afa71 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpFieldsTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpFieldsTest.java @@ -19,6 +19,7 @@ package org.eclipse.jetty.http; import java.nio.ByteBuffer; +import java.util.Collections; import java.util.Enumeration; import java.util.List; import java.util.Locale; @@ -340,7 +341,89 @@ public class HttpFieldsTest } @Test - public void testGetQualityValues() throws Exception + public void testGetCSV() throws Exception + { + HttpFields fields = new HttpFields(); + + fields.put("name0", "value0A,value0B"); + fields.add("name0", "value0C,value0D"); + fields.put("name1", "value1A, \"value\t, 1B\" "); + fields.add("name1", "\"value1C\",\tvalue1D"); + + Enumeration e = fields.getValues("name0"); + assertEquals(true, e.hasMoreElements()); + assertEquals(e.nextElement(), "value0A,value0B"); + assertEquals(true, e.hasMoreElements()); + assertEquals(e.nextElement(), "value0C,value0D"); + assertEquals(false, e.hasMoreElements()); + + e = Collections.enumeration(fields.getCSV("name0",false)); + assertEquals(true, e.hasMoreElements()); + assertEquals(e.nextElement(), "value0A"); + assertEquals(true, e.hasMoreElements()); + assertEquals(e.nextElement(), "value0B"); + assertEquals(true, e.hasMoreElements()); + assertEquals(e.nextElement(), "value0C"); + assertEquals(true, e.hasMoreElements()); + assertEquals(e.nextElement(), "value0D"); + assertEquals(false, e.hasMoreElements()); + + e = Collections.enumeration(fields.getCSV("name1",false)); + assertEquals(true, e.hasMoreElements()); + assertEquals(e.nextElement(), "value1A"); + assertEquals(true, e.hasMoreElements()); + assertEquals(e.nextElement(), "value\t, 1B"); + assertEquals(true, e.hasMoreElements()); + assertEquals(e.nextElement(), "value1C"); + assertEquals(true, e.hasMoreElements()); + assertEquals(e.nextElement(), "value1D"); + assertEquals(false, e.hasMoreElements()); + } + + @Test + public void testAddQuotedCSV() throws Exception + { + HttpFields fields = new HttpFields(); + + fields.put("some", "value"); + fields.add("name", "\"zero\""); + fields.add("name", "one, \"1 + 1\""); + fields.put("other", "value"); + fields.add("name", "three"); + fields.add("name", "four, I V"); + + List list = fields.getCSV("name",false); + assertEquals("zero",HttpFields.valueParameters(list.get(0),null)); + assertEquals("one",HttpFields.valueParameters(list.get(1),null)); + assertEquals("1 + 1",HttpFields.valueParameters(list.get(2),null)); + assertEquals("three",HttpFields.valueParameters(list.get(3),null)); + assertEquals("four",HttpFields.valueParameters(list.get(4),null)); + assertEquals("I V",HttpFields.valueParameters(list.get(5),null)); + + fields.addCSV("name","six"); + list = fields.getCSV("name",false); + assertEquals("zero",HttpFields.valueParameters(list.get(0),null)); + assertEquals("one",HttpFields.valueParameters(list.get(1),null)); + assertEquals("1 + 1",HttpFields.valueParameters(list.get(2),null)); + assertEquals("three",HttpFields.valueParameters(list.get(3),null)); + assertEquals("four",HttpFields.valueParameters(list.get(4),null)); + assertEquals("I V",HttpFields.valueParameters(list.get(5),null)); + assertEquals("six",HttpFields.valueParameters(list.get(6),null)); + + fields.addCSV("name","1 + 1","7","zero"); + list = fields.getCSV("name",false); + assertEquals("zero",HttpFields.valueParameters(list.get(0),null)); + assertEquals("one",HttpFields.valueParameters(list.get(1),null)); + assertEquals("1 + 1",HttpFields.valueParameters(list.get(2),null)); + assertEquals("three",HttpFields.valueParameters(list.get(3),null)); + assertEquals("four",HttpFields.valueParameters(list.get(4),null)); + assertEquals("I V",HttpFields.valueParameters(list.get(5),null)); + assertEquals("six",HttpFields.valueParameters(list.get(6),null)); + assertEquals("7",HttpFields.valueParameters(list.get(7),null)); + } + + @Test + public void testGetQualityCSV() throws Exception { HttpFields fields = new HttpFields(); @@ -351,7 +434,8 @@ public class HttpFieldsTest fields.add("name", "one;q=0.4"); fields.add("name", "three;x=y;q=0.2;a=b,two;q=0.3"); - List list = HttpFields.qualityList(fields.getValues("name",",")); + + List list = fields.getQualityCSV("name"); assertEquals("zero",HttpFields.valueParameters(list.get(0),null)); assertEquals("one",HttpFields.valueParameters(list.get(1),null)); assertEquals("two",HttpFields.valueParameters(list.get(2),null)); diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpTester.java b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpTester.java index 7da2f9bd010..e9ae0412714 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpTester.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpTester.java @@ -297,7 +297,7 @@ public class HttpTester @Override public void parsedHeader(HttpField field) { - put(field.getName(),field.getValue()); + add(field.getName(),field.getValue()); } @Override diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHandler.java index 4f12f8a8997..85669126ade 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHandler.java @@ -30,10 +30,8 @@ import javax.servlet.http.HttpServletResponse; import org.eclipse.jetty.http.GzipHttpContent; import org.eclipse.jetty.http.HttpField; -import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpMethod; -import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.http.MimeTypes; import org.eclipse.jetty.http.pathmap.PathSpecSet; import org.eclipse.jetty.server.HttpOutput; diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHttpOutputInterceptor.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHttpOutputInterceptor.java index b96930b0df5..0fc380f1782 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHttpOutputInterceptor.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHttpOutputInterceptor.java @@ -30,6 +30,7 @@ import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.MimeTypes; import org.eclipse.jetty.http.PreEncodedHttpField; +import org.eclipse.jetty.http.QuotedCSV; import org.eclipse.jetty.server.HttpChannel; import org.eclipse.jetty.server.HttpOutput; import org.eclipse.jetty.server.Response; @@ -189,7 +190,8 @@ public class GzipHttpOutputInterceptor implements HttpOutput.Interceptor } // Has the Content-Encoding header already been set? - String ce=response.getHeader("Content-Encoding"); + HttpFields fields = response.getHttpFields(); + String ce=fields.get(HttpHeader.CONTENT_ENCODING); if (ce != null) { LOG.debug("{} exclude by content-encoding {}",this,ce); @@ -202,9 +204,13 @@ public class GzipHttpOutputInterceptor implements HttpOutput.Interceptor if (_state.compareAndSet(GZState.MIGHT_COMPRESS,GZState.COMMITTING)) { // We are varying the response due to accept encoding header. - HttpFields fields = response.getHttpFields(); if (_vary != null) - fields.add(_vary); + { + if (fields.contains(HttpHeader.VARY)) + fields.addCSV(HttpHeader.VARY,_vary.getValues()); + else + fields.add(_vary); + } long content_length = response.getContentLength(); if (content_length<0 && complete) diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/GzipHandlerTest.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/GzipHandlerTest.java index 6ddadb5e6fc..e892f389e5a 100644 --- a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/GzipHandlerTest.java +++ b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/GzipHandlerTest.java @@ -134,6 +134,8 @@ public class GzipHandlerTest @Override protected void doGet(HttpServletRequest req, HttpServletResponse response) throws ServletException, IOException { + if (req.getParameter("vary")!=null) + response.addHeader("Vary",req.getParameter("vary")); response.setHeader("ETag",__contentETag); String ifnm = req.getHeader("If-None-Match"); if (ifnm!=null && ifnm.equals(__contentETag)) @@ -181,25 +183,25 @@ public class GzipHandlerTest HttpTester.Response response; request.setMethod("GET"); - request.setURI("/ctx/content"); + request.setURI("/ctx/content?vary=Other"); request.setVersion("HTTP/1.0"); request.setHeader("Host","tester"); response = HttpTester.parseResponse(_connector.getResponses(request.generate())); - + assertThat(response.getStatus(),is(200)); assertThat(response.get("Content-Encoding"),not(equalToIgnoringCase("gzip"))); assertThat(response.get("ETag"),is(__contentETag)); - assertThat(response.get("Vary"),is("Accept-Encoding")); + assertThat(response.getValuesList("Vary"),Matchers.contains("Other","Accept-Encoding")); InputStream testIn = new ByteArrayInputStream(response.getContentBytes()); ByteArrayOutputStream testOut = new ByteArrayOutputStream(); IO.copy(testIn,testOut); assertEquals(__content, testOut.toString("UTF8")); - } + @Test public void testGzipHandler() throws Exception { @@ -208,7 +210,7 @@ public class GzipHandlerTest HttpTester.Response response; request.setMethod("GET"); - request.setURI("/ctx/content"); + request.setURI("/ctx/content?vary=Accept-Encoding,Other"); request.setVersion("HTTP/1.0"); request.setHeader("Host","tester"); request.setHeader("accept-encoding","gzip"); @@ -218,7 +220,7 @@ public class GzipHandlerTest assertThat(response.getStatus(),is(200)); assertThat(response.get("Content-Encoding"),Matchers.equalToIgnoringCase("gzip")); assertThat(response.get("ETag"),is(__contentETagGzip)); - assertThat(response.get("Vary"),is("Accept-Encoding")); + assertThat(response.getCSV("Vary",false),Matchers.contains("Accept-Encoding","Other")); InputStream testIn = new GZIPInputStream(new ByteArrayInputStream(response.getContentBytes())); ByteArrayOutputStream testOut = new ByteArrayOutputStream();