Repository: zaproxy/zaproxy
Issue: Manual Request Editor CONNECT Http Method Broken

I'm not sure what CONNECT is even for, but when you switch a request that works to CONNECT, it eats all of the URL except for the protocol like so:

CONNECT https HTTP/1.1
User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:39.0) Gecko/20100101 Firefox/39.0
Pragma: no-cache
Cache-Control: no-cache
Content-Length: 78
Accept: text/html
Content-Type: application/x-www-form-urlencoded
Host: localhost:8443

And then when you hit SEND, you get this error:
1186267 [Thread-15] ERROR org.parosproxy.paros.extension.manualrequest.http.impl.HttpPanelSender - host parameter is null
java.lang.IllegalArgumentException: host parameter is null
	at org.apache.commons.httpclient.HttpConnection.<init>(HttpConnection.java:220) ~[zap-D-2021-01-20.jar:?]
	at org.apache.commons.httpclient.HttpConnection.<init>(HttpConnection.java:168) ~[zap-D-2021-01-20.jar:?]
	at org.apache.commons.httpclient.MultiThreadedHttpConnectionManager$HttpConnectionWithReference.<init>(MultiThreadedHttpConnectionManager.java:1145) ~[commons-httpclient-3.1.jar:?]
	at org.apache.commons.httpclient.MultiThreadedHttpConnectionManager$ConnectionPool.createConnection(MultiThreadedHttpConnectionManager.java:762) ~[commons-httpclient-3.1.jar:?]
	at org.apache.commons.httpclient.MultiThreadedHttpConnectionManager.doGetConnection(MultiThreadedHttpConnectionManager.java:476) ~[commons-httpclient-3.1.jar:?]
	at org.apache.commons.httpclient.MultiThreadedHttpConnectionManager.getConnectionWithTimeout(MultiThreadedHttpConnectionManager.java:416) ~[commons-httpclient-3.1.jar:?]
	at org.apache.commons.httpclient.HttpMethodDirector.executeMethod(HttpMethodDirector.java:189) ~[zap-D-2021-01-20.jar:?]
	at org.apache.commons.httpclient.HttpClient.executeMethod(HttpClient.java:397) ~[commons-httpclient-3.1.jar:?]
	at org.parosproxy.paros.network.HttpSender.executeMethod(HttpSender.java:429) ~[zap-D-2021-01-20.jar:D-2021-01-20]
	at org.parosproxy.paros.network.HttpSender.runMethod(HttpSender.java:671) ~[zap-D-2021-01-20.jar:D-2021-01-20]
	at org.parosproxy.paros.network.HttpSender.send(HttpSender.java:626) ~[zap-D-2021-01-20.jar:D-2021-01-20]
	at org.parosproxy.paros.network.HttpSender.sendAuthenticated(HttpSender.java:601) ~[zap-D-2021-01-20.jar:D-2021-01-20]
	at org.parosproxy.paros.network.HttpSender.sendAndReceiveImpl(HttpSender.java:1033) ~[zap-D-2021-01-20.jar:D-2021-01-20]
	at org.parosproxy.paros.network.HttpSender.sendAndReceive(HttpSender.java:993) ~[zap-D-2021-01-20.jar:D-2021-01-20]
	at org.parosproxy.paros.extension.manualrequest.http.impl.HttpPanelSender.handleSendMessage(HttpPanelSender.java:120) [zap-D-2021-01-20.jar:D-2021-01-20]
	at org.parosproxy.paros.extension.manualrequest.ManualRequestEditorDialog$2.run(ManualRequestEditorDialog.java:239) [zap-D-2021-01-20.jar:D-2021-01-20]
	at java.lang.Thread.run(Thread.java:834) [?:?]

(And I also notice the Response pane doesn't change when there was an error.) Seems like if you hit SEND, it should first clear out the response pane, then try to SEND, and if successful, show the response, and if not, maybe display some kind of error message?

And if you 'accidentally' select CONNECT, and then switch to another HTTP Method, the rest of the URL is still lost, so the manual request is now broken until you close it, find the original request in the history, and reopen it from that request, which is annoying.  (Maybe you should have a 'restore original request' button? Although I realize we are somewhat out of room for new buttons.)

So, please fix CONNECT, or drop it if it's not a useful HTTP method to test.


Changed files:
zap/src/main/java/org/parosproxy/paros/network/HttpMessage.java, zap/src/test/java/org/parosproxy/paros/network/HttpMessageUnitTest.java

diff --git a/zap/src/main/java/org/parosproxy/paros/network/HttpMessage.java b/zap/src/main/java/org/parosproxy/paros/network/HttpMessage.java
index 771bec347..0f5e49d60 100644
--- a/zap/src/main/java/org/parosproxy/paros/network/HttpMessage.java
+++ b/zap/src/main/java/org/parosproxy/paros/network/HttpMessage.java
@@ -58,6 +58,7 @@
 // ZAP: 2020/11/26 Use Log4j 2 classes for logging.
 // ZAP: 2020/12/09 Handle content encodings in request/response bodies.
 // ZAP: 2021/04/01 Detect WebSocket upgrade messages having multiple Connection directives
+// ZAP: 2021/05/11 Fixed conversion of Request Method to/from CONNECT
 package org.parosproxy.paros.network;
 
 import java.net.HttpCookie;
@@ -980,15 +981,11 @@ public class HttpMessage implements Message {
     }
 
     public void mutateHttpMethod(String method) {
-        // String header = reqPanel.getTxtHeader().getText();
-        String header = getRequestHeader().toString();
         try {
-            HttpRequestHeader hrh = new HttpRequestHeader(header);
-
-            URI uri = hrh.getURI();
-            // String body = reqPanel.getTxtBody().getText();
+            URI uri = getRequestHeader().getURI();
             String body = getRequestBody().toString();
-            String prevMethod = hrh.getMethod();
+            String prevMethod = getRequestHeader().getMethod();
+
             if (prevMethod.equalsIgnoreCase(method)) {
                 return;
             }
@@ -1016,7 +1013,6 @@ public class HttpMessage implements Message {
                     }
                     uri.setQuery(sb.toString());
                 }
-                hrh.setURI(uri);
                 // Clear the body
                 body = "";
 
@@ -1040,12 +1036,28 @@ public class HttpMessage implements Message {
                     }
                     body = sb.toString();
                     uri.setQuery(null);
-                    hrh.setURI(uri);
                 }
             }
-            hrh.setMethod(method);
-
-            getRequestHeader().setMessage(hrh.toString());
+            if (prevMethod.equalsIgnoreCase(HttpRequestHeader.CONNECT)) {
+                String scheme;
+                if (getRequestHeader().getHostPort() == 443) {
+                    scheme = "https://";
+                } else {
+                    scheme = "http://";
+                }
+                uri = new URI(scheme + uri, true);
+            } else if (method.equals(HttpRequestHeader.CONNECT)) {
+                uri = URI.fromAuthority(uri.getAuthority());
+            }
+            getRequestHeader()
+                    .setMessage(
+                            method
+                                    + " "
+                                    + uri
+                                    + " "
+                                    + getRequestHeader().getVersion()
+                                    + "\\r\\n"
+                                    + getRequestHeader().getHeadersAsString());
             getRequestBody().setBody(body);
         } catch (HttpMalformedHeaderException e) {
             // Ignore?
diff --git a/zap/src/test/java/org/parosproxy/paros/network/HttpMessageUnitTest.java b/zap/src/test/java/org/parosproxy/paros/network/HttpMessageUnitTest.java
index acf998bee..1c7fd5f17 100644
--- a/zap/src/test/java/org/parosproxy/paros/network/HttpMessageUnitTest.java
+++ b/zap/src/test/java/org/parosproxy/paros/network/HttpMessageUnitTest.java
@@ -200,6 +200,62 @@ public class HttpMessageUnitTest {
         assertThat(copy.isResponseFromTargetHost(), is(equalTo(false)));
     }
 
+    @Test
+    public void shouldCorrectlyMutateFromConnectHttpMethodWhenGenericPort() throws Exception {
+        // Given
+        HttpMessage message =
+                new HttpMessage(
+                        new HttpRequestHeader(
+                                "CONNECT "
+                                        + "www.example.com:9000 HTTP/1.1\\r\\nHost: www.example.com:9000"));
+        // When
+        message.mutateHttpMethod(HttpRequestHeader.GET);
+        // Then
+        assertThat(message.getRequestHeader().getMethod(), is(HttpRequestHeader.GET));
+        assertThat(
+                message.getRequestHeader().getURI().toString(), is("http://www.example.com:9000"));
+        assertThat(
+                message.getRequestHeader().getHeader(HttpRequestHeader.HOST),
+                is("www.example.com:9000"));
+    }
+
+    @Test
+    public void shouldCorrectlyMutateFromConnectHttpMethodWhenHttpsPort() throws Exception {
+        // Given
+        HttpMessage message =
+                new HttpMessage(
+                        new HttpRequestHeader(
+                                "CONNECT "
+                                        + "www.example.com:443 HTTP/1.1\\r\\nHost: www.example.com:443"));
+        // When
+        message.mutateHttpMethod(HttpRequestHeader.GET);
+        // Then
+        assertThat(message.getRequestHeader().getMethod(), is(HttpRequestHeader.GET));
+        assertThat(
+                message.getRequestHeader().getURI().toString(), is("https://www.example.com:443"));
+        assertThat(
+                message.getRequestHeader().getHeader(HttpRequestHeader.HOST),
+                is("www.example.com:443"));
+    }
+
+    @Test
+    public void shouldCorrectlyMutateToConnectHttpMethod() throws Exception {
+        // Given
+        HttpMessage message =
+                new HttpMessage(
+                        new HttpRequestHeader(
+                                "GET "
+                                        + "http://www.example.com:9000/path HTTP/1.1\\r\\nHost: www.example.com:9000"));
+        // When
+        message.mutateHttpMethod(HttpRequestHeader.CONNECT);
+        // Then
+        assertThat(message.getRequestHeader().getMethod(), is(HttpRequestHeader.CONNECT));
+        assertThat(message.getRequestHeader().getURI().toString(), is("www.example.com:9000"));
+        assertThat(
+                message.getRequestHeader().getHeader(HttpRequestHeader.HOST),
+                is("www.example.com:9000"));
+    }
+
     @Test
     void shouldNotSetContentEncodingsWhenSettingHttpRequestBody() {
         // Given