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

MockOsSysCalls::setsockopt has unwarranted ASSERT #19079

Closed
efimki opened this issue Nov 22, 2021 · 7 comments
Closed

MockOsSysCalls::setsockopt has unwarranted ASSERT #19079

efimki opened this issue Nov 22, 2021 · 7 comments
Labels
bug stale stalebot believes this issue/PR has not been touched recently

Comments

@efimki
Copy link
Contributor

efimki commented Nov 22, 2021

Title: MockOsSysCalls::setsockopt has unwarranted ASSERT

Description:
MockOsSysCalls::setsockopt has ASSERT(optlen == sizeof(int)); but setsockopt API allows socket options to have different length. This breaks our internal test that sets 16 byte socket option.

Repro steps:

The following test triggers an ASSERT in debug build:

TEST(MockOsSysCallsTest, TestSetSockopt) {
  Envoy::Api::MockOsSysCalls os_sys_calls;
  char big_sock_opt[11] = "bigsockopt";
  os_sys_calls.setsockopt(1, 2, 3, big_sock_opt, sizeof(big_sock_opt));
}
@efimki efimki added bug triage Issue requires triage labels Nov 22, 2021
@efimki
Copy link
Contributor Author

efimki commented Nov 22, 2021

I'm not a setsockopt expert.
Maybe there should be an assert that optval != nullptr, and/or optlen != 0.

@RyanTheOptimist
Copy link
Contributor

cc: @yanavlasov

@RyanTheOptimist RyanTheOptimist removed the triage Issue requires triage label Nov 22, 2021
@RyanTheOptimist
Copy link
Contributor

I can't quite find an appropriate label for this. @yanavlasov, any suggestions?

@ggreenway
Copy link
Contributor

@efimki feel free to create a PR to fix

@efimki
Copy link
Contributor Author

efimki commented Nov 29, 2021

Created #19126

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Dec 29, 2021
@efimki
Copy link
Contributor Author

efimki commented Dec 29, 2021

Fixed by #19126

@efimki efimki closed this as completed Dec 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug stale stalebot believes this issue/PR has not been touched recently
Projects
None yet
Development

No branches or pull requests

3 participants