hackney icon indicating copy to clipboard operation
hackney copied to clipboard

Fix 536

Open cirka opened this issue 7 years ago • 5 comments

Fixes connect tunnel proxy issue where SSL over proxy upgrade was unreliable because garbage was left in socket receive buffer.

cirka avatar Oct 02 '18 14:10 cirka

Thanks! This patch doesn't handle empty line which however. The main difference with #537 is using a buffer of 8192 where #537 is using a buffer of 4096, I wonder if it's not the initial issue.

benoitc avatar Oct 03 '18 08:10 benoitc

Well empty line after headers section of response is matched with \r\n\r\n. In case no headers are added you get response like: "HTTP/1.0 200 Connection Established\r\n\r\n" and with additional headers: "HTTP/1.0 200 Connection Established\r\nX-some-header: some_value\r\n\r\n" One "\r\n" sequence terminates the line, and other "\r\n" sequence immediately following the first sequence signifies empty line. That is what I am matching. I am not sure 8K is enough for all use cases, but it look sane limit. Who would want they HTTP response and headers weight more than 8K, right ?

cirka avatar Oct 03 '18 10:10 cirka

that's untrue, you can have empty line "\n" sent at the beginning :) Which is what I thought was the issue. Anyway let me show the spec about proxies but so far you patch is OK. I think reusing the hackney parser would also do the trick and may reuse more code but that cna be done in another iteration.

benoitc avatar Oct 03 '18 11:10 benoitc

I am not sure it is allowed to have empty line at the beginning, but we do not need to worry about this in the scope of this ticket/contribution. I am only interested in gathering full response from the proxy before allowing a socket to be reused. Any '\n' at the beginning wont break this matching code. I was thinking about using gen_tcp:unrecv in case we overshoot with gen_tcp:recv but it is undocumented....and i wont happen to me ;-)

cirka avatar Oct 03 '18 11:10 cirka

I am sorry I am not familiar with hackney code base to reuse existing response parsing code. I just wanted to communicate the problem and to propose working solution. The exact way of solving the problem is not an issue for me, please, feel free to implement your own. I already had my 'high' from understanding what causes the bug ;-)

cirka avatar Oct 03 '18 12:10 cirka