From a284e400c1a158425652c466ddf63abb4ed89a9c Mon Sep 17 00:00:00 2001 From: lperrey Date: Mon, 15 Aug 2022 02:14:27 +0200 Subject: [PATCH] changed client_body_limit (now in KB) + multiples little adjusts --- default.config | 32 +++++++----------------- memo.txt | 7 ++---- srcs/Client.cpp | 9 ++++--- srcs/config/ConfigParser.hpp | 21 +++++----------- srcs/config/LocationConfig.hpp | 29 +--------------------- srcs/config/ServerConfig.hpp | 11 ++------- srcs/config/extraConfig.cpp | 4 +-- srcs/config/parser.cpp | 45 ++++++++++++---------------------- srcs/config/postProcessing.cpp | 2 +- srcs/main.cpp | 11 ++------- srcs/utils.hpp | 2 ++ srcs/webserv/method_get.cpp | 2 +- 12 files changed, 48 insertions(+), 127 deletions(-) diff --git a/default.config b/default.config index 721ac51..2325bae 100644 --- a/default.config +++ b/default.config @@ -7,28 +7,19 @@ server { listen 0.0.0.0:4040; # client_body_limit asdfa; - client_body_limit 3000000; + client_body_limit 5000; + # Max == 18446744073709551615 / 1024 == 18014398509481984 index index.html; # this is another comment + #index mdr.html; # this is another comment root ./www/; error_page 404 ./www/error_pages/error_404.html; - -# something to do with /upload - - location /upload { - root ./www/test/; - index submit_form.html; -# upload_dir ./www/uploaded/; -# cgi_ext php; - } - - location /uploaded { -# autoindex on; - root ./www/uploaded/; - upload_dir ./www/uploaded/; + location / { + allow_methods GET; + root ./www/; } location /srcs/cgi-bin/ { @@ -46,22 +37,17 @@ server { cgi_ext out php sh; } - location /cgi-bin { - root ./srcs/cgi-bin/; - cgi_ext cpp php sh; - } - location /upload { allow_methods POST; autoindex on; - upload_dir ./www/user_files/; # TODO: append a '/' if there is none ? + upload_dir ./www/user_files/; # root doesn’t matter if used only with POST and no CGI } location /the_dump { - allow_methods GET; + allow_methods GET DELETE; root ./www/user_files; - autoindex on; + #autoindex on; } location /redirect { diff --git a/memo.txt b/memo.txt index 1e57ae9..057462b 100644 --- a/memo.txt +++ b/memo.txt @@ -4,9 +4,6 @@ IN 42 SUBJECT AND/OR PRIORITY : - Need to test normal body parsing - upload files testing and adjustements -- https://nginx.org/en/docs/http/ngx_http_core_module.html#client_max_body_size -Config en Ko (value * 2^10) serait plus commode que en octet -Et 0 valeur special pour desactiver - Ecrire des tests ! - handle redirection (Work, but weird behavior need deeper test) @@ -17,9 +14,9 @@ Et 0 valeur special pour desactiver ----------------------------- Si ce n'est pas deja fait : -- dans config, check erreur si port > 16bits -(peut-être check si ip > 32bits) +peut-être check si ip > 32bits ----------------------------- +- client_body_limit 0 valeur special pour desactiver dans config - gerer le champ "Accept" du client - gerer les ".." dans un URL (verifier que l'on ne sort pas du dossier "root") - do correct handling of special character in url (/rfc2119_files/errata.js.t%C3%A9l%C3%A9chargement -> /rfc2119_files/errata.js.téléchargement) diff --git a/srcs/Client.cpp b/srcs/Client.cpp index bd81115..6b04f61 100644 --- a/srcs/Client.cpp +++ b/srcs/Client.cpp @@ -183,8 +183,11 @@ void Client::_parse_chunked_body(size_t pos) /* endptr_copy = endptr; */ (void)endptr_copy; chunk_size = std::strtoul(&_request.body[pos], &endptr, 16); -/* if (chunk_size == LONG_MAX && errno == ERANGE) - status = 413; */ + if (errno == ERANGE) + { + status = 413; + return ; + } /* if (endptr == endptr_copy) { std::cerr << "parse_request_body(), no conversion possible\n"; @@ -407,7 +410,7 @@ void Client::_parse_port_hostname(std::string host) void Client::_check_request_errors() { - std::cerr << "Content-Length=" << get_rq_headers("Content-Length") << "\n"; + std::cerr << "Content-Length=" << get_rq_headers("Content-Length") << "\n"; std::cerr << "strtoul=" << std::strtoul(get_rq_headers("Content-Length").c_str(), NULL, 10) << "\n"; std::cerr << "client_body_limit=" << assigned_server->client_body_limit << "\n"; /////////////////////// diff --git a/srcs/config/ConfigParser.hpp b/srcs/config/ConfigParser.hpp index cc84993..58f5b11 100644 --- a/srcs/config/ConfigParser.hpp +++ b/srcs/config/ConfigParser.hpp @@ -14,8 +14,6 @@ # include // strtol, stroul # include // cout, cin # include // ifstream -//# include // access() -# include // opendir(), doesn't work... # include // stat(), replaces opendir() don't bother with ERRNO ? # include // sort() in Post @@ -23,21 +21,17 @@ class ConfigParser { public: - // canonical? - - ConfigParser(const char* path); // a string? + ConfigParser(); ~ConfigParser(); + ConfigParser(const std::string &config_file); - // ideally i wouldn't have one cuz it makes no sense, when would i use it? -// ConfigParser & operator=(const ConfigParser& rhs); - + void read_config(const std::string &config_file); std::vector * parse(); // const? -// std::vector parse(); // const? // i thought if it were an instance of this class you could call -// private member functions from anywhere... - void _print_content() const; +// private member functions from anywhere... // QUESTION : Wut ? + void print_content() const; private: @@ -45,25 +39,22 @@ private: // explicit? // what exaclty does explicit do again? - ConfigParser(); // might need a path as arg? + // ConfigParser(); // should this be in private since it always needs a path? ServerConfig _parse_server(size_t *start); LocationConfig _parse_location(size_t *start); - void _set_server_values(ServerConfig *server, const std::string key, std::string value); 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? - /* Post Processing */ void _post_processing(std::vector *servers); bool _find_root_path_location(std::vector locations); // const? diff --git a/srcs/config/LocationConfig.hpp b/srcs/config/LocationConfig.hpp index b6d96b3..2c89222 100644 --- a/srcs/config/LocationConfig.hpp +++ b/srcs/config/LocationConfig.hpp @@ -1,32 +1,14 @@ -/* ************************************************************************** */ -/* */ -/* ::: :::::::: */ -/* LocationConfig.hpp :+: :+: :+: */ -/* +:+ +:+ +:+ */ -/* By: lperrey +#+ +:+ +#+ */ -/* +#+#+#+#+#+ +#+ */ -/* Created: 2022/07/23 16:08:00 by me #+# #+# */ -/* Updated: 2022/08/12 18:12:23 by lperrey ### ########.fr */ -/* */ -/* ************************************************************************** */ #ifndef LOCATIONCONFIG_HPP # define LOCATIONCONFIG_HPP -# include # include # include -# include -# include // stat() # include "utils.hpp" -// again, struct instead? -class LocationConfig +struct LocationConfig { -public: - // canonic stuff? - std::string path; // /path and /path/ are fine // i remove trailing / systematically std::string root; @@ -87,13 +69,4 @@ public: }; - #endif - - - - - - - - diff --git a/srcs/config/ServerConfig.hpp b/srcs/config/ServerConfig.hpp index 693f0df..8501f6a 100644 --- a/srcs/config/ServerConfig.hpp +++ b/srcs/config/ServerConfig.hpp @@ -10,14 +10,9 @@ # include // string # include // cout, cin -// a class that's all public? just so we have options? -class ServerConfig +struct ServerConfig { -public: - // do i need some canonic stuff? - std::vector server_name; - // we could shove default in here if we wanted to... std::string host; std::string port; @@ -25,9 +20,7 @@ public: std::string root; // ./www/ or www work www/ and www work // i do remove trailing / tho - size_t client_body_limit; // set to default max if none set - // 413 (Request Entity Too Large) if exceeded - // default is 1m 1 000 000 ? + size_t client_body_limit; std::vector index; std::map error_pages; diff --git a/srcs/config/extraConfig.cpp b/srcs/config/extraConfig.cpp index 010bca9..ee958de 100644 --- a/srcs/config/extraConfig.cpp +++ b/srcs/config/extraConfig.cpp @@ -1,6 +1,4 @@ - - #include "ConfigParser.hpp" // should i be sending & references? @@ -63,7 +61,7 @@ std::string ConfigParser::_get_rest_of_line(size_t *curr) } -void ConfigParser::_print_content() const +void ConfigParser::print_content() const { std::cout << _content; } diff --git a/srcs/config/parser.cpp b/srcs/config/parser.cpp index 0159dbb..0c8d0b1 100644 --- a/srcs/config/parser.cpp +++ b/srcs/config/parser.cpp @@ -1,25 +1,26 @@ #include "ConfigParser.hpp" -// Default -ConfigParser::ConfigParser() +ConfigParser::ConfigParser() {}; +ConfigParser::~ConfigParser() {}; + +ConfigParser::ConfigParser(const std::string &config_file) { - std::cout << "Default Constructor\n"; - // don't use yet, you have no idea what the defaults are + read_config(config_file); } -ConfigParser::ConfigParser(const char* path) +void ConfigParser::read_config(const std::string &config_file) { -// std::cout << "Param Constructor\n"; - std::ifstream file; std::string buf; size_t comment; - _content.clear(); - file.open(path); - if (file.is_open()) + file.open(config_file.c_str()); + if (!file) + throw std::invalid_argument("failed to open config"); + else { + _content.clear(); while (!file.eof()) { getline(file, buf); @@ -37,28 +38,9 @@ ConfigParser::ConfigParser(const char* path) _content.append(tmp + '\n'); } } - file.close(); } - else - throw std::invalid_argument("failed to open config"); } -ConfigParser::~ConfigParser() -{ - // do i need to destroy anything, won't it handle itself? -} - -/* -ConfigParser & ConfigParser::operator=(const ConfigParser& rhs) -{ - if (this == rhs) // * & ? - return (*this); // * ? - - // make some stuff equal - return (*this); -} -*/ - // const? std::vector * ConfigParser::parse() { @@ -190,7 +172,7 @@ void ConfigParser::_set_server_values(ServerConfig *server, \ server->server_name.push_back(tmp_val[0]); } else if (key == "listen" && size == 1 && server->host == "" \ - && server->port == "") + && server->port == "") // QUESTION LUKE : C'est quoi cette condition ? Si listen est vide ? Je comprends pas trop. { if (tmp_val[0].find_first_of(":") == NPOS) { @@ -231,6 +213,9 @@ void ConfigParser::_set_server_values(ServerConfig *server, \ if (!::isNumeric(tmp_val[0])) throw std::invalid_argument("client_body_limit not a number"); server->client_body_limit = std::strtoul(tmp_val[0].c_str(), NULL, 10); + if (errno == ERANGE || server->client_body_limit > (ULONG_MAX / KB) ) + throw std::invalid_argument("client_body_limit too big"); + server->client_body_limit = server->client_body_limit * KB; } else if (key == "index") { diff --git a/srcs/config/postProcessing.cpp b/srcs/config/postProcessing.cpp index f4bb1a0..917d595 100644 --- a/srcs/config/postProcessing.cpp +++ b/srcs/config/postProcessing.cpp @@ -22,7 +22,7 @@ void ConfigParser::_post_processing(std::vector *servers) throw std::invalid_argument("Config file needs an Index"); if (it->client_body_limit == 0) - it->client_body_limit = 5000; // what is the recomended size? + it->client_body_limit = 1 * MB; // if error_pages is left empty, we'll use the defaults which diff --git a/srcs/main.cpp b/srcs/main.cpp index 3a44a33..b83bf83 100644 --- a/srcs/main.cpp +++ b/srcs/main.cpp @@ -11,15 +11,8 @@ int main(int ac, char **av) { std::string config = (ac == 2 ? av[1] : "./default.config"); - // like this just looks kinda gross, why bother creating an instance - // and not immediately parsing? like it servers no other purpose... - // what if i call parse directly in the constructor? - // oh because the constructor has no return, but still - // is there a better way? - - ConfigParser configParser(config.c_str()); - - // configParser._print_content(); + ConfigParser configParser(config); + // configParser.print_content(); // i don't love that servers has to be a pointer... std::vector* servers = configParser.parse(); diff --git a/srcs/utils.hpp b/srcs/utils.hpp index ede2808..215f690 100644 --- a/srcs/utils.hpp +++ b/srcs/utils.hpp @@ -22,6 +22,8 @@ # define CRLF CR LF # define CRLF_SIZE 2 # define NPOS std::string::npos +# define KB 1024 +# define MB 1048576 /* Equivalent for end of http header size : ** std::string(CRLF CRLF).size(); diff --git a/srcs/webserv/method_get.cpp b/srcs/webserv/method_get.cpp index 310e27a..d3c1783 100644 --- a/srcs/webserv/method_get.cpp +++ b/srcs/webserv/method_get.cpp @@ -39,7 +39,7 @@ void Webserv::_get(Client *client, std::string &path) _get_file(client, path); } -# define MAX_FILESIZE 1000000 // (1Mo) +# define MAX_FILESIZE 1 * MB void Webserv::_get_file(Client *client, const std::string &path) { /*