Merge pull request #296 from nite23/blacklist_fix

Capture blacklisted requests in HAR

Author: ericbeland

browserup-proxy-core/src/main/java/com/browserup/bup/BrowserUpProxyServer.java

@@ -465,6 +465,21 @@ public SSLEngine newSslEngine() {
         proxyServer = bootstrap.start();
 
         addHarCaptureFilter();
+
+        addHttpFilterFactory(new HttpFiltersSourceAdapter() {
+            @Override
+            public HttpFilters filterRequest(HttpRequest originalRequest, ChannelHandlerContext ctx) {
+                return new BlacklistFilter(originalRequest, ctx, getBlacklist());
+            }
+        });
+
+        addHttpFilterFactory(new HttpFiltersSourceAdapter() {
+            @Override
+            public HttpFilters filterRequest(HttpRequest originalRequest, ChannelHandlerContext ctx) {
+                Whitelist currentWhitelist = whitelist.get();
+                return new WhitelistFilter(originalRequest, ctx, isWhitelistEnabled(), currentWhitelist.getStatusCode(), currentWhitelist.getPatterns());
+            }
+        });
     }
 
     @Override
@@ -1453,21 +1468,6 @@ public HttpFilters filterRequest(HttpRequest originalRequest, ChannelHandlerCont
             }
         });
 
-        addHttpFilterFactory(new HttpFiltersSourceAdapter() {
-            @Override
-            public HttpFilters filterRequest(HttpRequest originalRequest, ChannelHandlerContext ctx) {
-                return new BlacklistFilter(originalRequest, ctx, getBlacklist());
-            }
-        });
-
-        addHttpFilterFactory(new HttpFiltersSourceAdapter() {
-            @Override
-            public HttpFilters filterRequest(HttpRequest originalRequest, ChannelHandlerContext ctx) {
-                Whitelist currentWhitelist = whitelist.get();
-                return new WhitelistFilter(originalRequest, ctx, isWhitelistEnabled(), currentWhitelist.getStatusCode(), currentWhitelist.getPatterns());
-            }
-        });
-
         addHttpFilterFactory(new HttpFiltersSourceAdapter() {
             @Override
             public HttpFilters filterRequest(HttpRequest originalRequest, ChannelHandlerContext ctx) {

browserup-proxy-core/src/main/java/com/browserup/bup/filters/BlacklistFilter.java

@@ -6,13 +6,14 @@
 
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.handler.codec.http.DefaultFullHttpResponse;
-import io.netty.handler.codec.http.HttpHeaders;
 import io.netty.handler.codec.http.HttpMethod;
 import io.netty.handler.codec.http.HttpObject;
 import io.netty.handler.codec.http.HttpRequest;
 import io.netty.handler.codec.http.HttpResponse;
 import io.netty.handler.codec.http.HttpResponseStatus;
 import com.browserup.bup.proxy.BlacklistEntry;
+import com.browserup.bup.util.HttpStatusClass;
+import io.netty.handler.codec.http.HttpUtil;
 
 import java.util.Collection;
 import java.util.Collections;
@@ -22,6 +23,7 @@
  * that the blacklist at the time of construction will contain the same values when the filter is actually invoked, if the entries are modified concurrently.
  */
 public class BlacklistFilter extends HttpsAwareFiltersAdapter {
+    public static final String BLOCKED_PHRASE = "Request blocked";
     private final Collection<BlacklistEntry> blacklistedUrls;
 
     public BlacklistFilter(HttpRequest originalRequest, ChannelHandlerContext ctx, Collection<BlacklistEntry> blacklistedUrls) {
@@ -39,18 +41,23 @@ public HttpResponse clientToProxyRequest(HttpObject httpObject) {
         if (httpObject instanceof HttpRequest) {
             HttpRequest httpRequest = (HttpRequest) httpObject;
 
-            String url = getFullUrl(httpRequest);
+            String url = getOriginalUrl();
 
             for (BlacklistEntry entry : blacklistedUrls) {
-                if (HttpMethod.CONNECT.equals(httpRequest.getMethod()) && entry.getHttpMethodPattern() == null) {
+                if (HttpMethod.CONNECT.equals(httpRequest.method()) && entry.getHttpMethodPattern() == null) {
                     // do not allow CONNECTs to be blacklisted unless a method pattern is explicitly specified
                     continue;
                 }
 
-                if (entry.matches(url, httpRequest.getMethod().name())) {
-                    HttpResponseStatus status = HttpResponseStatus.valueOf(entry.getStatusCode());
-                    HttpResponse resp = new DefaultFullHttpResponse(httpRequest.getProtocolVersion(), status);
-                    HttpHeaders.setContentLength(resp, 0L);
+                if (entry.matches(url, httpRequest.method().name())) {
+                    HttpResponseStatus status;
+                    if(HttpStatusClass.UNKNOWN.equals(HttpStatusClass.valueOf(entry.getStatusCode()))) {
+                        status = new HttpResponseStatus(entry.getStatusCode(), BLOCKED_PHRASE);
+                    } else {
+                        status = HttpResponseStatus.valueOf(entry.getStatusCode());
+                    }
+                    HttpResponse resp = new DefaultFullHttpResponse(httpRequest.protocolVersion(), status);
+                    HttpUtil.setContentLength(resp, 0L);
 
                     return resp;
                 }

browserup-proxy-core/src/main/java/com/browserup/bup/filters/HarCaptureFilter.java

@@ -47,7 +47,6 @@
 import java.util.EnumSet;
 import java.util.List;
 import java.util.Set;
-import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import static com.browserup.bup.util.BrowserUpProxyUtil.getTotalElapsedTime;
@@ -120,12 +119,14 @@ public class HarCaptureFilter extends HttpsAwareFiltersAdapter {
      */
     private volatile HttpRequest capturedOriginalRequest;
 
+    private volatile boolean isResponse = false;
+
     /**
      * True if this filter instance processed a {@link #proxyToServerResolutionSucceeded(String, java.net.InetSocketAddress)} call, indicating
      * that the hostname was resolved and populated in the HAR (if this is not a CONNECT).
      */
     private volatile boolean addressResolved = false;
-
+    
     /**
      * Create a new instance of the HarCaptureFilter that will capture request and response information. If no har is specified in the
      * constructor, this filter will do nothing.
@@ -255,6 +256,7 @@ public HttpResponse clientToProxyRequest(HttpObject httpObject) {
 
     @Override
     public HttpObject serverToProxyResponse(HttpObject httpObject) {
+        isResponse = true;
         // if a ServerResponseCaptureFilter is configured, delegate to it to collect the server's response. if it is not
         // configured, we still need to capture basic information (timings, HTTP status, etc.), just not content.
         if (responseCaptureFilter != null) {
@@ -286,8 +288,20 @@ public HttpObject serverToProxyResponse(HttpObject httpObject) {
         return super.serverToProxyResponse(httpObject);
     }
 
+    @Override
+    public HttpObject proxyToClientResponse(HttpObject httpObject) {
+        // if a subsequent filter short-circuited the response, capture it here
+        if (!isResponse && httpObject instanceof HttpResponse) {
+            HttpResponse httpResponse = (HttpResponse) httpObject;
+            captureResponse(httpResponse);
+            harEntry.setTime(getTotalElapsedTime(harEntry.getTimings()));
+        }
+        return super.proxyToClientResponse(httpObject); 
+    }
+
     @Override
     public void serverToProxyResponseTimedOut() {
+        isResponse = true;
         // replace any existing HarResponse that was created if the server sent a partial response
         HarResponse response = HarCaptureUtil.createHarResponseForFailure();
         harEntry.setResponse(response);
@@ -678,6 +692,7 @@ public InetSocketAddress proxyToServerResolutionStarted(String resolvingServerHo
 
     @Override
     public void proxyToServerResolutionFailed(String hostAndPort) {
+        isResponse = true;
         HarResponse response = HarCaptureUtil.createHarResponseForFailure();
         this.harEntry.setResponse(response);
 
@@ -720,6 +735,7 @@ public void proxyToServerConnectionStarted() {
 
     @Override
     public void proxyToServerConnectionFailed() {
+        isResponse = true;
         HarResponse response = HarCaptureUtil.createHarResponseForFailure();
         this.harEntry.setResponse(response);
 
@@ -733,6 +749,7 @@ public void proxyToServerConnectionFailed() {
 
     @Override
     public void proxyToServerConnectionSucceeded(ChannelHandlerContext serverCtx) {
+        isResponse = true;
         long connectionSucceededTimeNanos = System.nanoTime();
 
         // make sure the previous timestamp was captured, to avoid setting an absurd value in the har (see serverToProxyResponseReceiving())

browserup-proxy-core/src/main/java/com/browserup/bup/filters/WhitelistFilter.java

@@ -4,9 +4,9 @@
 
 package com.browserup.bup.filters;
 
+import com.browserup.bup.util.HttpStatusClass;
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.handler.codec.http.DefaultFullHttpResponse;
-import io.netty.handler.codec.http.HttpHeaders;
 import io.netty.handler.codec.http.HttpObject;
 import io.netty.handler.codec.http.HttpRequest;
 import io.netty.handler.codec.http.HttpResponse;
@@ -15,7 +15,6 @@
 import org.littleshoot.proxy.impl.ProxyUtils;
 
 import java.util.Collection;
-import java.util.Collections;
 import java.util.regex.Pattern;
 
 import static java.util.Collections.*;
@@ -58,12 +57,17 @@ public HttpResponse clientToProxyRequest(HttpObject httpObject) {
 
             boolean urlWhitelisted;
 
-            String url = getFullUrl(httpRequest);
+            String url = getOriginalUrl();
 
             urlWhitelisted = whitelistUrls.stream().anyMatch(pattern -> pattern.matcher(url).matches());
 
             if (!urlWhitelisted) {
-                HttpResponseStatus status = HttpResponseStatus.valueOf(whitelistResponseCode);
+                HttpResponseStatus status;
+                if(HttpStatusClass.UNKNOWN.equals(HttpStatusClass.valueOf(whitelistResponseCode))) {
+                    status = new HttpResponseStatus(whitelistResponseCode, BlacklistFilter.BLOCKED_PHRASE);
+                } else {
+                    status = HttpResponseStatus.valueOf(whitelistResponseCode);
+                }
                 HttpResponse resp = new DefaultFullHttpResponse(httpRequest.protocolVersion(), status);
                 HttpUtil.setContentLength(resp, 0L);
 

browserup-proxy-core/src/test/groovy/com/browserup/bup/proxy/NewHarTest.groovy

@@ -26,6 +26,7 @@ import org.junit.After
 import org.junit.Test
 import org.mockito.invocation.InvocationOnMock
 import org.mockito.stubbing.Answer
+import static org.hamcrest.Matchers.isEmptyOrNullString
 
 import java.text.SimpleDateFormat
 import java.util.concurrent.TimeUnit
@@ -916,6 +917,74 @@ class NewHarTest extends MockServerTest {
         assertTrue(har.log.entries[0].time > 0)
     }
 
+    @Test
+    void testHttpsBlacklistedUrlInHar() {
+        def stubUrl = "/httpsblacklistedurl"
+        stubFor(get(urlEqualTo(stubUrl))
+                .willReturn(ok())
+        )
+        proxy = new BrowserUpProxyServer()
+        proxy.blacklistRequests(".*", 405)
+        proxy.setTrustAllServers(true)
+        proxy.start()
+
+        proxy.newHar()
+
+        String requestUrl = "https://localhost:${mockServerHttpsPort}/httpsblacklistedurl"
+
+        NewProxyServerTestUtil.getNewHttpClient(proxy.port).withCloseable {
+            CloseableHttpResponse response = it.execute(new HttpGet(requestUrl))
+            assertEquals("Did not receive blacklisted status code in response", 405, response.getStatusLine().getStatusCode())
+
+            String responseBody = NewProxyServerTestUtil.toStringAndClose(response.getEntity().getContent())
+            assertThat("Expected blacklisted response to contain 0-length body", responseBody, isEmptyOrNullString())
+        }
+
+        Thread.sleep(500)
+        Har har = proxy.getHar()
+
+        assertThat("Expected to find entries in the HAR", har.getLog().getEntries(), not(empty()))
+
+        HarResponse harResponse = har.log.entries[0].response
+        assertNotNull("No HAR response found", harResponse)
+        assertEquals("Expected blacklisted status code for the request", 405, harResponse.status)
+        assertEquals("Expected default value for bodySize for response timeout", -1L, harResponse.bodySize)
+    }
+
+    @Test
+    void testHttpsWhitelistedUrlInHar() {
+        def stubUrl = "/httpsblacklistedurl"
+        stubFor(get(urlEqualTo(stubUrl))
+                .willReturn(ok())
+        )
+        proxy = new BrowserUpProxyServer()
+        proxy.whitelistRequests(null, 405)
+        proxy.setTrustAllServers(true)
+        proxy.start()
+
+        proxy.newHar()
+
+        String requestUrl = "https://localhost:${mockServerHttpsPort}/httpsblacklistedurl"
+
+        NewProxyServerTestUtil.getNewHttpClient(proxy.port).withCloseable {
+            CloseableHttpResponse response = it.execute(new HttpGet(requestUrl))
+            assertEquals("Did not receive blacklisted status code in response", 405, response.getStatusLine().getStatusCode())
+
+            String responseBody = NewProxyServerTestUtil.toStringAndClose(response.getEntity().getContent())
+            assertThat("Expected blacklisted response to contain 0-length body", responseBody, isEmptyOrNullString())
+        }
+
+        Thread.sleep(500)
+        Har har = proxy.getHar()
+
+        assertThat("Expected to find entries in the HAR", har.getLog().getEntries(), not(empty()))
+
+        HarResponse harResponse = har.log.entries[0].response
+        assertNotNull("No HAR response found", harResponse)
+        assertEquals("Expected blacklisted status code for the request", 405, harResponse.status)
+        assertEquals("Expected default value for bodySize for response timeout", -1L, harResponse.bodySize)
+    }
+
     @Test
     void testRedirectUrlCapturedForRedirects() {
         def stubUrl = "/test300"