Issue #898
Added addCSV method to HttpFields to more efficiently add values to a CSV field without duplicates. Improved usage of QuotedCSV and removed older parsing Used new method in GazipHttpOutPutInterceptor to avoid duplicate Vary fields
This commit is contained in:
parent
20cd7bcb35
commit
1f7c5a5b20
|
@ -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<String> 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()]);
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
@ -282,6 +282,100 @@ public class HttpFields implements Iterable<HttpField>
|
|||
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<HttpField>
|
|||
*/
|
||||
public List<String> 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<HttpField>
|
|||
*/
|
||||
public List<String> 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<HttpField>
|
|||
*/
|
||||
public List<String> 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<HttpField>
|
|||
*/
|
||||
public List<String> 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();
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
@ -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<String>
|
|||
}
|
||||
|
||||
/* ------------------------------------------------------------ */
|
||||
/** 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<String>
|
|||
{
|
||||
}
|
||||
|
||||
public int size()
|
||||
{
|
||||
return _values.size();
|
||||
}
|
||||
|
||||
public boolean isEmpty()
|
||||
{
|
||||
return _values.isEmpty();
|
||||
}
|
||||
|
||||
public List<String> getValues()
|
||||
{
|
||||
|
@ -252,7 +266,7 @@ public class QuotedCSV implements Iterable<String>
|
|||
{
|
||||
return _values.iterator();
|
||||
}
|
||||
|
||||
|
||||
public static String unquote(String s)
|
||||
{
|
||||
// handle trivial cases
|
||||
|
|
|
@ -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<String> 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<String> 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<String> list = HttpFields.qualityList(fields.getValues("name",","));
|
||||
|
||||
List<String> 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));
|
||||
|
|
|
@ -297,7 +297,7 @@ public class HttpTester
|
|||
@Override
|
||||
public void parsedHeader(HttpField field)
|
||||
{
|
||||
put(field.getName(),field.getValue());
|
||||
add(field.getName(),field.getValue());
|
||||
}
|
||||
|
||||
@Override
|
||||
|
|
|
@ -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;
|
||||
|
|
|
@ -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)
|
||||
|
|
|
@ -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();
|
||||
|
|
Loading…
Reference in New Issue