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

Consistently reject proxy requests and handle OPTIONS * requests also when run behind web server #46

Merged
merged 3 commits into from
Sep 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions examples/index.php
Original file line number Diff line number Diff line change
Expand Up @@ -136,4 +136,9 @@
yield null;
});

// OPTIONS *
$app->options('', function () {
return new React\Http\Message\Response(200);
});

$app->run();
2 changes: 1 addition & 1 deletion src/RouteHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public function map(array $methods, string $route, callable $handler, callable .
*/
public function __invoke(ServerRequestInterface $request)
{
if (\strpos($request->getRequestTarget(), '://') !== false || $request->getMethod() === 'CONNECT') {
if ($request->getRequestTarget()[0] !== '/' && $request->getRequestTarget() !== '*') {
return $this->errorHandler->requestProxyUnsupported($request);
}

Expand Down
11 changes: 10 additions & 1 deletion src/SapiHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,17 @@ public function requestFromGlobals(): ServerRequestInterface
}
// @codeCoverageIgnoreEnd

$target = ($_SERVER['REQUEST_URI'] ?? '/');
$url = $target;
if (($target[0] ?? '/') === '/' || $target === '*') {
$url = ($_SERVER['HTTPS'] ?? null === 'on' ? 'https://' : 'http://') . ($host ?? 'localhost') . ($target === '*' ? '' : $target);
}

$body = file_get_contents('php://input');

$request = new ServerRequest(
$_SERVER['REQUEST_METHOD'] ?? 'GET',
($_SERVER['HTTPS'] ?? null === 'on' ? 'https://' : 'http://') . ($host ?? 'localhost') . ($_SERVER['REQUEST_URI'] ?? '/'),
$url,
$headers,
$body,
substr($_SERVER['SERVER_PROTOCOL'] ?? 'http/1.1', 5),
Expand All @@ -55,6 +61,9 @@ public function requestFromGlobals(): ServerRequestInterface
if ($host === null) {
$request = $request->withoutHeader('Host');
}
if (isset($target[0]) && $target[0] !== '/') {
$request = $request->withRequestTarget($target);
}
$request = $request->withParsedBody($_POST);

// Content-Length / Content-Type are special <3
Expand Down
29 changes: 29 additions & 0 deletions tests/AppTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,35 @@ public function testHandleRequestWithMatchingRouteReturnsResponseFromMatchingRou
$this->assertEquals("OK\n", (string) $response->getBody());
}

public function testHandleRequestWithOptionsAsteriskRequestReturnsResponseFromMatchingEmptyRouteHandler()
{
$app = $this->createAppWithoutLogger();

$app->options('', function () {
return new Response(
200,
[
'Content-Type' => 'text/html'
],
"OK\n"
);
});

$request = new ServerRequest('OPTIONS', 'http://localhost');
$request = $request->withRequestTarget('*');

// $response = $app->handleRequest($request);
$ref = new ReflectionMethod($app, 'handleRequest');
$ref->setAccessible(true);
$response = $ref->invoke($app, $request);

/** @var ResponseInterface $response */
$this->assertInstanceOf(ResponseInterface::class, $response);
$this->assertEquals(200, $response->getStatusCode());
$this->assertEquals('text/html', $response->getHeaderLine('Content-Type'));
$this->assertEquals("OK\n", (string) $response->getBody());
}

public function testHandleRequestWithMatchingRouteReturnsPromiseWhichFulfillsWithResponseWhenHandlerReturnsPromiseWhichFulfillsWithResponse()
{
$app = $this->createAppWithoutLogger();
Expand Down
79 changes: 79 additions & 0 deletions tests/RouteHandlerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@
use FrameworkX\MiddlewareHandler;
use FrameworkX\RouteHandler;
use PHPUnit\Framework\TestCase;
use Psr\Http\Message\ResponseInterface;
use React\Http\Message\Response;
use React\Http\Message\ServerRequest;

class RouteHandlerTest extends TestCase
{
Expand Down Expand Up @@ -41,4 +44,80 @@ public function testMapRouteWithMiddlewareAndControllerAddsRouteWithMiddlewareHa

$handler->map(['GET'], '/', $middleware, $controller);
}

public function testHandleRequestWithProxyRequestReturnsResponseWithMessageThatProxyRequestsAreNotAllowed()
{
$request = new ServerRequest('GET', 'http://example.com/');
$request = $request->withRequestTarget('http://example.com/');

$handler = new RouteHandler();
$response = $handler($request);

/** @var ResponseInterface $response */
$this->assertInstanceOf(ResponseInterface::class, $response);
$this->assertEquals(400, $response->getStatusCode());
$this->assertEquals('text/html; charset=utf-8', $response->getHeaderLine('Content-Type'));
$this->assertStringMatchesFormat("<!DOCTYPE html>\n<html>%a</html>\n", (string) $response->getBody());

$this->assertStringContainsString("<title>Error 400: Proxy Requests Not Allowed</title>\n", (string) $response->getBody());
$this->assertStringContainsString("<p>Please check your settings and retry.</p>\n", (string) $response->getBody());
}

public function testHandleRequestWithConnectProxyRequestReturnsResponseWithMessageThatProxyRequestsAreNotAllowed()
{
$request = new ServerRequest('CONNECT', 'example.com:80');
$request = $request->withRequestTarget('example.com:80');

$handler = new RouteHandler();
$response = $handler($request);

/** @var ResponseInterface $response */
$this->assertInstanceOf(ResponseInterface::class, $response);
$this->assertEquals(400, $response->getStatusCode());
$this->assertEquals('text/html; charset=utf-8', $response->getHeaderLine('Content-Type'));
$this->assertStringMatchesFormat("<!DOCTYPE html>\n<html>%a</html>\n", (string) $response->getBody());

$this->assertStringContainsString("<title>Error 400: Proxy Requests Not Allowed</title>\n", (string) $response->getBody());
$this->assertStringContainsString("<p>Please check your settings and retry.</p>\n", (string) $response->getBody());
}

public function testHandleRequestWithGetRequestReturnsResponseFromMatchingHandler()
{
$request = new ServerRequest('GET', 'http://example.com/');
$response = new Response(200, [], '');

$handler = new RouteHandler();
$handler->map(['GET'], '/', function () use ($response) { return $response; });

$ret = $handler($request);

$this->assertSame($response, $ret);
}

public function testHandleRequestWithGetRequestWithHttpUrlInPathReturnsResponseFromMatchingHandler()
{
$request = new ServerRequest('GET', 'http://example.com/http://localhost/');
$response = new Response(200, [], '');

$handler = new RouteHandler();
$handler->map(['GET'], '/http://localhost/', function () use ($response) { return $response; });

$ret = $handler($request);

$this->assertSame($response, $ret);
}

public function testHandleRequestWithOptionsAsteriskRequestReturnsResponseFromMatchingEmptyHandler()
{
$request = new ServerRequest('OPTIONS', 'http://example.com');
$request = $request->withRequestTarget('*');
$response = new Response(200, [], '');

$handler = new RouteHandler();
$handler->map(['OPTIONS'], '', function () use ($response) { return $response; });

$ret = $handler($request);

$this->assertSame($response, $ret);
}
}
61 changes: 60 additions & 1 deletion tests/SapiHandlerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

use FrameworkX\SapiHandler;
use PHPUnit\Framework\TestCase;
use React\Http\Message\ServerRequest;
use React\Http\Message\Response;
use React\Stream\ThroughStream;

Expand Down Expand Up @@ -79,6 +78,66 @@ public function testRequestFromGlobalsWithGetRequestOverHttps()
$this->assertEquals('localhost', $request->getHeaderLine('Host'));
}

/**
* @backupGlobals enabled
*/
public function testRequestFromGlobalsWithOptionsAsterisk()
{
$_SERVER['REQUEST_METHOD'] = 'OPTIONS';
$_SERVER['REQUEST_URI'] = '*';
$_SERVER['SERVER_PROTOCOL'] = 'http/1.1';
$_SERVER['HTTP_HOST'] = 'localhost';

$sapi = new SapiHandler();
$request = $sapi->requestFromGlobals();

$this->assertEquals('OPTIONS', $request->getMethod());
$this->assertEquals('http://localhost', (string) $request->getUri());
$this->assertEquals('*', $request->getRequestTarget());
$this->assertEquals('1.1', $request->getProtocolVersion());
$this->assertEquals('localhost', $request->getHeaderLine('Host'));
}

/**
* @backupGlobals enabled
*/
public function testRequestFromGlobalsWithGetProxy()
{
$_SERVER['REQUEST_METHOD'] = 'GET';
$_SERVER['REQUEST_URI'] = 'http://example.com/';
$_SERVER['SERVER_PROTOCOL'] = 'http/1.1';
$_SERVER['HTTP_HOST'] = 'example.com';

$sapi = new SapiHandler();
$request = $sapi->requestFromGlobals();

$this->assertEquals('GET', $request->getMethod());
$this->assertEquals('http://example.com/', (string) $request->getUri());
$this->assertEquals('http://example.com/', $request->getRequestTarget());
$this->assertEquals('1.1', $request->getProtocolVersion());
$this->assertEquals('example.com', $request->getHeaderLine('Host'));
}

/**
* @backupGlobals enabled
*/
public function testRequestFromGlobalsWithConnectProxy()
{
$_SERVER['REQUEST_METHOD'] = 'CONNECT';
$_SERVER['REQUEST_URI'] = 'example.com:443';
$_SERVER['SERVER_PROTOCOL'] = 'http/1.1';
$_SERVER['HTTP_HOST'] = 'example.com:443';

$sapi = new SapiHandler();
$request = $sapi->requestFromGlobals();

$this->assertEquals('CONNECT', $request->getMethod());
$this->assertEquals('example.com:443', (string) $request->getUri());
$this->assertEquals('example.com:443', $request->getRequestTarget());
$this->assertEquals('1.1', $request->getProtocolVersion());
$this->assertEquals('example.com:443', $request->getHeaderLine('Host'));
}

public function testSendResponseSendsEmptyResponseWithNoHeadersAndEmptyBodyAndAssignsNoContentTypeAndEmptyContentLength()
{
if (headers_sent() || !function_exists('xdebug_get_headers')) {
Expand Down
6 changes: 6 additions & 0 deletions tests/acceptance.sh
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#!/bin/bash

base=${1:-http://localhost:8080}
baseWithPort=$(php -r 'echo parse_url($argv[1],PHP_URL_PORT) ? $argv[1] : $argv[1] . ":80";' "$base")

n=0
match() {
Expand Down Expand Up @@ -35,6 +36,7 @@ out=$(curl -v $base/uri/Wham%21 2>&1); match "HTTP/.* 200" && m
out=$(curl -v $base/uri/AC%2FDC 2>&1); skipif "HTTP/.* 404" && match "HTTP/.* 200" && match "$base/uri/AC%2FDC" # skip Apache (404 unless `AllowEncodedSlashes NoDecode`)
out=$(curl -v $base/uri/bin%00ary 2>&1); skipif "HTTP/.* 40[04]" && match "HTTP/.* 200" && match "$base/uri/bin%00ary" # skip nginx (400) and Apache (404)
out=$(curl -v $base/uri/AC/DC 2>&1); match "HTTP/.* 200" && match "$base/uri/AC/DC"
out=$(curl -v $base/uri/http://example.com:8080/ 2>&1); match "HTTP/.* 200" && match "$base/uri/http://example.com:8080/"
out=$(curl -v $base/uri? 2>&1); match "HTTP/.* 200" && match "$base/uri" # trailing "?" not reported for empty query string
out=$(curl -v $base/uri?query 2>&1); match "HTTP/.* 200" && match "$base/uri?query"
out=$(curl -v $base/uri?q=a 2>&1); match "HTTP/.* 200" && match "$base/uri?q=a"
Expand Down Expand Up @@ -94,6 +96,7 @@ out=$(curl -v $base/method -X PUT 2>&1); match "HTTP/.* 200" && match "PU
out=$(curl -v $base/method -X PATCH 2>&1); match "HTTP/.* 200" && match "PATCH"
out=$(curl -v $base/method -X DELETE 2>&1); match "HTTP/.* 200" && match "DELETE"
out=$(curl -v $base/method -X OPTIONS 2>&1); match "HTTP/.* 200" && match "OPTIONS"
out=$(curl -v $base -X OPTIONS --request-target "*" 2>&1); skipif "Server: nginx" && match "HTTP/.* 200" # skip nginx (400)

out=$(curl -v $base/headers -H 'Accept: text/html' 2>&1); match "HTTP/.* 200" && match "\"Accept\": \"text/html\""
out=$(curl -v $base/headers -d 'name=Alice' 2>&1); match "HTTP/.* 200" && match "\"Content-Type\": \"application/x-www-form-urlencoded\"" && match "\"Content-Length\": \"10\""
Expand All @@ -106,4 +109,7 @@ out=$(curl -v $base/headers -H 'Content-Type;' 2>&1); skipif "Server: Apac
out=$(curl -v $base/headers -H 'DNT: 1' 2>&1); skipif "Server: nginx" && match "HTTP/.* 200" && match "\"DNT\"" && notmatch "\"Dnt\"" # skip nginx which doesn't report original case (DNT->Dnt)
out=$(curl -v $base/headers -H 'V: a' -H 'V: b' 2>&1); skipif "Server: nginx" && skipif -v "Server:" && match "HTTP/.* 200" && match "\"V\": \"a, b\"" # skip nginx (last only) and PHP webserver (first only)

out=$(curl -v --proxy $baseWithPort $base/debug 2>&1); skipif "Server: nginx" && match "HTTP/.* 400" # skip nginx (continues like direct request)
out=$(curl -v --proxy $baseWithPort -p $base/debug 2>&1); skipif "CONNECT aborted" && match "HTTP/.* 400" # skip PHP development server (rejects as "Malformed HTTP request")

echo "OK ($n)"