From 0cb49ffa5ee1f70c32046dbb99336034c367bf19 Mon Sep 17 00:00:00 2001 From: Me Date: Sun, 31 Jul 2022 04:20:04 +0200 Subject: [PATCH] started adding more checks to the parser, some things settled, but mostly i have a lot more questions... --- Makefile | 1 + default.config | 15 +-- srcs/ConfigParser.cpp | 196 ++++++++++--------------------------- srcs/ConfigParser.hpp | 11 ++- srcs/ConfigParserUtils.cpp | 109 +++++++++++++++++++++ srcs/ServerConfig.hpp | 6 +- srcs/utils.cpp | 24 +++++ srcs/utils.hpp | 2 + 8 files changed, 205 insertions(+), 159 deletions(-) create mode 100644 srcs/ConfigParserUtils.cpp diff --git a/Makefile b/Makefile index 912ee6b..066f47c 100644 --- a/Makefile +++ b/Makefile @@ -29,6 +29,7 @@ SRCS = main.cpp \ accept.cpp request.cpp response.cpp \ run_loop.cpp \ ConfigParser.cpp \ + ConfigParserUtils.cpp \ utils.cpp \ OBJS_D = builds diff --git a/default.config b/default.config index 395acfc..136510b 100644 --- a/default.config +++ b/default.config @@ -6,6 +6,7 @@ server { listen 0.0.0.0:4040; + client_body_limit asdfa; index index.html; # this is another comment root ./www/; @@ -13,17 +14,3 @@ server { allow_methods GET; } -server { - -# this is a comment - - server_name our_server; - - listen 0.0.0.0:4047; - - - index index.html; # this is another comment - root ./www/; - - allow_methods GET; -} diff --git a/srcs/ConfigParser.cpp b/srcs/ConfigParser.cpp index d7a24a3..46df807 100644 --- a/srcs/ConfigParser.cpp +++ b/srcs/ConfigParser.cpp @@ -13,16 +13,6 @@ #include "ConfigParser.hpp" -/***** Stuf to rework - - -need to figure out why return std::vector * rather than just simple - not a pointer... - is there a good reason? - - -*/ - @@ -33,8 +23,6 @@ ConfigParser::ConfigParser() // don't use yet, you have no idea what the defaults are } -//ConfigParser::ConfigParser(std::string &path) -//ConfigParser::ConfigParser(char & path) ConfigParser::ConfigParser(const char* path) { std::cout << "Param Constructor\n"; @@ -61,7 +49,6 @@ ConfigParser::ConfigParser(const char* path) } else if (comment > 0 && (buf.find_first_not_of(" \t")) < comment) { -// else if (comment > 0 && (buf.find_first_not_of(" \t")) != std::string::npos) // is there a comment at the end of the line std::string tmp = buf.substr(0, comment - 1); _content.append(tmp + '\n'); @@ -109,15 +96,11 @@ std::vector * ConfigParser::parse() std::string key = _content.substr(start, curr - start); if (key != "server") throw std::invalid_argument("bad config file arguments 1"); -// Server server = parse_server(&curr); -// ret->push_back(server); - // why not this? ret->push_back(_parse_server(&curr)); } return (ret); } -// might need new names for Prev and Curr, not super descriptive... ServerConfig ConfigParser::_parse_server(size_t *start) { ServerConfig ret; @@ -141,17 +124,12 @@ ServerConfig ConfigParser::_parse_server(size_t *start) break ; } else if (key == "location") - { - // this does assume we have locations in Server... - // could change the name but it's so clear... ret.locations.push_back(_parse_location(&curr)); - } else { std::string values = _get_rest_of_line(&curr); // curr now should be \n - // checking for ; in _set_value, check key and value - _set_server_values(&ret, key, values); // handles the throws + _set_server_values(&ret, key, values); } } return (ret); @@ -170,7 +148,6 @@ LocationConfig ConfigParser::_parse_location(size_t *start) curr = _content.find_first_not_of(" \t\n", curr); - if (curr == std::string::npos || _content[curr] != '{') throw std::invalid_argument("bad config file syntax 2"); @@ -191,9 +168,7 @@ LocationConfig ConfigParser::_parse_location(size_t *start) { std::string values = _get_rest_of_line(&curr); // curr now should be \n - // checking for ; in _set_value, check key and value - - _set_location_values(&ret, key, values); //handles the throws + _set_location_values(&ret, key, values); } } return (ret); @@ -201,14 +176,13 @@ LocationConfig ConfigParser::_parse_location(size_t *start) -// ok you need to think through these throws, when will each occur? - void ConfigParser::_set_server_values(ServerConfig *server, \ const std::string key, std::string value) { +/* // check key for ; // check values for ; at end and right number of words depending on key @@ -238,30 +212,39 @@ void ConfigParser::_set_server_values(ServerConfig *server, \ // like call substr in split? //value = value.substr(0, i - 1); value = value.substr(0, i); +*/ + + value = _pre_set_val_check(key, value); std::vector tmp_val = split(value, ' '); size_t size = tmp_val.size(); - // would if if be more optimized? if (size < 1) throw std::invalid_argument("missing value"); else if (key == "server_name" && size == 1) { + // should i be checking if the field is already filled server->server_name = tmp_val[0]; } else if (key == "listen" && size == 1) { + // should i be checking if field already filled? + // are we saying only 1 possible? if (tmp_val[0].find_first_of(":") == std::string::npos) { - // why not store as vector [4] ? + if (!(isNumeric(tmp_val[0]))) + throw std::invalid_argument("value not a number"); server->host = "0.0.0.0"; server->port = tmp_val[0]; } else { - // maybe do this differently? std::vector tmp2 = split(tmp_val[0], ':'); - // i might take issue with this, will see + if (!(isNumeric(tmp2[1]))) + throw std::invalid_argument("value not a number"); + + // not sure if this is what we want, means there's only 1 host per + // server... if (server->host != "" && server->host != tmp2[0]) throw std::invalid_argument("bad listen"); server->host = tmp2[0]; @@ -278,6 +261,9 @@ void ConfigParser::_set_server_values(ServerConfig *server, \ } else if (key == "client_body_limit" && size == 1) { + //std::cout << "made it\n"; + if (!(isNumeric(tmp_val[0]))) + throw std::invalid_argument("value not a number"); server->client_body_limit = atoi(tmp_val[0].c_str()); } else if (key == "recv_timeout" && size == 1) @@ -290,32 +276,33 @@ void ConfigParser::_set_server_values(ServerConfig *server, \ { server->send_timeout.tv_sec = atoi(tmp_val[0].c_str()); } -/* else - { - throw std::invalid_argument("should only have 1 value"); - // yea ok but it could also be something else like too many - // args - - } -*/ else if (key == "index") { - // could run more tests on value content but meh... + // should i be doing an access? for (unsigned long i = 0; i != tmp_val.size(); i++) server->index.push_back(tmp_val[i]); } else if (key == "allow_methods") { - // might do something different here - // like change how methods are stored? + // you need to throw if it's a bad method type + // or should we skip? and see if any others are good? for (unsigned long i = 0; i != tmp_val.size(); i++) - server->allow_methods.push_back(_str_to_method_type(tmp_val[i])); + { + MethodType m = _str_to_method_type(tmp_val[i]); + if (m == 3) + throw std::invalid_argument("not a valid method"); + server->allow_methods.push_back(m); + } } else if (key == "return") { - // could run more checks here too - // like tmp_val.size() must be 2 + if (tmp_val.size() != 2) + throw std::invalid_argument("wrong number of values"); // and tmp_val[0] should be a number and tmp_val[1] a string? + if (!(isNumeric(tmp_val[0]))) + throw std::invalid_argument("value not a number"); + + // something about using access() to see if server->redirect_status = atoi(tmp_val[0].c_str()); server->redirect_uri = tmp_val[1]; } @@ -326,8 +313,12 @@ void ConfigParser::_set_server_values(ServerConfig *server, \ std::string path = tmp_val[tmp_val.size() - 1]; for (unsigned long i = 0; i != tmp_val.size() - 1; i++) { + // what are the bounds for Error codes? + if (!(isNumeric_btw(0, 600, tmp_val[i]))) + throw std::invalid_argument("value not a valid number"); int status_code = atoi(tmp_val[i].c_str()); - // yea IDK i might not want to store this like that... + + // yea cuz here we continue.. why suddenly flexible not throw ? if (server->error_pages.find(status_code) != server->error_pages.end()) continue ; server->error_pages[status_code] = path; @@ -337,36 +328,24 @@ void ConfigParser::_set_server_values(ServerConfig *server, \ { throw std::invalid_argument("wrong number of values"); } + + + +// std::cout << "End set\n"; + + } -// again not sure i want an int ret + + + + + + void ConfigParser::_set_location_values(LocationConfig *location, \ const std::string key, std::string value) { - // check key for ; - // check values for ; at end and right number of words depending on key - - if (key.find_first_of(";") != std::string::npos) - throw std::invalid_argument("bad config file arguments 5"); - - // there shouldn't be any tabs, right? not between values... - if (value.find_first_of("\t") != std::string::npos) - throw std::invalid_argument("bad config file arguments 6"); - - size_t i = value.find_first_of(";"); - // so you can't have no ; - // you can't have just ; - // and you can't have a ; not at the end or several ; - // in theory value_find_last_of should find the only ; - if (i == std::string::npos || (value.find_last_not_of(" \n")) != i \ - || value.compare(";") == 0) - throw std::invalid_argument("bad config file arguments 7"); - - - // we Trim value. - // is this valid? - // could do like above? - value = value.substr(0, i); + value = _pre_set_val_check(key, value); std::vector tmp_val = ::split(value, ' '); size_t size = tmp_val.size(); @@ -381,11 +360,6 @@ void ConfigParser::_set_location_values(LocationConfig *location, \ { location->client_body_limit = atoi(tmp_val[0].c_str()); } -/* else - { - throw std::invalid_argument("should only have 1 argument"); - } -*/ else if (key == "index") { for (unsigned long i = 0; i != tmp_val.size(); i++) @@ -412,70 +386,8 @@ void ConfigParser::_set_location_values(LocationConfig *location, \ } } -// assumes curr is on a space or \t or \n -// get first word? next word? word? -std::string ConfigParser::_get_first_word(size_t *curr) -{ - size_t start; - -// are these checks excessive? - if ((start = _content.find_first_not_of(" \t\n", *curr)) == std::string::npos) - throw std::invalid_argument("bad config file arguments"); - if ((*curr = _content.find_first_of(" \t\n", start)) == std::string::npos) - throw std::invalid_argument("bad config file arguments"); - - std::string key = _content.substr(start, *curr - start); - - return (key); -} - -// also assumes curr is on a space \t or \n -std::string ConfigParser::_get_rest_of_line(size_t *curr) -{ - size_t start; - - if ((start = _content.find_first_not_of(" \t\n", *curr)) == std::string::npos) - throw std::invalid_argument("bad config file arguments"); - -// std::cout << "start + 4 = " << _content.substr(start, 4) << "\n"; -// std::cout << "curr + 4 = " << _content.substr(*curr, 4) << "\n"; - - - if ((*curr = _content.find_first_of("\n", start)) == std::string::npos) - throw std::invalid_argument("bad config file arguments"); - - std::string values = _content.substr(start, *curr - start); - -// std::cout << "curr + 4 = " << _content.substr(*curr, 4) << "\n"; -// std::cout << "rest of Line values: " << values << "\n"; - - return (values); -} - - -MethodType ConfigParser::_str_to_method_type(std::string str) -{ - if (str == "GET") - return GET; - else if (str == "POST") - return POST; - else if (str == "DELETE") - return DELETE; - return INVALID; -} - - - - -void ConfigParser::_print_content() const -{ - std::cout << _content; -} - - - - -// I might need to make my own Exceptions to throw... + + diff --git a/srcs/ConfigParser.hpp b/srcs/ConfigParser.hpp index 81b1cdf..eeaa93d 100644 --- a/srcs/ConfigParser.hpp +++ b/srcs/ConfigParser.hpp @@ -74,6 +74,9 @@ private: void _set_location_values(LocationConfig *location, const std::string key, std::string value); + std::string _pre_set_val_check(const std::string key, \ + const std::string value); + std::string _get_first_word(size_t *curr); // const? std::string _get_rest_of_line(size_t *curr); // const? @@ -87,7 +90,13 @@ private: }; - +// def needs work line a better name an do i even need this? +// should it be in Utils instead? +class MyException : public std::invalid_argument +{ + MyException(const std::string str) + : std::invalid_argument(str) {} +}; #endif diff --git a/srcs/ConfigParserUtils.cpp b/srcs/ConfigParserUtils.cpp new file mode 100644 index 0000000..6947a4c --- /dev/null +++ b/srcs/ConfigParserUtils.cpp @@ -0,0 +1,109 @@ + + + +#include "Webserv.hpp" + + + +std::string ConfigParser::_pre_set_val_check(const std::string key, \ + const std::string value) +{ + + // check key for ; + // check values for ; at end and right number of words depending on key + +// std::cout << "pre check\n"; + if (key.find_first_of(";") != std::string::npos) + throw std::invalid_argument("bad config file arguments 2"); + + // there shouldn't be any tabs, right? not between values... + if (value.find_first_of("\t") != std::string::npos) + { + std::cout << value << "\n"; + throw std::invalid_argument("bad config file arguments 3"); + } + + size_t i = value.find_first_of(";"); + // so you can't have no ; + // you can't have just ; + // and you can't have a ; not at the end or several ; + // in theory value_find_last_of should find the only ; + if (i == std::string::npos || (value.find_last_not_of(" \n")) != i \ + || value.compare(";") == 0) + throw std::invalid_argument("bad config file arguments 4"); + + + // we Trim value. + // is this valid? + // would it be better to shove the result directly in tmp_val? + // like call substr in split? + //value = value.substr(0, i - 1); + return (value.substr(0, i)); +} + + + +// assumes curr is on a space or \t or \n +// get first word? next word? word? +std::string ConfigParser::_get_first_word(size_t *curr) +{ + size_t start; + +// are these checks excessive? + if ((start = _content.find_first_not_of(" \t\n", *curr)) == std::string::npos) + throw std::invalid_argument("bad config file arguments"); + if ((*curr = _content.find_first_of(" \t\n", start)) == std::string::npos) + throw std::invalid_argument("bad config file arguments"); + + std::string key = _content.substr(start, *curr - start); + + return (key); +} + +// also assumes curr is on a space \t or \n +std::string ConfigParser::_get_rest_of_line(size_t *curr) +{ + size_t start; + + if ((start = _content.find_first_not_of(" \t\n", *curr)) == std::string::npos) + throw std::invalid_argument("bad config file arguments"); + +// std::cout << "start + 4 = " << _content.substr(start, 4) << "\n"; +// std::cout << "curr + 4 = " << _content.substr(*curr, 4) << "\n"; + + + if ((*curr = _content.find_first_of("\n", start)) == std::string::npos) + throw std::invalid_argument("bad config file arguments"); + + std::string values = _content.substr(start, *curr - start); + +// std::cout << "rest of Line values: " << values << "\n"; + + return (values); +} + + +MethodType ConfigParser::_str_to_method_type(std::string str) +{ + if (str == "GET") + return GET; + else if (str == "POST") + return POST; + else if (str == "DELETE") + return DELETE; + return INVALID; +} + + + + + + + + +void ConfigParser::_print_content() const +{ + std::cout << _content; +} + +// I might need to make my own Exceptions to throw... diff --git a/srcs/ServerConfig.hpp b/srcs/ServerConfig.hpp index a08d8a1..a020d9d 100644 --- a/srcs/ServerConfig.hpp +++ b/srcs/ServerConfig.hpp @@ -43,7 +43,7 @@ public: struct timeval send_timeout; struct timeval recv_timeout; - int client_body_limit; + int client_body_limit; // set to default max if none set bool autoindex; // not sure what these look like in config file @@ -52,7 +52,9 @@ public: // is this the best way? std::string host; - std::string port; + std::string port; // port needs to be something else... not quite an int + // should a Server be able to handle several? + // do i need a print all for testing? diff --git a/srcs/utils.cpp b/srcs/utils.cpp index b66cd62..6cf4df0 100644 --- a/srcs/utils.cpp +++ b/srcs/utils.cpp @@ -16,4 +16,28 @@ std::vector split(std::string input, char delimiter) return answer; } +bool isNumeric(std::string str) +{ + for (size_t i = 0; i < str.length(); i++) + { + if (std::isdigit(str[i]) == false) + return false; + } + return true; +} + + +bool isNumeric_btw(int low, int high, std::string str) +{ + for (size_t i = 0; i < str.length(); i++) + { + if (std::isdigit(str[i]) == false) + return false; + } + int n = atoi(str.c_str()); + if (n < low || n > high) + return false; + return true; +} + diff --git a/srcs/utils.hpp b/srcs/utils.hpp index 3f3b48c..83ebee8 100644 --- a/srcs/utils.hpp +++ b/srcs/utils.hpp @@ -6,5 +6,7 @@ std::vector split(std::string input, char delimiter); +bool isNumeric(std::string str); +bool isNumeric_btw(int low, int high, std::string str); #endif