Skip to content

Commit

Permalink
Add some more integration tests for forward proxy
Browse files Browse the repository at this point in the history
 - Test the difference in behavior with port
   numbers included in the uri and not
  • Loading branch information
Matt Woodyard committed Aug 22, 2017
1 parent 363df1b commit eea1f7f
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 0 deletions.
11 changes: 11 additions & 0 deletions test/config/integration/server.json
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,17 @@
}
]
},
{
"name": "redirect",

This comment has been minimized.

Copy link
@alyssawilk

alyssawilk Aug 22, 2017

Contributor

Thanks for adding this test!

The proxy I previously worked on didn't allow route matching based on client port. I don't think it's a bad thing to do -
I'd let @mattklein123 make the call if we want to allow or if we should strip out port for matching entirely. My concern is if we have a normal route going to "www.foo.com" it won't match "http://www.foo.com:80" or "https://www.foo.com:443" and I'd consider that a bug, and the use case I'd like to see tested and (if necessary) fixed.

"domains": [ "www.namewithport.com:1234" ],
"require_ssl": "all",
"routes": [
{
"prefix": "/",
"cluster": "cluster_1"
}
]
},
{
"name": "integration",
"domains": [ "*" ],
Expand Down
30 changes: 30 additions & 0 deletions test/integration/integration.cc
Original file line number Diff line number Diff line change
Expand Up @@ -967,6 +967,36 @@ void BaseIntegrationTest::testAbsolutePath() {
EXPECT_FALSE(response.find("HTTP/1.1 404 Not Found\r\n") == 0);
}

void BaseIntegrationTest::testAbsolutePathWithPort() {
Buffer::OwnedImpl buffer("GET http://www.namewithport.com:1234 HTTP/1.1\r\nHost: host\r\n\r\n");
std::string response;
RawConnectionDriver connection(
lookupPort("http_forward"), buffer,
[&](Network::ClientConnection& client, const Buffer::Instance& data) -> void {
response.append(TestUtility::bufferToString(data));
client.close(Network::ConnectionCloseType::NoFlush);
},
version_);

connection.run();
EXPECT_FALSE(response.find("HTTP/1.1 404 Not Found\r\n") == 0);
}

void BaseIntegrationTest::testAbsolutePathWithoutPort() {
Buffer::OwnedImpl buffer("GET http://www.namewithport.com HTTP/1.1\r\nHost: host\r\n\r\n");
std::string response;
RawConnectionDriver connection(
lookupPort("http_forward"), buffer,
[&](Network::ClientConnection& client, const Buffer::Instance& data) -> void {
response.append(TestUtility::bufferToString(data));
client.close(Network::ConnectionCloseType::NoFlush);
},
version_);

connection.run();
EXPECT_TRUE(response.find("HTTP/1.1 404 Not Found\r\n") == 0);
}

void BaseIntegrationTest::testAllowAbsoluteSameRelative() {
// Ensure that relative urls behave the same with allow_absolute_url enabled and without
testEquivalent("GET /foo/bar HTTP/1.1\r\nHost: host\r\n\r\n");
Expand Down
2 changes: 2 additions & 0 deletions test/integration/integration.h
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,8 @@ class BaseIntegrationTest : Logger::Loggable<Logger::Id::testing> {
void testUpstreamProtocolError();
void testBadPath();
void testAbsolutePath();
void testAbsolutePathWithPort();
void testAbsolutePathWithoutPort();
void testConnect();
void testAllowAbsoluteSameRelative();
// Test that a request returns the same content with both allow_absolute_urls enabled and
Expand Down
4 changes: 4 additions & 0 deletions test/integration/integration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,10 @@ TEST_P(IntegrationTest, BadPath) { testBadPath(); }

TEST_P(IntegrationTest, AbsolutePath) { testAbsolutePath(); }

TEST_P(IntegrationTest, AbsolutePathWithPort) { testAbsolutePathWithPort(); }

TEST_P(IntegrationTest, AbsolutePathWithoutPort) { testAbsolutePathWithoutPort(); }

TEST_P(IntegrationTest, Connect) { testConnect(); }

TEST_P(IntegrationTest, ValidZeroLengthContent) {
Expand Down

0 comments on commit eea1f7f

Please sign in to comment.