18 response building handle cgi requests

[u413-284-si]

Overview

1. Introduce CGIHandler class
2. Check for CGI in handleCompleteRequestHeader()
3. Register CGI
4. New Connection status: SendToCGI
5. New Connection status: ReceiveFromCGI
6. Signal handling in child
7. Tests

re 1.

• init(): public method, takes clientSocket, serverSocket, HTTPRequest and active location as input to create the environment provided to CGI
• execute(): public method, creates argv and envp (input params for execve) as vectors of char* which are pointing to a vector of strings (avoiding manual allocation of memory), creates input and output pipe to send and receive data to/from CGI, fork to create child process which defers to the CGI by execve-ing with the interpreter as argv[0] and the script to be executed as arg[1]

re 2.

• after the complete request header has been received, parsed and the target location identified, a check is done to determine whether a CGI is requested
• if so, a CGIHandler object is created, initialised and executed
• a boolean isCGI is switched on
• the resulting relevant pipe ends (write end of ingoing pipe, read end of outgoing pipe) and the pid of the child process are extracted and saved in the calling connection
• the pipe ends are registered in the epoll instance to react to pending read/write events
• if a body is expected, the ConnectionStatus will switch to ::ReceiveBody, then after parsing the whole body check if isCGI=true, then switch to ::SendToCGI and then to ::ReceiveFromCGI, implementing CGI POST requests
• if no body is expected and isCGI=true, the ConnectionStatus will switch to ::ReceiveFromCGI implementing CGI GET requests

re 3.

• adds pipe fd to epoll with either EPOLLIN or EPOLLOUT
• adds a new entry to the std::map<int, Connection&> m_cgiConnections, connecting the fd to a reference to its calling Connection
• when epoll detects a new event, it checks whether the event.fd belongs to this map and if so, calls a handleCGIConnection() function which uses a switch to call the appropriate function depending on the ConnectionStatus similar to handleConnection() -> this separation was required to avoid sending fds/sockets to the wrong connection...() function as several sockets and pipe fds can be triggering events at the same time

re 4.

• calls connectionSendToCGI()
• expects a request body to send, if none reports an error and sets the request.httpStatus to StatusInternalServerError and ConnectionStatus to ::BuildResponse, unregisters pipe fds
• if write() encounters an error, it reports an error and sets the request.httpStatus to StatusInternalServerError and ConnectionStatus to ::BuildResponse, unregisters pipe fds
• if the whole body is sent, sets the ConnectionStatus to ::ReceiveFromCGI, unregisters the write end pipe fd and clears the request body
• if only partial body is sent, replaces the request body with what remains to be sent

re 5.

• calls connectionReceiveFromCGI()
• reads data (= new body) from CGI and uses request.body to save the new body
• if read() encounters an error, it reports an error and sets the request.httpStatus to StatusInternalServerError and ConnectionStatus to ::BuildResponse, unregisters pipe fds
• if the whole body is received, sets the ConnectionStatus to ::BuildResponse, unregisters the read end pipe fd and waits for the child to finish (if any error is encountered sets request.httpStatus to StatusInternalServerError)
• if only partial body is received, appends received data to request body

re 6.

• implements simple signal handling in child process
• catches SIGINT, SIGTERM, SIGQUIT and performs default behaviour
• ignores SIGHUP
• to be discussed: necessary to close pipe ends (private members of CGIHandler) when signal is caught? if yes, how to make pipe ends accessible to signalhandler (which is not part of CGIHandler). One could argue that it is not likely for a signal to be caught before all the pipe ends are closed anyways which happens right after the child is born, but not impossible?

re 7.

• unittests for CGIHandler, registerCGIFileDescriptor, connectionSendtoCGI, connectionReceiveFromCGI
• added scripts and sample requests (in subdir: doc) to test CGI
• to switch between .sh and .py scripts, the dummyConfig needs to be adapted (would be solved if multiple CGIs per location is implemented)

closes #18 closes #60

[gwolf-011235]

[re3, last point]

this separation was required to avoid sending fds/sockets to the wrong connection...() function as several sockets and pipe fds can be triggering events at the same time

Could you elaborate on this?

I think firing for a specific connection could be

• clientFd
• pipeToCGIWriteEnd
• pipeFromCGIReadEnd

This means we could just check if the firing fd corresponds to the connection status. If not, just return. E.g. if connection_status == Connection::SendToCGI and fd == connection.clientFd -> return

This would eliminate the second dispatch function with the switch case, which could be integrated into the existing switch.

[u413-284-si]

[re3, last point]

this separation was required to avoid sending fds/sockets to the wrong connection...() function as several sockets and pipe fds can be triggering events at the same time

Could you elaborate on this?

I think firing for a specific connection could be

* clientFd

* pipeToCGIWriteEnd

* pipeFromCGIReadEnd

This means we could just check if the firing fd corresponds to the connection status. If not, just return. E.g. if connection_status == Connection::SendToCGI and fd == connection.clientFd -> return

This would eliminate the second dispatch function with the switch case, which could be integrated into the existing switch.

If I were to do that I would need to cover not only the two cases where a CGI action is asked for, but also all the other cases e.g. Connection::ReceiveBody where maybe a pipe fd comes in, but it should be the clientFd.

Just tried it out, it would look like this:

if ((connection.m_status == Connection::ReceiveFromCGI || connection.m_status == Connection::SendToCGI)
        && clientFd == connection.m_clientFd) {
        return;
    }

if ((connection.m_status == Connection::Idle || connection.m_status == Connection::ReceiveHeader
    || connection.m_status == Connection::ReceiveBody || connection.m_status == Connection::BuildResponse
    || connection.m_status == Connection::SendResponse)
&& clientFd != connection.m_clientFd) {
return;
}

If we were to integrate this before the switch, it would kinda render the switch useless, don't you think? Besides that it's pretty ugly. But yeah, that way we could loose the other handle function (which is honestly also ugly... not rly happy with either solutions, yet lacking an elegant one)

[gwolf-011235]

I meant to add this check in the specific connection.. function.

E.g. connectionReceiveHeader() would have

if (clientFd != connection.clientfd) // if NOT equal then don't work
  return;

And likewise connectionSendToCGI() would have

if (clientFd == connection.clientfd) // if equal then don't work
  return;

This way the switch would route every connection status and each connection.. function makes sure that only correct fds are handled

[u413-284-si]

Ah, that makes more sense. Adapted and tested it. Works. Removed handleCGIConnections()

[gwolf-011235]

Ah, that makes more sense. Adapted and tested it. Works. Removed handleCGIConnections()

After change unittest does not compile because of changed function signature

[gwolf-011235]

Bug: we fork-exec but do not close file descriptors (mainly sockets, but also the epoll instance).

Reproduce: run with valgrind --trace-children=yes --track-fds=yes and execute cgi script.

Solution: "correct" way would be to mark the sockets and epoll with a "close-on-exec-flag". However we can't do it everywhere, because of limited functions allowed:

• epoll: we have to use epoll_create(), flag can only be set with epoll_create1()
• virtual servers: when creating the socket() we could also pass SOCK_CLOEXEC
• clients: we have to use accept(), flag can only be set with accept4()
• cgi: pipe() can receive O_CLOEXEC flag.

Alternative would be to just close by hand:

• pack functionality of Server destructor in a function and run it
• Server destructor should also close all open cgi pipe fds. If packed in a function it should be run after the dup2() calls
• add getter for epoll fd to EpollWrapper (sadly destroys the invariant, but we have to close epoll instance) - maybe with Doxygen that this should only be used for fork-exec - and pass it to CGIHandler::execute()

Note: since the destructor of ConfigFileParser() never gets called, the opened configfile also isn't closed. Should be fixed by #53, since file can be closed after read in as string

(edited)