Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Slombard/web 286 fix test for parallel requests to the cgi #385

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
b3bca46
Merge pull request #380 from dantol29/lmangall/fix-buffer-overflow-ov…
552020 May 20, 2024
8152918
Merge branch 'main' into slombard/web-279-check-and-fix-cgi
552020 May 20, 2024
bc49233
Merge pull request #372 from dantol29/slombard/web-279-check-and-fix-cgi
dantol29 May 20, 2024
abe64e2
tests(parallel cgi): fix the test to send requests in parallel
May 21, 2024
733ee6a
Merge branch 'main' into slombard/web-286-fix-test-for-parallel-reque…
May 21, 2024
b255e79
chore(pipes): WIP fixing CGI stuck in read
May 21, 2024
7b9cd42
fix(Server): remove unused prints
dantol29 May 22, 2024
8de0753
fix(Server): minor fix, WIP CGI
dantol29 May 22, 2024
53d6293
fix(CGI): fix multiple CGI launch
dantol29 May 22, 2024
c05793b
feat(CGI-tester): add exit code
dantol29 May 22, 2024
320faf1
Merge remote-tracking branch 'origin/main' into slombard/web-286-fix-…
dantol29 May 22, 2024
b443e00
fix(uploadHandler): fix parth issues
dantol29 May 22, 2024
fb77a93
chore(GitHub Actions): rename YAML file from demo to webserv
May 22, 2024
5df3a79
fix(CGI): correct behaviuor in case of an error in CGI(permission den…
dantol29 May 22, 2024
ff447f2
feat(CGI): add files with syntax errors and without permissions
dantol29 May 22, 2024
87e63eb
tests(Git Hub Actions): add test for zombie processes and open pipes/…
May 22, 2024
3a6f354
test(parallel cgi): clean up test duration_ts.sh
May 22, 2024
58e8281
fix(CGI): read from pipe in a non-blocking way
dantol29 May 22, 2024
21dddb8
tests(parallel CGI) add test in the yml file
May 22, 2024
1747ba0
test(yaml github actions): fix syntax
May 22, 2024
8067034
fix: small typo
May 22, 2024
4d95e86
fix: another small typo
May 22, 2024
bef0a7c
fix: fix alignment
May 22, 2024
8751a69
test(Git Hub Actions): fix syntax zombies
May 22, 2024
e77981f
test(Git Hub Actions): fix syntax zombies
May 22, 2024
8f64a65
test(Git Hub Actions): dont exit for zombie atm
May 22, 2024
a018b78
test(Git Hub Actions): change logic to spot zombies
May 22, 2024
fb0eaa3
feat(poll): implement client timeout + add tester
dantol29 May 22, 2024
8fb5602
test(Git Hub Actions): remove exit
May 22, 2024
2c45282
test(Git Hub Actions): comment out exit
May 22, 2024
cefb871
docs(HTTP): add 408
dantol29 May 22, 2024
da45279
chore(add): add development config and reduce time or response of end…
May 22, 2024
33ba693
tests(yaml): remove exit completely
May 22, 2024
0430f6d
tests(yaml): fix syntax of last test
May 22, 2024
dbae1a1
tests(yaml): fix last test to return only a warning
May 22, 2024
6cbb399
fix(_CGIPipeFD): init to -1 instead of 0 - cause 0 is a valid fd, and…
552020 May 23, 2024
803fb7f
tests(parallel_cgi): fix tests not failing if test if failing
552020 May 23, 2024
ff058bc
chore(duration_ts.cgi): change duration value so that it doesnt timeout
552020 May 23, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
name: GitHub Actions Demo
name: Webserv Testing Workflow
run-name: ${{ github.actor }} is testing ${{ github.repository }}
on:
push: # Trigger on push events
Expand Down Expand Up @@ -42,6 +42,8 @@ jobs:
python3 -m pip install aiohttp asyncio aiofiles
- name: Start webserv in background
run: ./webserv &
- name: Install needed dependencies
run: sudo apt-get update && sudo apt-get install -y lsof curl
- name: Run Config file tests
if: always()
working-directory: tests/config
Expand Down Expand Up @@ -102,4 +104,55 @@ jobs:
echo "❌ Tests failed. Exiting with status code $exit_code."
exit $exit_code
fi
- name: Make parallel_cgi/duration_ts.sh executable
run: chmod +x tests/parallel_cgi/duration_ts.sh
- name: Run parallel CGIs Test
run: |
./tests/parallel_cgi/duration_ts.sh
exit_code=$?
if [ $exit_code -ne 0 ]; then
echo "❌ Parallel CGI tests failed. Exiting with status code $exit_code."
exit $exit_code
else
echo "✅ Parallel CGI tests passed."
fi
- name: Check for Zombie Processes
run: |
zombies=$(ps aux | awk '$8=="Z" {print $2}')
if [ -n "$zombies" ]; then
echo " ❌ Zombie processes found:"
echo "$zombies"
ps aux | awk '$8=="Z"'
for pid in $zombies; do
parent_pid=$(ps -o ppid= -p $pid)
echo "Sending SIGCHLD to parent process PID $parent_pid"
kill -SIGCHLD $parent_pid || echo "Failed to signal parent PID $parent_pid"
done
else
echo "✅ No zombie processes found."
fi
- name: Check for open listening sockets
run: |
listening_sockets=$(lsof -iTCP -sTCP:LISTEN -nP | grep 'webserv')
if [ -z "$listening_sockets" ]; then
echo "Expected listening sockets are not open."
else
echo "Listening sockets are correctly open:"
echo "$listening_sockets"
fi

- name: Stop webserv
run: pkill -f webserv

- name: Verify no lingering sockets
run: |
sleep 5 # Give some time for sockets to close properly
lingering_sockets=$(lsof -iTCP -sTCP:LISTEN -nP | grep 'webserv' || true)
if [ -n "$lingering_sockets" ]; then
echo "Unexpected lingering sockets found:"
echo "$lingering_sockets"
else
echo "No lingering sockets found."
fi

- run: echo "🍏 This job's status is ${{ job.status }}."
8 changes: 8 additions & 0 deletions conf/development.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
server {
listen 8080;
autoindex on;
cgi_ext .cgi;
server_name www.development_site;
allow_methods GET POST DELETE;
root var/;
}
1 change: 1 addition & 0 deletions conf/webserv_default.conf
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ server {
allow_methods GET POST DELETE;
autoindex on;
root /var/;
upload_path /upload/;
error_page 404 404.html;
cgi_ext .cgi;

Expand Down
2 changes: 1 addition & 1 deletion docs/HTTP_codes.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ _not supported_

## 408 Request Timeout
### How to trigger?
(TODO: @Stefano)
Send a request with incomplete headers

## 409 Conflict
_not supported_
Expand Down
2 changes: 1 addition & 1 deletion include/webserv.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

#define SEND_BUFFER_SIZE 1024 * 100 // 100 KB
#define BUFFER_SIZE 1025
#define CGI_TIMEOUT_MS 300000 // 3 seconds
#define CGI_TIMEOUT_MS 10000
#define CONFIG_FILE_DEFAULT_PATH "./conf/webserv_default.conf"

#define RED "\033[1;31m"
Expand Down
34 changes: 25 additions & 9 deletions src/CGIHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,10 @@ void CGIHandler::handleRequest(HTTPRequest &request, HTTPResponse &response)
env.HTTPRequestToMetaVars(request, env);
if (!executeCGI(env, response))
{
response.setStatusCode(500, "");
response.setStatusCode(500, "Internal Server Error");
// TODO: it should be hardcoded
response.setBody("500 Internal Server Error");
}
std::cout << GREEN << _connection.getCGIPid() << RESET << std::endl;
std::cout << RED << "Exiting CGIHandler::handleRequest" << RESET << std::endl;
return;
}

Expand Down Expand Up @@ -127,6 +126,16 @@ bool CGIHandler::executeCGI(const MetaVariables &env, HTTPResponse &response)
}
else if (pid == 0)
{
// clang-format off
// std::vector<std::pair<int, int> > pipes = _eventManager.getPipeFDs();
// std::cerr << "CGIHandler: pipes: " << pipes.size() << std::endl;
// for (std::vector<std::pair<int, int> >::const_iterator it = pipes.begin(); it != pipes.end(); ++it)
// {
// std::cerr << GREEN << "CLOSING: " << (*it).first << ", " << (*it).second << RESET << std::endl;
// close((*it).first);
// close((*it).second);
// }
// clang-format on
close(pipeFD[0]);
dup2(pipeFD[1], STDOUT_FILENO);
close(pipeFD[1]);
Expand Down Expand Up @@ -158,16 +167,23 @@ bool CGIHandler::executeCGI(const MetaVariables &env, HTTPResponse &response)
response.setIsCGI(true);
response.setCGIpipeFD(pipeFD);

std::cout << "PIPE SAVED: "<< *response.getCGIpipeFD() << std::endl;

close(pipeFD[1]);
EventData data = {1, pid}; // Assuming 1 is the event type for CGI started
std::cout << "CGIHandler: Emitting event indicating a CGI process has started" << std::endl;
EventData data = {1, pid, pipeFD[0], pipeFD[1]}; // Assuming 1 is the event type for CGI started

_eventManager.emit(data); // Emit event indicating a CGI process has started
// conn.addCGI(pid);

_connection.addCGI(pid);
std::cout << GREEN << _connection.getCGIPid() << RESET << std::endl;
// TODO: is this used? To which process to you want to send this signal/ @Leo
// signal(SIGALRM, handleTimeout);
// alarm(4);

// clang-format off
std::vector<std::pair<int, int> > pipes = _eventManager.getPipeFDs();
for (std::vector<std::pair<int, int> >::const_iterator it = pipes.begin(); it != pipes.end(); ++it)
{
std::cout << GREEN << "CGIHandler: pipeFDs: " << (*it).first << RESET << std::endl;
}
// clang-format on
std::cout << RED << "Exiting CGIHandler::executeCGI with true" << RESET << std::endl;
return true;
}
Expand Down
41 changes: 41 additions & 0 deletions src/Connection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ Connection::Connection(struct pollfd &pollFd, Server &server)
_CGIExitStatus = 0;
_CGIHasCompleted = false;
_CGIHasTimedOut = false;
_CGIHasReadPipe = false;
_startTime = 0;
}

Connection::Connection(const Connection &other)
Expand Down Expand Up @@ -56,6 +58,9 @@ Connection::Connection(const Connection &other)
_CGIExitStatus = other._CGIExitStatus;
_CGIHasCompleted = other._CGIHasCompleted;
_CGIHasTimedOut = other._CGIHasTimedOut;
_CGIHasReadPipe = other._CGIHasReadPipe;
_cgiOutputBuffer = other._cgiOutputBuffer;
_startTime = other._startTime;
// std::cout << "Connection object copied" << std::endl;
}

Expand Down Expand Up @@ -87,6 +92,9 @@ Connection &Connection::operator=(const Connection &other)
_CGIExitStatus = other._CGIExitStatus;
_CGIHasCompleted = other._CGIHasCompleted;
_CGIHasTimedOut = other._CGIHasTimedOut;
_CGIHasReadPipe = other._CGIHasReadPipe;
_cgiOutputBuffer = other._cgiOutputBuffer;
_startTime = other._startTime;
}
std::cout << "Connection object assigned" << std::endl;
return *this;
Expand Down Expand Up @@ -219,8 +227,38 @@ bool Connection::getCGIHasCompleted() const
return _CGIHasCompleted;
}

bool Connection::getCGIHasReadPipe() const
{
return _CGIHasReadPipe;
}

std::string Connection::getCGIOutputBuffer() const
{
return _cgiOutputBuffer;
}

time_t Connection::getStartTime() const
{
return _startTime;
}

// SETTERS

void Connection::setStartTime(time_t time)
{
_startTime = time;
}

void Connection::setCGIOutputBuffer(std::string buffer)
{
_cgiOutputBuffer = buffer;
}

void Connection::setCGIHasReadPipe(bool value)
{
_CGIHasReadPipe = value;
}

void Connection::setCGIHasCompleted(bool value)
{
_CGIHasCompleted = value;
Expand Down Expand Up @@ -335,8 +373,11 @@ bool Connection::readHeaders(Parser &parser)
}
else if (bytesRead == 0)
{
// TODO: think about it
std::cout << "Connection closed before headers being completely sent" << std::endl;
return false;
// std::cout << "bytes_read == 0" << std::endl;
// return true;
}
else
{
Expand Down
10 changes: 9 additions & 1 deletion src/Connection.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,16 @@ class Connection
size_t _responseSize;
size_t _responseSizeSent;
std::string _responseString;
time_t _startTime;
bool _hasCGI;
bool _CGIHasExited;
pid_t _CGIPid;
int _CGIExitStatus;
time_t _CGIStartTime;
bool _CGIHasCompleted;
bool _CGIHasTimedOut;
bool _CGIHasReadPipe;
std::string _cgiOutputBuffer;

public:
Connection(struct pollfd &pollFd, Server &server);
Expand All @@ -66,7 +69,6 @@ class Connection
bool readChunkedBody(Parser &parser);
bool readChunkSize(std::string &line);
bool readChunk(size_t chunkSize, std::string &chunkedData, HTTPResponse &response);
bool readBody(Parser &parser, HTTPRequest &req, HTTPResponse &res, Config &config);
bool readBody(Parser &parser, HTTPRequest &req, HTTPResponse &res);

/* Getters */
Expand All @@ -84,6 +86,7 @@ class Connection
unsigned short getServerPort() const;
size_t getResponseSize() const;
size_t getResponseSizeSent() const;
time_t getStartTime() const;

bool getHasReadSocket() const;
bool getHasFinishedReading() const;
Expand All @@ -98,6 +101,8 @@ class Connection
int getCGIExitStatus() const;
bool getCGIHasCompleted() const;
bool getCGIHasTimedOut() const;
bool getCGIHasReadPipe() const;
std::string getCGIOutputBuffer() const;

/* Setters */
void setResponseString(std::string responseString);
Expand All @@ -113,6 +118,7 @@ class Connection
void setCanBeClosed(bool value);
void setHasDataToSend(bool value);
void setHasFinishedSending(bool value);
void setStartTime(time_t startTime);

void setHasCGI(bool value);
void setCGIPid(pid_t pid);
Expand All @@ -121,6 +127,8 @@ class Connection
void setCGIExitStatus(int status);
void setCGIHasCompleted(bool value);
void setCGIHasTimedOut(bool value);
void setCGIHasReadPipe(bool value);
void setCGIOutputBuffer(std::string output);
/* CGI */
void addCGI(pid_t pid);
void removeCGI(int status);
Expand Down
12 changes: 11 additions & 1 deletion src/HTTPResponse.cpp
Original file line number Diff line number Diff line change
@@ -1,14 +1,22 @@
#include "HTTPResponse.hpp"
#include <sstream>

HTTPResponse::HTTPResponse() : _statusCode(0)
HTTPResponse::HTTPResponse()
{
// We initialize the status code to 0 to indicate that it has not been set
_statusCode = 0;
_isCGI = false;
// We should initialize the pipe file descriptors to -1 to indicate that they are not set
// 0 is a valid file descriptor, so we can't use it to indicate that the pipe is not set
_CGIpipeFD[0] = -1;
_CGIpipeFD[1] = -1;
}

HTTPResponse::HTTPResponse(const HTTPResponse &other)
: _statusCode(other._statusCode), _headers(other._headers), _body(other._body), _isCGI(other._isCGI)
{
_CGIpipeFD[0] = other._CGIpipeFD[0];
_CGIpipeFD[1] = other._CGIpipeFD[1];
}
void HTTPResponse::setErrorResponse(int statusCode)
{
Expand Down Expand Up @@ -56,6 +64,8 @@ HTTPResponse &HTTPResponse::operator=(const HTTPResponse &other)
_headers = other._headers;
_body = other._body;
_isCGI = other._isCGI;
_CGIpipeFD[0] = other._CGIpipeFD[0];
_CGIpipeFD[1] = other._CGIpipeFD[1];
}
return *this;
}
Expand Down
4 changes: 0 additions & 4 deletions src/HTTPResponse.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,9 @@
class HTTPResponse
{
public:
// Default constructor
HTTPResponse();
// Copy constructor
HTTPResponse(const HTTPResponse &other);
// Assignment operator
HTTPResponse &operator=(const HTTPResponse &other);
// Destructor
~HTTPResponse();

int getStatusCode() const;
Expand Down
Loading
Loading