http-server icon indicating copy to clipboard operation
http-server copied to clipboard

3rd argument in memcpy.

Open mikeliaohm opened this issue 1 year ago • 3 comments

Hi, thank you for the great work.

I've encountered a seg fault after registering a new uri. The issue seems to related to memcpy of response_string. I think the 3rd argument should be the size of the response string to be copied instead of kMaxBufferSize, or whichever is smaller.

https://github.com/trungams/http-server/blob/1c171367234d245fa9a5452c86354681bbfe383c/src/http_server.cc#L238-L242

this is a potential fix.

  response_string
      = to_string (http_response, http_request.method () != HttpMethod::HEAD);
  const size_t res_len = response_string.length ();
  size_t len_to_copy = (res_len < kMaxBufferSize) 
    ? res_len : kMaxBufferSize;

  memmove (raw_response->buffer, response_string.c_str (), len_to_copy);
  
  raw_response->buffer[response_string.length ()] = '\0';
  raw_response->length = response_string.length ();

I've forked the project to reproduce my error at this repo.

mikeliaohm avatar Mar 04 '23 14:03 mikeliaohm

Hi Mike, thank you for your interest in the repo, and thank you for your help identifying this bug. The previous code was using strncpy to copy the response content, but its behaviour is a bit different from memcpy, so we end up with the issue you described. I'll make a fix, or you can also submit a pull request if you want.

trungams avatar Mar 05 '23 07:03 trungams

hi, thanks for the response. I've opened a PR. I also tried to properly null-terminate raw_response->buffer. See discussion in stackoverflow. Cheers.

mikeliaohm avatar Mar 06 '23 03:03 mikeliaohm

Thanks for your discussion

jmu201621143028 avatar Mar 12 '24 03:03 jmu201621143028