Persistent HttpFields (#10339)

Use a marked field rather than freeze/thaw, to support fields that cannot be removed.
This commit is contained in:
Greg Wilkins 2023-08-25 13:02:32 +10:00 committed by GitHub
parent ff2e36e239
commit 4086c7ee47
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 210 additions and 23 deletions

View File

@ -1582,6 +1582,13 @@ public interface HttpFields extends Iterable<HttpField>, Supplier<HttpFields>
return this;
}
@Override
public Mutable clear()
{
_fields.clear();
return this;
}
@Override
public ListIterator<HttpField> listIterator()
{

View File

@ -46,11 +46,6 @@ public class HttpGenerator
private static final byte[] __colon_space = new byte[]{':', ' '};
public static final MetaData.Response CONTINUE_100_INFO = new MetaData.Response(100, null, HttpVersion.HTTP_1_1, HttpFields.EMPTY);
public static final MetaData.Response PROGRESS_102_INFO = new MetaData.Response(102, null, HttpVersion.HTTP_1_1, HttpFields.EMPTY);
public static final MetaData.Response RESPONSE_400_INFO =
new MetaData.Response(HttpStatus.BAD_REQUEST_400, null, HttpVersion.HTTP_1_1, HttpFields.build().add(HttpFields.CONNECTION_CLOSE), 0);
public static final MetaData.Response RESPONSE_500_INFO =
new MetaData.Response(INTERNAL_SERVER_ERROR_500, null, HttpVersion.HTTP_1_1, HttpFields.build().add(HttpFields.CONNECTION_CLOSE), 0);
// states
public enum State
@ -808,12 +803,6 @@ public class HttpGenerator
private static final byte[] CONNECTION_CLOSE = StringUtil.getBytes("Connection: close\r\n");
private static final byte[] HTTP_1_1_SPACE = StringUtil.getBytes(HttpVersion.HTTP_1_1 + " ");
private static final byte[] TRANSFER_ENCODING_CHUNKED = StringUtil.getBytes("Transfer-Encoding: chunked\r\n");
private static final byte[][] SEND = new byte[][]{
new byte[0],
StringUtil.getBytes("Server: Jetty(12.x.x)\r\n"),
StringUtil.getBytes("X-Powered-By: Jetty(12.x.x)\r\n"),
StringUtil.getBytes("Server: Jetty(12.x.x)\r\nX-Powered-By: Jetty(12.x.x)\r\n")
};
// Build cache of response lines for status
private static class PreparedResponse

View File

@ -33,12 +33,12 @@ import org.eclipse.jetty.http.DateGenerator;
import org.eclipse.jetty.http.HttpField;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.MimeTypes;
import org.eclipse.jetty.http.PreEncodedHttpField;
import org.eclipse.jetty.io.ArrayByteBufferPool;
import org.eclipse.jetty.io.ByteBufferPool;
import org.eclipse.jetty.io.Connection;
import org.eclipse.jetty.server.handler.ContextHandler;
import org.eclipse.jetty.server.handler.ErrorHandler;
import org.eclipse.jetty.server.internal.ResponseHttpFields;
import org.eclipse.jetty.util.Attributes;
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.util.DecoratedObjectFactory;
@ -474,6 +474,11 @@ public class Server extends Handler.Wrapper implements Attributes
_dumpBeforeStop = dumpBeforeStop;
}
/**
* @return A {@link HttpField} instance efficiently recording the current time to a second resolution,
* that cannot be cleared from a {@link ResponseHttpFields} instance.
* @see ResponseHttpFields.PersistentPreEncodedHttpField
*/
public HttpField getDateField()
{
long now = System.currentTimeMillis();
@ -487,7 +492,7 @@ public class Server extends Handler.Wrapper implements Attributes
df = _dateField;
if (df == null || df._seconds != seconds)
{
HttpField field = new PreEncodedHttpField(HttpHeader.DATE, DateGenerator.formatDate(now));
HttpField field = new ResponseHttpFields.PersistentPreEncodedHttpField(HttpHeader.DATE, DateGenerator.formatDate(now));
_dateField = new DateField(seconds, field);
return field;
}

View File

@ -37,7 +37,6 @@ import org.eclipse.jetty.http.HttpURI;
import org.eclipse.jetty.http.HttpVersion;
import org.eclipse.jetty.http.MetaData;
import org.eclipse.jetty.http.MultiPartFormData.Parts;
import org.eclipse.jetty.http.PreEncodedHttpField;
import org.eclipse.jetty.http.Trailers;
import org.eclipse.jetty.http.UriCompliance;
import org.eclipse.jetty.io.ByteBufferPool;
@ -97,8 +96,8 @@ public class HttpChannelState implements HttpChannel, Components
private static final Logger LOG = LoggerFactory.getLogger(HttpChannelState.class);
private static final Throwable DO_NOT_SEND = new Throwable("No Send");
private static final HttpField SERVER_VERSION = new PreEncodedHttpField(HttpHeader.SERVER, HttpConfiguration.SERVER_VERSION);
private static final HttpField POWERED_BY = new PreEncodedHttpField(HttpHeader.X_POWERED_BY, HttpConfiguration.SERVER_VERSION);
private static final HttpField SERVER_VERSION = new ResponseHttpFields.PersistentPreEncodedHttpField(HttpHeader.SERVER, HttpConfiguration.SERVER_VERSION);
private static final HttpField POWERED_BY = new ResponseHttpFields.PersistentPreEncodedHttpField(HttpHeader.X_POWERED_BY, HttpConfiguration.SERVER_VERSION);
private final AutoLock _lock = new AutoLock();
private final HandlerInvoker _handlerInvoker = new HandlerInvoker();
@ -273,8 +272,8 @@ public class HttpChannelState implements HttpChannel, Components
_request = new ChannelRequest(this, request);
_response = new ChannelResponse(_request);
HttpFields.Mutable responseHeaders = _response.getHeaders();
HttpConfiguration httpConfiguration = getHttpConfiguration();
HttpFields.Mutable responseHeaders = _response.getHeaders();
if (httpConfiguration.getSendServerVersion())
responseHeaders.add(SERVER_VERSION);
if (httpConfiguration.getSendXPoweredBy())

View File

@ -20,9 +20,13 @@ import java.util.stream.Stream;
import org.eclipse.jetty.http.HttpField;
import org.eclipse.jetty.http.HttpFields;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.PreEncodedHttpField;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import static org.eclipse.jetty.server.internal.ResponseHttpFields.PersistentField.isPersistent;
public class ResponseHttpFields implements HttpFields.Mutable
{
private static final Logger LOG = LoggerFactory.getLogger(ResponseHttpFields.class);
@ -88,7 +92,17 @@ public class ResponseHttpFields implements HttpFields.Mutable
@Override
public Mutable clear()
{
return _committed.get() ? this : _fields.clear();
if (!_committed.get())
{
// TODO iterate backwards when the list iterator of that form is available
for (Iterator<HttpField> iterator = _fields.iterator(); iterator.hasNext();)
{
HttpField field = iterator.next();
if (!PersistentField.isPersistent(field))
iterator.remove();
}
}
return this;
}
@Override
@ -104,6 +118,8 @@ public class ResponseHttpFields implements HttpFields.Mutable
Iterator<HttpField> i = _fields.iterator();
return new Iterator<>()
{
HttpField _current;
@Override
public boolean hasNext()
{
@ -113,7 +129,8 @@ public class ResponseHttpFields implements HttpFields.Mutable
@Override
public HttpField next()
{
return i.next();
_current = i.next();
return _current;
}
@Override
@ -121,7 +138,10 @@ public class ResponseHttpFields implements HttpFields.Mutable
{
if (_committed.get())
throw new UnsupportedOperationException("Read Only");
if (isPersistent(_current))
throw new IllegalStateException("Persistent field");
i.remove();
_current = null;
}
};
}
@ -132,6 +152,8 @@ public class ResponseHttpFields implements HttpFields.Mutable
ListIterator<HttpField> i = _fields.listIterator();
return new ListIterator<>()
{
HttpField _current;
@Override
public boolean hasNext()
{
@ -141,7 +163,8 @@ public class ResponseHttpFields implements HttpFields.Mutable
@Override
public HttpField next()
{
return i.next();
_current = i.next();
return _current;
}
@Override
@ -153,7 +176,8 @@ public class ResponseHttpFields implements HttpFields.Mutable
@Override
public HttpField previous()
{
return i.previous();
_current = i.previous();
return _current;
}
@Override
@ -173,7 +197,10 @@ public class ResponseHttpFields implements HttpFields.Mutable
{
if (_committed.get())
throw new UnsupportedOperationException("Read Only");
if (isPersistent(_current))
throw new IllegalStateException("Persistent field");
i.remove();
_current = null;
}
@Override
@ -181,10 +208,23 @@ public class ResponseHttpFields implements HttpFields.Mutable
{
if (_committed.get())
throw new UnsupportedOperationException("Read Only");
if (isPersistent(_current))
{
// cannot change the field name
if (field == null || !field.isSameName(_current))
throw new IllegalStateException("Persistent field");
// new field must also be persistent
if (!isPersistent(field))
field = (field instanceof PreEncodedHttpField)
? new PersistentPreEncodedHttpField(field.getHeader(), field.getValue())
: new PersistentHttpField(field);
}
if (field == null)
i.remove();
else
i.set(field);
_current = field;
}
@Override
@ -203,4 +243,59 @@ public class ResponseHttpFields implements HttpFields.Mutable
{
return _fields.toString();
}
/**
* A marker interface for {@link HttpField}s that cannot be {@link #remove(HttpHeader) removed} or {@link #clear() cleared}
* from a {@link ResponseHttpFields} instance. Persistent fields are not immutable in the {@link ResponseHttpFields}
* and may be replaced with a different value.
*/
public interface PersistentField
{
static boolean isPersistent(HttpField field)
{
return field instanceof PersistentField;
}
}
/**
* A {@link HttpField} that is a {@link PersistentField}.
*/
public static class PersistentHttpField extends HttpField implements PersistentField
{
private final HttpField _field;
public PersistentHttpField(HttpField field)
{
super(field.getHeader(), field.getName(), field.getValue());
_field = field;
}
@Override
public int getIntValue()
{
return _field.getIntValue();
}
@Override
public long getLongValue()
{
return _field.getIntValue();
}
}
/**
* A {@link PreEncodedHttpField} that is a {@link PersistentField}.
*/
public static class PersistentPreEncodedHttpField extends PreEncodedHttpField implements PersistentField
{
public PersistentPreEncodedHttpField(HttpHeader header, String value)
{
this(header, value, true);
}
public PersistentPreEncodedHttpField(HttpHeader header, String value, boolean immutable)
{
super(header, value);
}
}
}

View File

@ -13,6 +13,7 @@
package org.eclipse.jetty.server;
import java.util.Iterator;
import java.util.List;
import java.util.ListIterator;
@ -34,6 +35,8 @@ import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.notNullValue;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
public class ResponseTest
{
@ -56,6 +59,97 @@ public class ResponseTest
connector = null;
}
@Test
public void testGET() throws Exception
{
server.setHandler(new Handler.Abstract()
{
@Override
public boolean handle(Request request, Response response, Callback callback)
{
callback.succeeded();
return true;
}
});
server.start();
String request = """
GET /path HTTP/1.0\r
Host: hostname\r
\r
""";
HttpTester.Response response = HttpTester.parseResponse(connector.getResponse(request));
assertEquals(HttpStatus.OK_200, response.getStatus());
}
@Test
public void testServerDateFieldsFrozen() throws Exception
{
server.setHandler(new Handler.Abstract()
{
@Override
public boolean handle(Request request, Response response, Callback callback)
{
response.getHeaders().add("Temp", "field");
response.getHeaders().add("Test", "before reset");
assertThrows(IllegalStateException.class, () -> response.getHeaders().remove(HttpHeader.SERVER));
assertThrows(IllegalStateException.class, () -> response.getHeaders().remove(HttpHeader.DATE));
response.getHeaders().remove("Temp");
response.getHeaders().add("Temp", "field");
Iterator<HttpField> iterator = response.getHeaders().iterator();
assertThat(iterator.next().getHeader(), is(HttpHeader.SERVER));
assertThrows(IllegalStateException.class, iterator::remove);
assertThat(iterator.next().getHeader(), is(HttpHeader.DATE));
assertThrows(IllegalStateException.class, iterator::remove);
assertThat(iterator.next().getName(), is("Test"));
assertThat(iterator.next().getName(), is("Temp"));
iterator.remove();
assertFalse(response.getHeaders().contains("Temp"));
assertThrows(IllegalStateException.class, () -> response.getHeaders().remove(HttpHeader.SERVER));
assertFalse(iterator.hasNext());
ListIterator<HttpField> listIterator = response.getHeaders().listIterator();
assertThat(listIterator.next().getHeader(), is(HttpHeader.SERVER));
assertThrows(IllegalStateException.class, listIterator::remove);
assertThat(listIterator.next().getHeader(), is(HttpHeader.DATE));
assertThrows(IllegalStateException.class, () -> listIterator.set(new HttpField("Something", "else")));
listIterator.set(new HttpField(HttpHeader.DATE, "1970-01-01"));
assertThat(listIterator.previous().getHeader(), is(HttpHeader.DATE));
assertThrows(IllegalStateException.class, listIterator::remove);
assertThat(listIterator.previous().getHeader(), is(HttpHeader.SERVER));
assertThrows(IllegalStateException.class, listIterator::remove);
assertThat(listIterator.next().getHeader(), is(HttpHeader.SERVER));
assertThat(listIterator.next().getHeader(), is(HttpHeader.DATE));
assertThrows(IllegalStateException.class, listIterator::remove);
listIterator.add(new HttpField("Temp", "value"));
assertThat(listIterator.previous().getName(), is("Temp"));
listIterator.remove();
assertFalse(response.getHeaders().contains("Temp"));
response.getHeaders().add("Temp", "field");
response.reset();
response.getHeaders().add("Test", "after reset");
response.getHeaders().put(HttpHeader.SERVER, "jettyrocks");
assertThrows(IllegalStateException.class, () -> response.getHeaders().put(HttpHeader.SERVER, (String)null));
callback.succeeded();
return true;
}
});
server.start();
String request = """
GET /path HTTP/1.0\r
Host: hostname\r
\r
""";
HttpTester.Response response = HttpTester.parseResponse(connector.getResponse(request));
assertEquals(HttpStatus.OK_200, response.getStatus());
assertThat(response.get(HttpHeader.SERVER), notNullValue());
assertThat(response.get(HttpHeader.DATE), notNullValue());
assertThat(response.get("Test"), is("after reset"));
}
@Test
public void testRedirectGET() throws Exception
{

View File

@ -130,8 +130,6 @@ public abstract class AbstractHandshaker implements Handshaker
connectionMetaData.getConnector().getEventListeners().forEach(connection::addEventListener);
prepareResponse(response, negotiation);
if (httpConfig.getSendServerVersion())
response.getHeaders().put(SERVER_VERSION);
request.setAttribute(HttpStream.UPGRADE_CONNECTION_ATTRIBUTE, connection);