Moved most request errors check in Client

+ bugfix of "Bad file descriptor" due to empty operator=() in Client
+ added a condition in _determine_location_COOP_FIX()
+ udpate memo.txt
+ need fix for index/autoindex (location "/list" dont work)

.gitignore

@@ -20,3 +20,5 @@ ubuntu_cgi_tester
 webserv
 !**/webserv/
 *.log
+
+large.jpg

default.config

@@ -15,6 +15,14 @@ server {
 
 	error_page 404 ./www/error_pages/error_404.html;
 
+	location /list {
+		autoindex on;
+	}
+
+	location /redirect {
+		redirect 307 https://fr.wikipedia.org/wiki/Ketchup;
+#		redirect 307 https://www.youtube.com/watch?v=rG6b8gjMEkw;
+	}
 
 	location /test {
 		index index1.html subdex.html;

memo.txt

@@ -1,17 +1,17 @@
-IN 42 SUBJECT, PRIORITY :
+IN 42 SUBJECT AND/OR PRIORITY :
 - CGI
 - chunked request (response not mandatory it seems)
-- index (default file directory)
+- fix need for index and autoindex config
 - Ecrire des tests !
-
-- handle redirection (weird behavior, to fix)
-- upload files with config "upload_repo"
+- "root" need to replace "location->path" part of "client.path"
+	replace up to the last "/" of the "location->path" part
+	(if its a folder this will be in fact the entire path)
+- handle redirection (Work, but weird behavior need deeper test)
+- upload files with config "upload_dir"
 - _determine_location() review (New version to complete and test)
-- bug with verification of "redirect uri" (valid uri are rejected)
+- replace atoi() with a better function to avoid overflow
+	like strtol : https://www32.cplusplus.com/reference/cstdlib/strtol/
 -----------------------------
-- autoindex (done, need test)
---------------
-- replace atoi() with a better function
 - 408 Request Timeout
 - gerer le champ "Accept" du client
 - gerer les ".." dans un URL (verifier que l'on ne sort pas du dossier "root")

srcs/Client.cpp

@@ -37,7 +37,7 @@ Client::~Client() {
 	return;
 }
 
-// WIP not sure fo what is more logic here
+/* // WIP not sure fo what is more logic here
 Client::Client( Client const & src )
 	: status			( src.status ),
 	header_complete		( src.header_complete ),
@@ -54,8 +54,9 @@ Client::Client( Client const & src )
 //	buf = strdup(src.buf); // TODO: this doesn't work
 	return;
 }
+ */
 
-// WIP placeholder because of const values
+/* // WIP placeholder because of const values
 Client &	Client::operator=( Client const & rhs )
 {
 	if ( this != &rhs )
@@ -64,7 +65,7 @@ Client &	Client::operator=( Client const & rhs )
 	}
 	return *this;
 }
-
+ */
 
 /*********************************************
  * PUBLIC MEMBER FUNCTIONS
@@ -74,19 +75,37 @@ Client &	Client::operator=( Client const & rhs )
 // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers
 // https://www.ibm.com/docs/en/cics-ts/5.3?topic=protocol-http-requests
 // https://www.tutorialspoint.com/http/http_requests.htm
-void	Client::parse_request()
+void	Client::parse_request(std::vector<ServerConfig> &servers)
 {
 	std::map<std::string, std::string>	headers;
 	std::string							body;
 
 // DEBUG
-std::cout << "\nREQUEST ____________\n" << raw_request << "\n_____________\n";
+// std::cout << "\nREQUEST ____________\n" << raw_request << "\n_____________\n";
+	clear_request(); // not mandatory
 
 	_parse_request_line();
+	if (status)
+		return;
 	_parse_request_headers();
-	_parse_request_body();
+	assigned_server = ::_determine_process_server(this, servers);
+	assigned_location = ::_determine_location(*assigned_server, _request.abs_path);
+	_check_request_errors();
+	if (status)
+		return;
 	_parse_port_hostname(this->get_rq_headers("Host"));
-	raw_request.clear();
+
+/*	dont clear raw_request, we need it for future reparsing of body
+	see call of parse_request() in _read_request()  */
+	// raw_request.clear(); 
+}
+
+void	Client::parse_request_body()
+{
+	// TODO: check error and adjust status
+	_request.body = ::parse_http_body(raw_request);
+	if (_request.body.size() > assigned_server->client_body_limit)
+		status = 413;
 }
 
 bool	Client::fill_script_path(std::string script)
@@ -216,12 +235,6 @@ void	Client::_parse_request_headers()
 	_request.headers = ::parse_http_headers(raw_request);
 }
 
-void	Client::_parse_request_body()
-{
-	// TODO: check error and adjust status
-	_request.body = ::parse_http_body(raw_request);
-}
-
 void	Client::_parse_port_hostname(std::string host)
 {
 	size_t	pos;
@@ -241,6 +254,41 @@ void	Client::_parse_port_hostname(std::string host)
 	_request.hostname = host.substr(0, pos);
 }
 
+void	Client::_check_request_errors()
+{
+//////////////////////
+// Request line checks
+	if (_request.method == UNKNOWN)
+		status = 501;
+	else if (_request.version.compare(0, sizeof("HTTP/1") - 1, "HTTP/1") != 0)
+		status = 505;
+	else if (!(assigned_location->allow_methods & _request.method))
+	{
+		status = 405;
+		response.append("Allow: ");
+		response.append(::http_methods_to_str(assigned_location->allow_methods));
+		response.append(CRLF);
+	}
+	else if (assigned_location->redirect_status)
+	{ // Weird behavior. Sometimes, the web browser seems to wait for a complete response until timeout.
+		// (for codes 301, 302, 303, 307, and 308)
+		status = assigned_location->redirect_status;
+		response.append("Location: ");
+		response.append(assigned_location->redirect_uri);
+		response.append(CRLF CRLF);
+	}
+
+	if (status)
+		return;
+
+/////////////////
+// Headers checks
+	if (!this->get_rq_headers("Content-Length").empty()
+	&& ::atoi(this->get_rq_headers("Content-Length").c_str()) > (int)assigned_server->client_body_limit)
+		status = 413;
+	
+	return;
+}
 
 /*********************************************
  * OVERLOAD

srcs/Client.hpp

@@ -39,8 +39,8 @@ class Client
 		Client();
 		Client(int afd, listen_socket *lsocket, std::string aport, std::string aip);
 		~Client();
-		Client(Client const &src);
-		Client &operator=(Client const &rhs);
+		// Client(Client const &src);
+		// Client &operator=(Client const &rhs);
 
 		std::string				raw_request;
 		std::string				response;
@@ -70,7 +70,8 @@ class Client
 		std::string		get_rq_script_info() const;
 		std::string		get_rq_headers(const std::string & key) const;
 
-		void	parse_request();
+		void	parse_request(std::vector<ServerConfig> &servers);
+		void	parse_request_body();
 		void	clear();
 		void	clear_request();
 		void	clear_script();
@@ -85,15 +86,19 @@ class Client
 
 		void	_parse_request_line();
 		void	_parse_request_headers();
-		void	_parse_request_body();
 		void	_parse_request_uri( std::string uri );
 		void	_parse_port_hostname(std::string host);
 
+		void	_check_request_errors();
 };
 
 bool	operator==(const Client& lhs, const Client& rhs);
 bool	operator==(const Client& lhs, int fd);
 bool	operator==(int fd, const Client& rhs);
 
+// Temporary Global Scope. Probably move to Client in the future.
+ServerConfig	*_determine_process_server(Client *client, std::vector<ServerConfig> &servers);
+const LocationConfig	*_determine_location(const ServerConfig &server, const std::string &path);
+
 #endif
 

srcs/webserv/Webserv.hpp

@@ -84,8 +84,8 @@ class Webserv
 		void					_insert_status_line(Client *client);
 		void					_error_html_response(Client *client);
 		void					_append_body(Client *client, const std::string &body, const std::string &file_extension = "");
-		ServerConfig			*_determine_process_server(Client *client); // cant be const cause of error_pages.operator[]
-		const LocationConfig	*_determine_location(const ServerConfig &server, const std::string &path) const;
+		// ServerConfig			*_determine_process_server(Client *client); // cant be const cause of error_pages.operator[]
+		// const LocationConfig	*_determine_location(const ServerConfig &server, const std::string &path) const;
 		std::string				_determine_file_extension(const std::string &path) const;
 	// method_get.cpp
 		void	_get(Client *client);

srcs/webserv/close.cpp

@@ -21,12 +21,6 @@ void	Webserv::_close_client(int fd)
 
 void	Webserv::_close_all_clients()
 {
-/*
-**	BUG : (since last Hugo changes ? commit 482a389 ? or redirection problem ?)
-**	sometimes "err close(): Bad file descriptor"
-** 	on fd>0 that should be valid (5, 6, ...)
-** 	I seems to trigger the bug when i request multiples ressources before timeout
-*/
 	while (!_clients.empty())
 	{
 		// _epoll_update(_clients.back().fd, 0, EPOLL_CTL_DEL); // normalement superflu, DEBUG

srcs/webserv/method_get.cpp

@@ -109,16 +109,15 @@ void	Webserv::_get_file(Client *client, const std::string &path)
 	}
 	else
 	{
-		std::streampos	size = ifd.tellg();
-
-		// WIP : Chunk or not chunk (if filesize too big)
+		/* std::streampos	size = ifd.tellg();
+		// WIP Low priority : Chunk or not chunk (if filesize too big)
 		if (size > MAX_FILESIZE)
 		{
 			// Then chunk
 			client->status = 500; // WIP temp
 			std::cerr << "File too large for non chunk body\n";
 			return ;
-		}
+		} */
 
 		ifd.seekg(0, std::ios::beg);
 		buf << ifd.rdbuf();

srcs/webserv/method_post.cpp

@@ -43,9 +43,6 @@ void	Webserv::_post_file(Client *client, const std::string &path)
 	else
 	{
 		// Content-Length useless at this point ?
-		// Maybe usefull in _read_request() for rejecting too big content.
-		// Need to _determine_process_server() as soon as possible,
-		// like in _read_request() for stopping read if body is too big ?
 		ofd << client->get_rq_body();
 		if (!ofd)
 		{

srcs/webserv/request.cpp

@@ -1,8 +1,9 @@
 
 #include "Webserv.hpp"
 
-#define BUFSIZE 8192
-#define MAX_HEADER_SIZE 42000 // arbitrary
+// Arbitrary values
+#define BUFSIZE 8192 // (8Ko)
+#define MAX_HEADER_SIZE 16384 // (16Ko)
 
 enum read_return
 {
@@ -54,28 +55,19 @@ int	Webserv::_read_request(Client *client) // Messy, Need refactoring
 	{
 		if (client->raw_request.find(CRLF CRLF) != std::string::npos)
 		{
-			// std::cerr << "Raw_request :\n|||||||||||||||||||||||||||||\n " << client->raw_request << "\n|||||||||||||||||||||||||||||\n"; // DEBUG
 			client->header_complete = true;
-			client->parse_request(); // TODO : split function to avoid useless parsing ?
-			if (client->status) // WIP, need to change client->parse_request() for status update
+			client->parse_request(_servers);
+			std::cerr << client->get_rq_method_str() << " " << client->get_rq_uri() << " " << client->get_rq_version() << "\n"; // DEBUG
+			if (client->status)
 				return READ_COMPLETE;
-			client->assigned_server = _determine_process_server(client);
-			client->assigned_location = _determine_location(*client->assigned_server, client->get_rq_abs_path());
-			if (client->get_rq_version().compare(0, sizeof("HTTP/1") - 1, "HTTP/1") != 0)
-			{ // TODO : move in Client parsing ?
-				client->status = 505;
+
+			if (client->get_rq_headers("Content-Type").empty() && client->get_rq_headers("Content-Length").empty()) // No body case
 				return READ_COMPLETE;
-			}
-			if (!client->get_rq_headers("Content-Length").empty()
-			&& ::atoi(client->get_rq_headers("Content-Length").c_str()) > (int)client->assigned_server->client_body_limit)
-			{
-				client->status = 413;
-				return READ_COMPLETE;
-			}
 		}
 		else if (client->raw_request.size() > MAX_HEADER_SIZE)
 		{
-			client->status = 400;
+			// 413 or 400 ? 413 seems common among http server, but don't fit perfectly.
+			client->status = 413;
 			return READ_COMPLETE;
 		}
 	}
@@ -89,17 +81,10 @@ int	Webserv::_read_request(Client *client) // Messy, Need refactoring
 		}
 		if ((int)client->read_body_size >= ::atoi(client->get_rq_headers("Content-Length").c_str()))
 		{
-			client->parse_request(); // reparse for the body
+			client->parse_request_body();
 			return READ_COMPLETE;
 		}
 	}
 
-
-	if (client->header_complete && client->get_rq_headers("Content-Type").empty() && client->get_rq_headers("Content-Length").empty() )
-	{
-		return READ_COMPLETE;
-	}
-
 	return READ_IN_PROGRESS;
 }
-

srcs/webserv/response.cpp

@@ -69,22 +69,8 @@ void	Webserv::_append_base_headers(Client *client)
 
 void	Webserv::_construct_response(Client *client)
 {
-	// TODO : Move this in read(), stop read if content too large
-	if (client->get_rq_body().size() > client->assigned_server->client_body_limit)
-	{
-		client->status = 413;
-		return;
-	}
-	if (client->assigned_location->redirect_status)
-	{ // Weird behavior. Sometimes, the web browser seems to wait for a complete response until timeout.
-		// (for codes 301, 302, 303, 307, and 308)
-		client->status = client->assigned_location->redirect_status;
-		client->response.append("Location: ");
-		client->response.append(client->assigned_location->redirect_uri);
-		client->response.append(CRLF);
-		client->response.append(CRLF);
-		return ;
-	}
+	/* Switch between normal behavior or CGI here ?
+	maybe better than in _get(), _post(), ...*/
 	_process_method(client);
 }
 
@@ -92,30 +78,17 @@ void	Webserv::_process_method(Client *client)
 {
 	std::cerr << "assigned_location->path = " << client->assigned_location->path << "\n"; // debug
 	std::cerr << "allow_methods = " << client->assigned_location->allow_methods << "\n"; // debug
-	if (client->get_rq_method() == UNKNOWN)
+
+	switch (client->get_rq_method())
 	{
-		client->status = 501;
-	}
-	else if (client->assigned_location->allow_methods & client->get_rq_method())
-	{
-		switch (client->get_rq_method())
-		{
-			case (GET):
-				_get(client); break;
-			case (POST):
-				_post(client); break;
-			case (DELETE):
-				_delete(client); break;
-			default:
-				break;
-		}
-	}
-	else
-	{
-		client->status = 405;
-		client->response.append("Allow: ");
-		client->response.append(::http_methods_to_str(client->assigned_location->allow_methods));
-		client->response.append(CRLF);
+		case (GET):
+			_get(client); break;
+		case (POST):
+			_post(client); break;
+		case (DELETE):
+			_delete(client); break;
+		default:
+			break;
 	}
 }
 
@@ -165,7 +138,8 @@ void	Webserv::_append_body(Client *client, const std::string &body, const std::s
 	client->response.append(body);
 }
 
-ServerConfig	*Webserv::_determine_process_server(Client *client)
+// Temporary Global Scope. Probably move to Client in the future.
+ServerConfig	*_determine_process_server(Client *client, std::vector<ServerConfig> &servers)
 {
 /*
 	http://nginx.org/en/docs/http/request_processing.html
@@ -174,22 +148,22 @@ ServerConfig	*Webserv::_determine_process_server(Client *client)
 */
 
 	std::string	const &server_name = client->get_rq_headers("Host");
-	std::vector<ServerConfig>::iterator	it = _servers.begin();
-	std::vector<ServerConfig>::iterator	default_server = _servers.end();
+	std::vector<ServerConfig>::iterator	it = servers.begin();
+	std::vector<ServerConfig>::iterator	default_server = servers.end();
 
-	while (it != _servers.end())
+	while (it != servers.end())
 	{
 		if (it->host == client->get_cl_lsocket()->host
 		&&	it->port == client->get_cl_lsocket()->port)
 		{
 			if ( std::find(it->server_name.begin(), it->server_name.end(), server_name) != it->server_name.end() )
 				break;
-			else if (default_server == _servers.end())
+			else if (default_server == servers.end())
 				default_server = it;
 		}
 		++it;
 	}
-	if (it != _servers.end())
+	if (it != servers.end())
 		return (&(*it));
 	else
 		return (&(*default_server));
@@ -221,16 +195,20 @@ else
 	std::vector<LocationConfig>::const_iterator	it = server.locations.begin();
 	while (it != server.locations.end())
 	{
-		if (it->path.size() > path.size()) // Warning : il faut aussi prendre en compte l'éventuel "/" final
+		if (it->path.size() > path.size())
 		{
-			++it;
-			continue;
+			// prendre en compte l'éventuel "/" final si location est un dossier
+			if (it->path.size()-1 > path.size() || it->path[it->path.size()-1] != '/')
+			{
+				++it;
+				continue;
+			}
 		}
 		if (path.compare(0, it->path.size(), it->path) == 0)
 		{
 			if (it->path.size() == path.size())
 				break;
-			else if (path[it->path.size() - 1] == '/')
+			else if (path[it->path.size()-1] == '/')
 				break;
 		}
 		++it;
@@ -241,8 +219,8 @@ else
 		return (&(server.locations.back()));
 }
 
-
-const LocationConfig	*Webserv::_determine_location(const ServerConfig &server, const std::string &path) const
+// Temporary Global Scope. Probably move to Client in the future.
+const LocationConfig	*_determine_location(const ServerConfig &server, const std::string &path)
 {
 	std::cout << "determin location path sent: " << path << '\n';