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

GetNextRequest issue #39

Closed
mlanca-c opened this issue Apr 5, 2023 · 1 comment
Closed

GetNextRequest issue #39

mlanca-c opened this issue Apr 5, 2023 · 1 comment
Assignees

Comments

@mlanca-c
Copy link
Collaborator

mlanca-c commented Apr 5, 2023

I think we all understood this problem. I have this proposal:

int http_handle(smt::shared_ptr<ServerSocket> sock, int client_fd) {

	try {
    	        // receiving request string
    	        std::string msg = sock->recv(client_fd);

		// checking if anything was received
		if (msg.empty()) { return (0); }

		LOG_D("Received " + msg + " from client");

		// getting first request from string (there can be more than one request, or request can be incomplete)
		std::string req_str = sock->get_next_request(client_fd, msg);

		// if request is incomplete, return 1 --> this makes HTTPServer not close the SocketConnection yet
		if (req_str.empty()) { return (1); }

		// handling all requests
		while (!req_str.empty()) {

			// handling request here
			....

			// checking for more requests
			req_str = sock->get_next_request(client_fd);
		}
	}
	catch(std::exception const& e) {
		LOG_E(e.what());
		return (-1);
	}
	return (0);
}

what this means:

This means ServerSocket and SocketConnection will need a getNextRequest function. And that HTTPRequest will need to be refactored.

This is my proposal for getNextRequest function in SocketConnection

std::string SocketConnection::getNextRequest(std::string req_str) {

    static std::string buff;
    std::string        ret;

    // adding req_str to buff
    if (req_str != "") { buff += req_str; }

    size_t end_headers = buff.find("\r\n\r\n");
    // Request is incomplete
    if (end_headers == std::string::npos) { return (""); }

    // getting request until the end of headers
    ret = buff.substr(0, end_headers + 4);

    // getting Content-Length
    size_t pos;
    if ((pos = ret.find("Content-Length: ")) != std::string::npos) {

        // getting Content-Length
        std::string        l;
        std::istringstream iss(buff.substr(pos + 16));
        iss >> l;

        // converting to int
        int               len;
        std::stringstream ss(l);
        ss >> len;

        // adding body to ret
        if (len) { ret += buff.substr(end_headers + 4, len); }
    }

    buff = buff.substr(ret.size());

    return (ret);
}

This function returns 0 if a request is incomplete. If somehow the req_str has more than one request, by calling it on a loop like we do in the http_handle function we will be able to receive all complete requests.

@mlanca-c mlanca-c mentioned this issue Apr 5, 2023
@mlanca-c mlanca-c self-assigned this Apr 5, 2023
@mlanca-c mlanca-c linked a pull request Apr 6, 2023 that will close this issue
@J0Santos
Copy link
Collaborator

J0Santos commented Apr 6, 2023

For a request to be incomplete, it is not only about the CRLF sequences not showing up, in fact, the sequence might be in the request, but the content not be complete. The sequence appears at the end of the headers section, and in the case of POST requests, at the end of the body section as well. On top of that, in POST request, the sequence can be at the end of the request, but if the Content Length is not the same as the received body length, then the request is also considered incomplete. I think that part is missing, therefore, my suggestion:

    static std::string buff;
    std::string        ret;

    // adding req_str to buff
    if (req_str != "") { buff += req_str; }

    size_t end_headers = buff.find("\r\n\r\n");
    // Request is incomplete
    if (end_headers == std::string::npos) { return (""); }

    // getting request until the end of headers
    ret = buff.substr(0, end_headers + 4);

    // getting Content-Length
    size_t pos;
    if ((pos = ret.find("Content-Length: ")) != std::string::npos) {

        // getting Content-Length
        std::string        l;
        std::istringstream iss(buff.substr(pos + 16));
        iss >> l;

        // converting to int
        int               len;
        std::stringstream ss(l);
        ss >> len;
        
        // getting body of request
        std::string body = buff.substr(end_headers + 4);

        // adding body to ret
        if (len == body.size()) { ret += buff.substr(end_headers + 4, len); }
        else { return (""); }
    }

    buff = buff.substr(ret.size());

    return (ret);
}

Also, I dont agree that an incomplete request should return an empty string. I think an incomplete request should be in the loop for getNextRequest() so it stops being incomplete and becomes complete at some point. In the case above I used the return empty string so it would be consistent with the proposal, but what if we did return pair<std::string, int> where std::string was the request we got and the int was status of 0 - OK, 1 - Incomplete, 2 - Invalid... Something like this

@mlanca-c mlanca-c closed this as not planned Won't fix, can't repro, duplicate, stale Jan 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants