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

3rd argument in memcpy. #4

Open
mikeliaohm opened this issue Mar 4, 2023 · 3 comments
Open

3rd argument in memcpy. #4

mikeliaohm opened this issue Mar 4, 2023 · 3 comments

Comments

@mikeliaohm
Copy link

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.

response_string =
to_string(http_response, http_request.method() != HttpMethod::HEAD);
memcpy(raw_response->buffer, response_string.c_str(), kMaxBufferSize);
raw_response->length = response_string.length();
}

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.

@trungams
Copy link
Owner

trungams commented Mar 5, 2023

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.

@mikeliaohm
Copy link
Author

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.

@jmu201621143028
Copy link

Thanks for your discussion

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

No branches or pull requests

3 participants