Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Manual Request Editor CONNECT Http Method Broken #6427

Closed
davewichers opened this issue Jan 28, 2021 · 6 comments · Fixed by #6464
Closed

Manual Request Editor CONNECT Http Method Broken #6427

davewichers opened this issue Jan 28, 2021 · 6 comments · Fixed by #6464
Assignees
Milestone

Comments

@davewichers
Copy link

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.(HttpConnection.java:220) ~[zap-D-2021-01-20.jar:?]
at org.apache.commons.httpclient.HttpConnection.(HttpConnection.java:168) ~[zap-D-2021-01-20.jar:?]
at org.apache.commons.httpclient.MultiThreadedHttpConnectionManager$HttpConnectionWithReference.(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.

@kingthorin
Copy link
Member

Ref: https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods/CONNECT

For my two cents CONNECT should stay on the list, however the parsing/adjustment of the target does need to be fixed.

@pranavsaxena17
Copy link
Contributor

I'll like to pick this up. Any pointers as to where to begin?

@kingthorin
Copy link
Member

This package: https://github.com/zaproxy/zaproxy/tree/main/zap/src/main/java/org/parosproxy/paros/extension/manualrequest

Find where the method combobox is, check it's action handling, see what is modified when CONNECT is selected.

@pranavsaxena17
Copy link
Contributor

pranavsaxena17 commented Feb 14, 2021

This issue exists because of the change :
// ZAP: 2019/03/19 Changed the parse method to only parse the authority on CONNECT requests
in https://github.com/zaproxy/zaproxy/blob/main/zap/src/main/java/org/parosproxy/paros/network/HttpRequestHeader.java

Due to this, while parsing, the URI becomes only the protocol of the request.
So how should I fix this? Should I add additional logic in: https://github.com/zaproxy/zaproxy/blob/main/zap/src/main/java/org/zaproxy/zap/extension/httppanel/HttpPanelRequest.java so that instead of calling the method mutateHttpMethod on the request like it currently does for any selected method in the menu, it crafts a new, properly parsed HttpMessage when the selected method is CONNECT ,and sets the original message equal to it. I can think of either this or entirely undoing the change made above(which most probably would break something else somewhere). @kingthorin @thc202

@thc202
Copy link
Member

thc202 commented Feb 19, 2021

The mutateHttpMethod should be changed to set the appropriate URI based on the method.

Worth noting that the CONNECT method is not properly handled when sending the manual request.

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked and limited conversation to collaborators Aug 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.