From dfaf663ab917e6fa442ebc632e89e23cbf418a50 Mon Sep 17 00:00:00 2001 From: Ezequiel Werner Date: Fri, 26 Aug 2022 12:51:43 -0300 Subject: [PATCH] W-11410315: Notify HttpProbes when a BadRequest is detected during header parsing (#56) --- modules/http/pom.xml | 6 + .../grizzly/http/HttpServerFilter.java | 6 +- .../HttpBadRequestNotifiesProbesTest.java | 111 ++++++++++++++++++ 3 files changed, 121 insertions(+), 2 deletions(-) create mode 100644 modules/http/src/test/java/org/glassfish/grizzly/http/HttpBadRequestNotifiesProbesTest.java diff --git a/modules/http/pom.xml b/modules/http/pom.xml index ac2cd45254..79fef27687 100644 --- a/modules/http/pom.xml +++ b/modules/http/pom.xml @@ -103,5 +103,11 @@ org.mule.glassfish.grizzly grizzly-framework + + org.mockito + mockito-all + 1.9.5 + test + diff --git a/modules/http/src/main/java/org/glassfish/grizzly/http/HttpServerFilter.java b/modules/http/src/main/java/org/glassfish/grizzly/http/HttpServerFilter.java index 9027a6912c..2a7f95e340 100644 --- a/modules/http/src/main/java/org/glassfish/grizzly/http/HttpServerFilter.java +++ b/modules/http/src/main/java/org/glassfish/grizzly/http/HttpServerFilter.java @@ -62,6 +62,7 @@ import java.util.concurrent.TimeUnit; import org.glassfish.grizzly.ThreadCache; +import static org.glassfish.grizzly.http.HttpProbeNotifier.notifyDataSent; import static org.glassfish.grizzly.http.Method.PayloadExpectation; import static org.glassfish.grizzly.http.util.HttpCodecUtils.*; import org.glassfish.grizzly.http.util.HttpUtils; @@ -791,7 +792,7 @@ protected void onHttpHeaderError(final HttpHeader httpHeader, final FilterChainContext ctx, final Throwable t) throws IOException { - final ServerHttpRequestImpl request = (ServerHttpRequestImpl) httpHeader; + final HttpRequestPacket request = (HttpRequestPacket) httpHeader; final HttpResponsePacket response = request.getResponse(); if (t instanceof HttpErrorException) { @@ -1212,12 +1213,13 @@ private void sendErrorResponse(final FilterChainContext ctx, /* - * caller has the responsibility to set the status of th response. + * caller has the responsibility to set the status of the response. */ private void commitAndCloseAsError(FilterChainContext ctx, HttpResponsePacket response) { final HttpContent errorHttpResponse = customizeErrorResponse(response); final Buffer resBuf = encodeHttpPacket(ctx, errorHttpResponse); ctx.write(resBuf); + notifyDataSent(this, ctx.getConnection(), resBuf); response.getProcessingState().getHttpContext().close(); } diff --git a/modules/http/src/test/java/org/glassfish/grizzly/http/HttpBadRequestNotifiesProbesTest.java b/modules/http/src/test/java/org/glassfish/grizzly/http/HttpBadRequestNotifiesProbesTest.java new file mode 100644 index 0000000000..32d5ea0e41 --- /dev/null +++ b/modules/http/src/test/java/org/glassfish/grizzly/http/HttpBadRequestNotifiesProbesTest.java @@ -0,0 +1,111 @@ +/* + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS HEADER. + * + * Copyright (c) 2010-2015 Oracle and/or its affiliates. All rights reserved. + * + * The contents of this file are subject to the terms of either the GNU + * General Public License Version 2 only ("GPL") or the Common Development + * and Distribution License("CDDL") (collectively, the "License"). You + * may not use this file except in compliance with the License. You can + * obtain a copy of the License at + * https://glassfish.dev.java.net/public/CDDL+GPL_1_1.html + * or packager/legal/LICENSE.txt. See the License for the specific + * language governing permissions and limitations under the License. + * + * When distributing the software, include this License Header Notice in each + * file and include the License file at packager/legal/LICENSE.txt. + * + * GPL Classpath Exception: + * Oracle designates this particular file as subject to the "Classpath" + * exception as provided by Oracle in the GPL Version 2 section of the License + * file that accompanied this code. + * + * Modifications: + * If applicable, add the following below the License Header, with the fields + * enclosed by brackets [] replaced by your own identifying information: + * "Portions Copyright [year] [name of copyright owner]" + * + * Contributor(s): + * If you wish your version of this file to be governed by only the CDDL or + * only the GPL Version 2, indicate your decision by adding "[Contributor] + * elects to include this software in this distribution under the [CDDL or GPL + * Version 2] license." If you don't indicate a single choice of license, a + * recipient has the option to distribute your version of this file under + * either the CDDL, the GPL Version 2 or to extend the choice of license to + * its licensees as provided above. However, if you add GPL Version 2 code + * and therefore, elected the GPL Version 2 license, then the option applies + * only if the new code is made subject to such option by the copyright + * holder. + */ + +package org.glassfish.grizzly.http; + +import static org.glassfish.grizzly.http.util.HttpStatus.REQUEST_URI_TOO_LONG_414; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.io.IOException; + +import junit.framework.TestCase; +import org.glassfish.grizzly.Buffer; +import org.glassfish.grizzly.Connection; +import org.glassfish.grizzly.filterchain.FilterChainContext; +import org.glassfish.grizzly.http.HttpCodecFilter.HeaderParsingState; + +public class HttpBadRequestNotifiesProbesTest extends TestCase { + + public static final int PORT = 19004; + private static final int MAX_HEADERS_SIZE = 8192; + + private HttpServerFilter httpServerFilter; + private HttpProbe testProbe; + + @Override + protected void setUp() throws Exception { + super.setUp(); + + testProbe = spy(new HttpProbe.Adapter()); + + httpServerFilter = new HttpServerFilter(false, MAX_HEADERS_SIZE, new KeepAlive(), null); + httpServerFilter.getMonitoringConfig().addProbes(testProbe); + } + + public void testProbesAreNotifiedOnBadRequest() throws IOException { + FilterChainContext ctx = mock(FilterChainContext.class); + HttpRequestPacket httpHeader = mock(HttpRequestPacket.class); + + HttpResponsePacket errorResponse = mock(HttpResponsePacket.class); + when(httpHeader.getResponse()).thenReturn(errorResponse); + when(errorResponse.getHttpStatus()).thenReturn(REQUEST_URI_TOO_LONG_414); + when(errorResponse.isCommitted()).thenReturn(true); + + ProcessingState processingState = new ProcessingState(); + HttpContext httpContext = mock(HttpContext.class); + processingState.setHttpContext(httpContext); + when(errorResponse.getProcessingState()).thenReturn(processingState); + + Buffer input = mock(Buffer.class); + when(ctx.getMessage()).thenReturn(input); + when(input.hasArray()).thenReturn(true); + + Exception mockError = new IllegalStateException("Mock URI too long"); + when(input.limit()).thenThrow(mockError); + + Connection connection = mock(Connection.class); + when(ctx.getConnection()).thenReturn(connection); + + HttpPacketParsing parsingState = mock(HttpPacketParsing.class); + when(httpHeader.getParsingState()).thenReturn(parsingState); + when(parsingState.isHeaderParsed()).thenReturn(false); + + HeaderParsingState headerParsingState = new HeaderParsingState(); + when(parsingState.getHeaderParsingState()).thenReturn(headerParsingState); + + httpServerFilter.handleRead(ctx, httpHeader); + verify(testProbe).onDataReceivedEvent(connection, input); + verify(testProbe).onErrorEvent(connection, httpHeader, mockError); + verify(testProbe).onDataSentEvent(connection, null); + } +}