u413-284-si / webserv

This project is about setting up a http web server, providing a static website.
MIT License
0 stars 0 forks source link

ResponseStream of ResponseBuilder not cleared of previous response #64

Closed u413-284-si closed 1 day ago

u413-284-si commented 1 week ago

When sending multiple requests to the same server it is witnessed that the received response contains part of the previous response.

This happens when the previous response is longer than the current one. Reason behind this behaviour is that resetStream() in the ResponseBuilder only sets the stream to the beginning. When the new content is shorter than what is already in the stream, only the length of the new content is overwritten and a remainder of the old content stays. This is sent together with new response.

Solution: ResponseStream needs to be cleared before a new response is generated.

gwolf-011235 commented 1 week ago

Are we sure the problem is in ResponseBuilder? As explained in #20 the stream is intentionally not cleared. Since we return .c_str() it should only return upto the first zero terminator.

Did you observe it in connection with CGI? How are you adding the CGI result to the response? Could it be that it doesn't add a zero terminator?

Another place could be in connectionBuildResponse() where the connection buffer gets assigned to the response. The assignment operator should replace the contents of the string with the other string (operator=). Maybe its not actually replacing but copying into? In #59 the connection buffer gets .clear() always, which crosses out this possibility.

Also: I can't reproduce. I tried with

Switching back and forth always resulted in the correct response being sent. So maybe CGI stuff?

gwolf-011235 commented 1 week ago

Stringstream seems to cause problems when appending the response body, maybe because of the insertion operator << and how it interacts with appending a std::string.

When the body is a png file, it doesn't work. However if I just switch the stringstream with a std::string, then it works.

I could look into this, but I think it's easier to just use a std::string and do the conversions (mainly in the response headers) with webutils::toString()

webutils::toString() uses stringstream for conversion. Could be improved by using std::sprintf, if we have performance issues: see https://en.cppreference.com/w/cpp/string/basic_string/to_string for c++11 std::to_string() which works "as if using sprintf". However CPP Core Guidelines really don't like sprintf as it is a va_args function