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.

Sign up for GitHub

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

fix: skip http proxy added header #7622

Merged
merged 4 commits into from
Jun 29, 2023
Merged

fix: skip http proxy added header #7622

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1db90c4
fix: skip http proxy added header
jozefrebjak Jun 26, 2023
9a41f6b
test: add testSendProxied
jozefrebjak Jun 26, 2023
8d023d9
fix: testSendProxied typo
jozefrebjak Jun 26, 2023
a20365b
test: improve testSendProxied
jozefrebjak Jun 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions system/HTTP/CURLRequest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,10 @@ public function send(string $method, string $url)
$output = substr($output, strpos($output, $breakString) + 4);
}

if (strpos($output, 'HTTP/1.1 200 Connection established') === 0) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be "Connection Established".

Is it always "HTTP/1.1" ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kenjis You are right, I'm studying now whole concept behind this and it seems that there should be different headers.

The original tunnelling spec required the proxy to return a 200 Connection Established to the client once it had successfully connected to the server, which is what we're seeing. This is potentially followed by more headers, and then an empty line, before the connection becomes transparent and we get the actual response from the server. So, we get two sets of headers. RFC 2817 relaxes this, and allows any 2xx response to a CONNECT request.

Some very late clarification. When you connect to an SSL/TLS server through a proxy, the proxy establishes a tunnel, using HTTP CONNECT. This is covered in RFC2817 and the expired tunneling spec, and not RFC261.

When making an https request through a proxy, a CONNECT request is of
course made to the proxy before the SSL connection can be made with the
remote server.

Currently, the proxy's response to the CONNECT is written to the libcurl
callback function like the rest of the response, such that it appears
before the remote server's response headers. So, for instance, to my
client code, the response looks like this:

HTTP/1.1 200 Connection established <-- proxy's response 
Proxy-Agent: Fortinet-Proxy/1.0
HTTP/2 200 <-- remote server's response 
date: Mon, 26 Jun 2023 07:44:50 GMT 
etc

some proxies are rather minimalist and only give:

HTTP/1.0 200 <-- proxy's response 
HTTP/1.0 200 OK <-- remote server's response 
Date: Tue, 04 Oct 2005 20:17:22 GMT 
etc

The best method I have thought of for detecting whether the response
contains a header like this (with a proxy's CONNECT response first) is
to set a header callback and see if I get an empty header line in the
middle of the headers. I haven't actually tested this solution yet.

$output = substr($output, strpos($output, $breakString) + 4);
}

// If request and response have Digest
if (isset($this->config['auth'][2]) && $this->config['auth'][2] === 'digest' && strpos($output, 'WWW-Authenticate: Digest') !== false) {
$output = substr($output, strpos($output, $breakString) + 4);
Expand Down
15 changes: 15 additions & 0 deletions tests/system/HTTP/CURLRequestTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -765,6 +765,21 @@ public function testSendContinuedWithManyHeaders()
$this->assertSame(200, $response->getStatusCode());
}

public function testSendProxied()
{
$request = $this->getRequest([
'base_uri' => 'http://www.foo.com/api/v1/',
'delay' => 100,
]);

$output = "HTTP/1.1 200 Connection established
Proxy-Agent: Fortinet-Proxy/1.0\x0d\x0a\x0d\x0aHTTP/1.1 200 OK\x0d\x0a\x0d\x0aHi there";
$request->setOutput($output);

$response = $request->get('answer');
$this->assertSame('Hi there', $response->getBody());
}

/**
* See: https://github.com/codeigniter4/CodeIgniter4/issues/7394
*/
Expand Down