Skip to content

Commit 8dda4c6

Browse files
iboukhssiboukhss
committed
wip: more fixes
1 parent c9bb68b commit 8dda4c6

File tree

6 files changed

+94
-86
lines changed

6 files changed

+94
-86
lines changed

‎src/config/config_parser.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,9 @@ AstNode ConfigParser::parse_directive()
5151
return parse_statement();
5252
}
5353
parse_error(peek(i));
54+
55+
/* UNREACHABLE */
56+
return AstNode();
5457
}
5558

5659
AstNode ConfigParser::parse_block()

‎src/http/http_response.cpp‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ struct HttpStatusInfo {
2020
};
2121

2222
/* clang-format off */
23-
static const HttpStatusInfo g_status_table[] = {
23+
static const HttpStatusInfo kStatusTable[] = {
2424
{ 200, "OK" },
2525
{ 201, "Created" },
2626
{ 204, "No Content" },
@@ -41,9 +41,9 @@ const char* HttpResponse::reason_phrase(HttpResponse::Status status)
4141
{
4242
int code = static_cast<int>(status);
4343

44-
for (size_t i = 0; i < sizeof(g_status_table) / sizeof(g_status_table[0]); i++) {
45-
if (g_status_table[i].code == code)
46-
return g_status_table[i].reason;
44+
for (size_t i = 0; i < sizeof(kStatusTable) / sizeof(kStatusTable[0]); i++) {
45+
if (kStatusTable[i].code == code)
46+
return kStatusTable[i].reason;
4747
}
4848
return "Unknown";
4949
}
@@ -52,8 +52,8 @@ HttpResponse::Status HttpResponse::parse_status(const std::string& s)
5252
{
5353
int code = std::atoi(s.c_str());
5454

55-
for (size_t i = 0; i < sizeof(g_status_table) / sizeof(g_status_table[0]); i++) {
56-
if (g_status_table[i].code == code)
55+
for (size_t i = 0; i < sizeof(kStatusTable) / sizeof(kStatusTable[0]); i++) {
56+
if (kStatusTable[i].code == code)
5757
return static_cast<HttpResponse::Status>(code);
5858
}
5959
return HttpResponse::kStatusNone;

‎tests/config/router_unittest.conf‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
server {
2+
listen 8080;
3+
root /tmp/www;
4+
5+
location / {
6+
allow_methods GET;
7+
}
8+
9+
location /files {
10+
allow_methods DELETE;
11+
12+
}
13+
14+
location /files/private {
15+
allow_methods POST;
16+
allow_uploads off;
17+
18+
}
19+
20+
location /upload {
21+
allow_methods POST;
22+
allow_uploads on;
23+
upload_store /tmp/store;
24+
}
25+
}

‎tests/router_unittest.cpp‎

Lines changed: 39 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
#include "config/server_config.hpp"
12
#include "handler/delete_handler.hpp"
23
#include "handler/error_handler.hpp"
34
#include "handler/static_file_handler.hpp"
@@ -11,88 +12,61 @@
1112
#include <string>
1213

1314
// This is very brittle and horrible
14-
static ServerConfig make_unittest_config()
15+
static HttpConfig make_unittest_config()
1516
{
16-
SharedConfig dummy("");
17-
std::vector<std::string> methods;
18-
19-
RouteConfig loc1("/", dummy);
20-
methods.push_back("GET");
21-
loc1.shared().set_allowed_methods(methods);
22-
23-
RouteConfig loc2("/files", dummy);
24-
methods.clear();
25-
methods.push_back("DELETE");
26-
loc2.shared().set_allowed_methods(methods);
27-
28-
RouteConfig loc3("/files/private", dummy);
29-
methods.clear();
30-
methods.push_back("POST");
31-
loc3.shared().set_allowed_methods(methods);
32-
loc3.shared().set_uploads_off();
33-
34-
RouteConfig loc4("/upload", dummy);
35-
methods.clear();
36-
methods.push_back("POST");
37-
loc4.shared().set_allowed_methods(methods);
38-
loc4.shared().set_uploads_on();
39-
40-
ServerConfig srv("");
41-
42-
srv.add_location(loc1);
43-
srv.add_location(loc2);
44-
srv.add_location(loc3);
45-
srv.add_location(loc4);
46-
47-
return srv;
17+
return load_http_config("tests/config/router_unittest.conf");
4818
}
4919

5020
UTEST(RouterTest, MatchesDefaultRoute)
5121
{
52-
Router router(make_unittest_config());
22+
HttpConfig conf = make_unittest_config();
23+
Router router(conf.servers[0].locations);
5324

5425
std::string request_path = "/";
55-
std::string result = router.find_best_route(request_path).path();
26+
std::string result = router.find_best_route(request_path).path;
5627

5728
ASSERT_STREQ("/", result.c_str());
5829
}
5930

6031
UTEST(RouterTest, MatchesFilePrefix)
6132
{
62-
Router router(make_unittest_config());
33+
HttpConfig conf = make_unittest_config();
34+
Router router(conf.servers[0].locations);
6335

6436
std::string request_path = "/files/42.txt";
65-
std::string result = router.find_best_route(request_path).path();
37+
std::string result = router.find_best_route(request_path).path;
6638

6739
ASSERT_STREQ("/files", result.c_str());
6840
}
6941

7042
UTEST(RouterTest, NoMatchFallsBackToDefaultRoute)
7143
{
72-
Router router(make_unittest_config());
44+
HttpConfig conf = make_unittest_config();
45+
Router router(conf.servers[0].locations);
7346

7447
std::string request_path = "/unknown/42.txt";
75-
std::string result = router.find_best_route(request_path).path();
48+
std::string result = router.find_best_route(request_path).path;
7649

7750
ASSERT_STREQ("/", result.c_str());
7851
}
7952

8053
UTEST(RouterTest, MatchesDirectory)
8154
{
82-
Router router(make_unittest_config());
55+
HttpConfig conf = make_unittest_config();
56+
Router router(conf.servers[0].locations);
8357

8458
std::string request_path = "/files";
85-
std::string result = router.find_best_route(request_path).path();
59+
std::string result = router.find_best_route(request_path).path;
8660

8761
ASSERT_STREQ("/files", result.c_str());
8862
}
8963

9064
UTEST(RouterTest, MatchesDirectoryWithTrailingSlash)
9165
{
92-
Router router(make_unittest_config());
93-
66+
HttpConfig conf = make_unittest_config();
67+
Router router(conf.servers[0].locations);
9468
std::string request_path = "/files/";
95-
std::string result = router.find_best_route(request_path).path();
69+
std::string result = router.find_best_route(request_path).path;
9670

9771
ASSERT_STREQ("/files", result.c_str());
9872
}
@@ -101,17 +75,19 @@ UTEST(RouterTest, MatchesDirectoryWithMutlipleLeadingSlashes)
10175
{
10276
UTEST_SKIP("TODO: IMPLEMENT THIS FEATURE");
10377

104-
Router router(make_unittest_config());
78+
HttpConfig conf = make_unittest_config();
79+
Router router(conf.servers[0].locations);
10580

10681
std::string request_path = "///files";
107-
std::string result = router.find_best_route(request_path).path();
82+
std::string result = router.find_best_route(request_path).path;
10883

10984
ASSERT_STREQ("/files", result.c_str());
11085
}
11186

11287
UTEST(RouterTest, ReturnsStaticFileHandler)
11388
{
114-
Router router(make_unittest_config());
89+
HttpConfig conf = make_unittest_config();
90+
Router router(conf.servers[0].locations);
11591

11692
HttpRequest req;
11793
req.method = "GET";
@@ -124,7 +100,8 @@ UTEST(RouterTest, ReturnsStaticFileHandler)
124100

125101
UTEST(RouterTest, ReturnsUploadHandler)
126102
{
127-
Router router(make_unittest_config());
103+
HttpConfig conf = make_unittest_config();
104+
Router router(conf.servers[0].locations);
128105

129106
HttpRequest req;
130107
req.method = "POST";
@@ -139,7 +116,8 @@ UTEST(RouterTest, ReturnsUploadHandler)
139116

140117
UTEST(RouterTest, ReturnsDeleteHandler)
141118
{
142-
Router router(make_unittest_config());
119+
HttpConfig conf = make_unittest_config();
120+
Router router(conf.servers[0].locations);
143121

144122
HttpRequest req;
145123
req.method = "DELETE";
@@ -152,7 +130,8 @@ UTEST(RouterTest, ReturnsDeleteHandler)
152130

153131
UTEST(RouterTest, UploadsNotAllowed)
154132
{
155-
Router router(make_unittest_config());
133+
HttpConfig conf = make_unittest_config();
134+
Router router(conf.servers[0].locations);
156135

157136
HttpRequest req;
158137
req.method = "POST";
@@ -167,7 +146,8 @@ UTEST(RouterTest, UploadsNotAllowed)
167146

168147
UTEST(RouterTest, EmptyPostBypassesUploadRestriction)
169148
{
170-
Router router(make_unittest_config());
149+
HttpConfig conf = make_unittest_config();
150+
Router router(conf.servers[0].locations);
171151

172152
HttpRequest req;
173153
req.method = "POST";
@@ -180,7 +160,8 @@ UTEST(RouterTest, EmptyPostBypassesUploadRestriction)
180160
}
181161
UTEST(RouterTest, UnsupportedMethodReturnsErrorHandler)
182162
{
183-
Router router(make_unittest_config());
163+
HttpConfig conf = make_unittest_config();
164+
Router router(conf.servers[0].locations);
184165

185166
HttpRequest req;
186167
req.method = "PATCH";
@@ -191,6 +172,7 @@ UTEST(RouterTest, UnsupportedMethodReturnsErrorHandler)
191172
delete h;
192173
}
193174

175+
/*
194176
UTEST(RouterTest, BuildRequestPath)
195177
{
196178
UTEST_SKIP("TODO: MOVE THIS TO A BETTER LOCATION");
@@ -200,7 +182,8 @@ UTEST(RouterTest, BuildRequestPath)
200182
req.method = "GET";
201183
req.path = "/index.html";
202184
203-
Router router(config);
185+
HttpConfig conf = make_unittest_config();
186+
Router router(conf.servers[0].locations);
204187
205188
Handler* handler = router.handle_request(req);
206189
@@ -219,7 +202,8 @@ UTEST(RouterTest, BuildRequestPath_Long)
219202
req.method = "GET";
220203
req.path = "/example/test_subfolder/test_subfolder/someFile.txt";
221204
222-
Router router(config);
205+
HttpConfig conf = make_unittest_config();
206+
Router router(conf.servers[0].locations);
223207
224208
Handler* handler = router.handle_request(req);
225209
ASSERT_TRUE(handler != NULL);
@@ -231,3 +215,4 @@ UTEST(RouterTest, BuildRequestPath_Long)
231215
232216
delete handler;
233217
}
218+
*/

‎tests/server_config_unittest.cpp‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,24 +8,24 @@
88

99
UTEST(ServerConfigTest, ListenPortOnly)
1010
{
11-
ServerConfig srv("");
11+
ServerConfig srv;
1212
srv.add_listen_addr("8080");
1313

14-
ASSERT_EQ(1u, srv.listen_addrs().size());
14+
ASSERT_EQ(1u, srv.listen_addrs.size());
1515

16-
const sockaddr_in& addr = srv.listen_addrs()[0];
16+
const sockaddr_in& addr = srv.listen_addrs[0];
1717
EXPECT_EQ(8080, ntohs(addr.sin_port));
1818
EXPECT_EQ(INADDR_ANY, ntohl(addr.sin_addr.s_addr));
1919
}
2020

2121
UTEST(ServerConfigTest, ListenIpOnly)
2222
{
23-
ServerConfig srv("");
23+
ServerConfig srv;
2424
srv.add_listen_addr("127.0.0.1");
2525

26-
ASSERT_EQ(1u, srv.listen_addrs().size());
26+
ASSERT_EQ(1u, srv.listen_addrs.size());
2727

28-
const sockaddr_in& addr = srv.listen_addrs()[0];
28+
const sockaddr_in& addr = srv.listen_addrs[0];
2929
EXPECT_EQ(8080, ntohs(addr.sin_port));
3030

3131
in_addr expected;
@@ -35,12 +35,12 @@ UTEST(ServerConfigTest, ListenIpOnly)
3535

3636
UTEST(ServerConfigTest, ListenIpPort)
3737
{
38-
ServerConfig srv("");
38+
ServerConfig srv;
3939
srv.add_listen_addr("127.0.0.1:4242");
4040

41-
ASSERT_EQ(1u, srv.listen_addrs().size());
41+
ASSERT_EQ(1u, srv.listen_addrs.size());
4242

43-
const sockaddr_in& addr = srv.listen_addrs()[0];
43+
const sockaddr_in& addr = srv.listen_addrs[0];
4444
EXPECT_EQ(4242, ntohs(addr.sin_port));
4545

4646
in_addr expected;
@@ -50,24 +50,24 @@ UTEST(ServerConfigTest, ListenIpPort)
5050

5151
UTEST(ServerConfigTest, ListenAnyPort)
5252
{
53-
ServerConfig srv("");
53+
ServerConfig srv;
5454
srv.add_listen_addr(":1234");
5555

56-
ASSERT_EQ(1u, srv.listen_addrs().size());
56+
ASSERT_EQ(1u, srv.listen_addrs.size());
5757

58-
const sockaddr_in& addr = srv.listen_addrs()[0];
58+
const sockaddr_in& addr = srv.listen_addrs[0];
5959
EXPECT_EQ(1234, ntohs(addr.sin_port));
6060
EXPECT_EQ(INADDR_ANY, addr.sin_addr.s_addr);
6161
}
6262

6363
UTEST(ServerConfigTest, ListenDefaultPort)
6464
{
65-
ServerConfig srv("");
65+
ServerConfig srv;
6666
srv.add_listen_addr("1.2.3.4:");
6767

68-
ASSERT_EQ(1u, srv.listen_addrs().size());
68+
ASSERT_EQ(1u, srv.listen_addrs.size());
6969

70-
const sockaddr_in& addr = srv.listen_addrs()[0];
70+
const sockaddr_in& addr = srv.listen_addrs[0];
7171
EXPECT_EQ(8080, ntohs(addr.sin_port));
7272

7373
in_addr expected;
@@ -77,7 +77,7 @@ UTEST(ServerConfigTest, ListenDefaultPort)
7777

7878
UTEST(ServerConfigTest, InvalidListenAddrThrows)
7979
{
80-
ServerConfig srv("");
80+
ServerConfig srv;
8181

8282
EXPECT_EXCEPTION(srv.add_listen_addr(""), std::runtime_error);
8383
EXPECT_EXCEPTION(srv.add_listen_addr("abc"), std::runtime_error);

0 commit comments

Comments
 (0)