Issue #448 - RFC2616 Compliance Mode should track and report RFC7230 violations
+ added Optional behavior to record violations in a Request attribute + added servlet testcase showing this violation recording abillity + currently the recorded violation doesn't report the correct compliance mode that it violates + there is still a problem with a quoted empty string value from Issue #451
This commit is contained in:
parent
0e2472b8e1
commit
7be58f9730
|
@ -21,6 +21,8 @@ package org.eclipse.jetty.server;
|
|||
|
||||
import java.io.IOException;
|
||||
import java.nio.ByteBuffer;
|
||||
import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
|
||||
import org.eclipse.jetty.http.BadMessageException;
|
||||
import org.eclipse.jetty.http.HostPortHttpField;
|
||||
|
@ -48,6 +50,7 @@ public class HttpChannelOverHttp extends HttpChannel implements HttpParser.Reque
|
|||
{
|
||||
private static final Logger LOG = Log.getLogger(HttpChannelOverHttp.class);
|
||||
private final static HttpField PREAMBLE_UPGRADE_H2C = new HttpField(HttpHeader.UPGRADE,"h2c");
|
||||
private static final String ATTR_COMPLIANCE_VIOLATIONS = "org.eclipse.jetty.http.compliance.violations";
|
||||
|
||||
private final HttpFields _fields = new HttpFields();
|
||||
private final MetaData.Request _metadata = new MetaData.Request(_fields);
|
||||
|
@ -58,7 +61,7 @@ public class HttpChannelOverHttp extends HttpChannel implements HttpParser.Reque
|
|||
private boolean _unknownExpectation = false;
|
||||
private boolean _expect100Continue = false;
|
||||
private boolean _expect102Processing = false;
|
||||
|
||||
private List<String> _complianceViolations;
|
||||
|
||||
public HttpChannelOverHttp(HttpConnection httpConnection, Connector connector, HttpConfiguration config, EndPoint endPoint, HttpTransport transport)
|
||||
{
|
||||
|
@ -444,6 +447,8 @@ public class HttpChannelOverHttp extends HttpChannel implements HttpParser.Reque
|
|||
@Override
|
||||
public boolean messageComplete()
|
||||
{
|
||||
if(_complianceViolations != null)
|
||||
this.getRequest().setAttribute(ATTR_COMPLIANCE_VIOLATIONS, _complianceViolations);
|
||||
return onRequestComplete();
|
||||
}
|
||||
|
||||
|
@ -456,7 +461,16 @@ public class HttpChannelOverHttp extends HttpChannel implements HttpParser.Reque
|
|||
@Override
|
||||
public void onComplianceViolation(HttpCompliance compliance,HttpCompliance required, String reason)
|
||||
{
|
||||
if (LOG.isDebugEnabled())
|
||||
LOG.debug(compliance+"<="+required+": "+reason+" for "+getHttpTransport());
|
||||
if (_httpConnection.isRecordHttpComplianceViolations())
|
||||
{
|
||||
if (_complianceViolations == null)
|
||||
{
|
||||
_complianceViolations = new ArrayList<>();
|
||||
}
|
||||
String violation = String.format("%s<=%s: %s for %s", compliance, required, reason, getHttpTransport());
|
||||
_complianceViolations.add(violation);
|
||||
if (LOG.isDebugEnabled())
|
||||
LOG.debug(violation);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -71,6 +71,7 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http
|
|||
private final BlockingReadCallback _blockingReadCallback = new BlockingReadCallback();
|
||||
private final AsyncReadCallback _asyncReadCallback = new AsyncReadCallback();
|
||||
private final SendCallback _sendCallback = new SendCallback();
|
||||
private boolean _recordHttpComplianceViolations = false;
|
||||
|
||||
/**
|
||||
* Get the current connection that this thread is dispatched to.
|
||||
|
@ -110,6 +111,16 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http
|
|||
return _config;
|
||||
}
|
||||
|
||||
public boolean isRecordHttpComplianceViolations()
|
||||
{
|
||||
return _recordHttpComplianceViolations;
|
||||
}
|
||||
|
||||
public void setRecordHttpComplianceViolations(boolean recordHttpComplianceViolations)
|
||||
{
|
||||
this._recordHttpComplianceViolations = recordHttpComplianceViolations;
|
||||
}
|
||||
|
||||
protected HttpGenerator newHttpGenerator()
|
||||
{
|
||||
return new HttpGenerator(_config.getSendServerVersion(),_config.getSendXPoweredBy());
|
||||
|
@ -117,7 +128,9 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http
|
|||
|
||||
protected HttpChannelOverHttp newHttpChannel()
|
||||
{
|
||||
return new HttpChannelOverHttp(this, _connector, _config, getEndPoint(), this);
|
||||
HttpChannelOverHttp httpChannel = new HttpChannelOverHttp(this, _connector, _config, getEndPoint(), this);
|
||||
|
||||
return httpChannel;
|
||||
}
|
||||
|
||||
protected HttpParser newHttpParser(HttpCompliance compliance)
|
||||
|
|
|
@ -33,6 +33,7 @@ public class HttpConnectionFactory extends AbstractConnectionFactory implements
|
|||
{
|
||||
private final HttpConfiguration _config;
|
||||
private HttpCompliance _httpCompliance;
|
||||
private boolean _recordHttpComplianceViolations = false;
|
||||
|
||||
public HttpConnectionFactory()
|
||||
{
|
||||
|
@ -65,6 +66,11 @@ public class HttpConnectionFactory extends AbstractConnectionFactory implements
|
|||
return _httpCompliance;
|
||||
}
|
||||
|
||||
public boolean isRecordHttpComplianceViolations()
|
||||
{
|
||||
return _recordHttpComplianceViolations;
|
||||
}
|
||||
|
||||
/**
|
||||
* @param httpCompliance String value of {@link HttpCompliance}
|
||||
*/
|
||||
|
@ -76,8 +82,14 @@ public class HttpConnectionFactory extends AbstractConnectionFactory implements
|
|||
@Override
|
||||
public Connection newConnection(Connector connector, EndPoint endPoint)
|
||||
{
|
||||
return configure(new HttpConnection(_config, connector, endPoint, _httpCompliance), connector, endPoint);
|
||||
HttpConnection conn = new HttpConnection(_config, connector, endPoint, _httpCompliance);
|
||||
conn.setRecordHttpComplianceViolations(_recordHttpComplianceViolations);
|
||||
return configure(conn, connector, endPoint);
|
||||
}
|
||||
|
||||
|
||||
public void setRecordHttpComplianceViolations(boolean recordHttpComplianceViolations)
|
||||
{
|
||||
this._recordHttpComplianceViolations = recordHttpComplianceViolations;
|
||||
}
|
||||
}
|
||||
|
|
|
@ -0,0 +1,208 @@
|
|||
//
|
||||
// ========================================================================
|
||||
// Copyright (c) 1995-2016 Mort Bay Consulting Pty. Ltd.
|
||||
// ------------------------------------------------------------------------
|
||||
// All rights reserved. This program and the accompanying materials
|
||||
// are made available under the terms of the Eclipse Public License v1.0
|
||||
// and Apache License v2.0 which accompanies this distribution.
|
||||
//
|
||||
// The Eclipse Public License is available at
|
||||
// http://www.eclipse.org/legal/epl-v10.html
|
||||
//
|
||||
// The Apache License v2.0 is available at
|
||||
// http://www.opensource.org/licenses/apache2.0.php
|
||||
//
|
||||
// You may elect to redistribute this code under either of these licenses.
|
||||
// ========================================================================
|
||||
//
|
||||
|
||||
package org.eclipse.jetty.servlet;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.io.PrintWriter;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Collections;
|
||||
import java.util.EnumSet;
|
||||
import java.util.List;
|
||||
|
||||
import javax.servlet.DispatcherType;
|
||||
import javax.servlet.Filter;
|
||||
import javax.servlet.FilterChain;
|
||||
import javax.servlet.FilterConfig;
|
||||
import javax.servlet.ServletException;
|
||||
import javax.servlet.ServletRequest;
|
||||
import javax.servlet.ServletResponse;
|
||||
import javax.servlet.http.HttpServlet;
|
||||
import javax.servlet.http.HttpServletRequest;
|
||||
import javax.servlet.http.HttpServletResponse;
|
||||
|
||||
import org.eclipse.jetty.http.HttpCompliance;
|
||||
import org.eclipse.jetty.server.HttpConfiguration;
|
||||
import org.eclipse.jetty.server.HttpConnectionFactory;
|
||||
import org.eclipse.jetty.server.LocalConnector;
|
||||
import org.eclipse.jetty.server.Server;
|
||||
import org.junit.AfterClass;
|
||||
import org.junit.BeforeClass;
|
||||
import org.junit.Test;
|
||||
|
||||
import static org.hamcrest.Matchers.containsString;
|
||||
import static org.junit.Assert.assertThat;
|
||||
|
||||
public class ComplianceViolations2616Test
|
||||
{
|
||||
private static Server server;
|
||||
private static LocalConnector connector;
|
||||
|
||||
public static class ReportViolationsFilter implements Filter
|
||||
{
|
||||
@Override
|
||||
public void init(FilterConfig filterConfig) throws ServletException
|
||||
{
|
||||
}
|
||||
|
||||
@Override
|
||||
public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException
|
||||
{
|
||||
if (request instanceof HttpServletRequest)
|
||||
{
|
||||
List<String> violations = (List<String>) request.getAttribute("org.eclipse.jetty.http.compliance.violations");
|
||||
if (violations != null)
|
||||
{
|
||||
HttpServletResponse httpResponse = (HttpServletResponse) response;
|
||||
int i = 0;
|
||||
for (String violation : violations)
|
||||
{
|
||||
httpResponse.setHeader("X-Http-Violation-" + (i++), violation);
|
||||
}
|
||||
}
|
||||
}
|
||||
chain.doFilter(request, response);
|
||||
}
|
||||
|
||||
@Override
|
||||
public void destroy()
|
||||
{
|
||||
}
|
||||
}
|
||||
|
||||
public static class DumpRequestHeadersServlet extends HttpServlet
|
||||
{
|
||||
@Override
|
||||
protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
|
||||
{
|
||||
resp.setContentType("text/plain");
|
||||
PrintWriter out = resp.getWriter();
|
||||
List<String> headerNames = new ArrayList<>();
|
||||
headerNames.addAll(Collections.list(req.getHeaderNames()));
|
||||
Collections.sort(headerNames);
|
||||
for (String name : headerNames)
|
||||
{
|
||||
out.printf("[%s] = [%s]%n", name, req.getHeader(name));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@BeforeClass
|
||||
public static void startServer() throws Exception
|
||||
{
|
||||
server = new Server();
|
||||
|
||||
HttpConfiguration config = new HttpConfiguration();
|
||||
config.setSendServerVersion(false);
|
||||
|
||||
HttpConnectionFactory httpConnectionFactory = new HttpConnectionFactory(config, HttpCompliance.RFC2616);
|
||||
httpConnectionFactory.setRecordHttpComplianceViolations(true);
|
||||
connector = new LocalConnector(server, null, null, null, -1, httpConnectionFactory);
|
||||
|
||||
ServletContextHandler context = new ServletContextHandler();
|
||||
context.setContextPath("/");
|
||||
context.setWelcomeFiles(new String[]{"index.html", "index.jsp", "index.htm"});
|
||||
|
||||
context.addServlet(DumpRequestHeadersServlet.class, "/dump/*");
|
||||
context.addFilter(ReportViolationsFilter.class, "/*", EnumSet.of(DispatcherType.REQUEST));
|
||||
|
||||
server.setHandler(context);
|
||||
server.addConnector(connector);
|
||||
|
||||
server.start();
|
||||
}
|
||||
|
||||
@AfterClass
|
||||
public static void stopServer() throws Exception
|
||||
{
|
||||
server.stop();
|
||||
server.join();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testNoColonHeader_Middle() throws Exception
|
||||
{
|
||||
StringBuffer req1 = new StringBuffer();
|
||||
req1.append("GET /dump/ HTTP/1.1\r\n");
|
||||
req1.append("Name\r\n");
|
||||
req1.append("Host: local\r\n");
|
||||
req1.append("Accept: */*\r\n");
|
||||
req1.append("Connection: close\r\n");
|
||||
req1.append("\r\n");
|
||||
|
||||
String response = connector.getResponses(req1.toString());
|
||||
assertThat("Response status", response, containsString("HTTP/1.1 200 OK"));
|
||||
assertThat("Response headers", response, containsString("X-Http-Violation-0: RFC7230 (name only header)"));
|
||||
|
||||
assertThat("Response body", response, containsString("[Name] = []"));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testNoColonHeader_End() throws Exception
|
||||
{
|
||||
StringBuffer req1 = new StringBuffer();
|
||||
req1.append("GET /dump/ HTTP/1.1\r\n");
|
||||
req1.append("Host: local\r\n");
|
||||
req1.append("Connection: close\r\n");
|
||||
req1.append("Accept: */*\r\n");
|
||||
req1.append("Name\r\n");
|
||||
req1.append("\r\n");
|
||||
|
||||
String response = connector.getResponses(req1.toString());
|
||||
assertThat("Response status", response, containsString("HTTP/1.1 200"));
|
||||
assertThat("Response headers", response, containsString("X-Http-Violation-0: RFC7230 (name only header)"));
|
||||
|
||||
assertThat("Response body", response, containsString("[Name] = []"));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testEmptyQuotedHeader() throws Exception
|
||||
{
|
||||
StringBuffer req1 = new StringBuffer();
|
||||
req1.append("GET /dump/ HTTP/1.1\r\n");
|
||||
req1.append("Host: local\r\n");
|
||||
req1.append("Name: \"\"\r\n");
|
||||
req1.append("Connection: close\r\n");
|
||||
req1.append("Accept: */*\r\n");
|
||||
req1.append("\r\n");
|
||||
|
||||
String response = connector.getResponses(req1.toString());
|
||||
assertThat("Response status", response, containsString("HTTP/1.1 200"));
|
||||
assertThat("Response headers", response, containsString("X-Http-Violation-0: RFC7230 (name only header)"));
|
||||
|
||||
assertThat("Response body", response, containsString("[Name] = []"));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testEmptyHeader() throws Exception
|
||||
{
|
||||
StringBuffer req1 = new StringBuffer();
|
||||
req1.append("GET /dump/ HTTP/1.1\r\n");
|
||||
req1.append("Host: local\r\n");
|
||||
req1.append("Name:\r\n");
|
||||
req1.append("Connection: close\r\n");
|
||||
req1.append("Accept: */*\r\n");
|
||||
req1.append("\r\n");
|
||||
|
||||
String response = connector.getResponses(req1.toString());
|
||||
assertThat("Response status", response, containsString("HTTP/1.1 200"));
|
||||
assertThat("Response headers", response, containsString("X-Http-Violation-0: RFC7230 (name only header)"));
|
||||
|
||||
assertThat("Response body", response, containsString("[Name] = []"));
|
||||
}
|
||||
}
|
Loading…
Reference in New Issue