Skip to content

Support LOCAL_ADDR and LOCAL_PORT header in client Request #1450

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

Merged
merged 1 commit into from
Dec 6, 2022
Merged

Support LOCAL_ADDR and LOCAL_PORT header in client Request #1450

merged 1 commit into from
Dec 6, 2022

Conversation

ibauersachs
Copy link
Contributor

Having the local address/port is useful if the server is bound to all interfaces, e.g. to serve different content for developers on localhost only.

@yhirose
Copy link
Owner

yhirose commented Dec 5, 2022

@ibauersachs thank you for the reply. Could you add at least one unit test case to verify your modification works? Then, I'll do code review. Thanks!

@ibauersachs
Copy link
Contributor Author

@yhirose Updated to include a simple test. I would have liked to test against e.g. 127.0.0.2 in addition to 127.0.0.1, but that would require a larger setup for the entire ServerTest class or an additional class. Please let me know if it's enough like this or if I should create a distinct test class.

@yhirose
Copy link
Owner

yhirose commented Dec 5, 2022

@ibauersachs thanks for adding the test. That's enough for me. After I approved the change, the build on GitHub Actions failed at FuzzedStream. Could you fix it and update your pull request? Thanks!

@ibauersachs
Copy link
Contributor Author

I'll look at it.
I didn't notice this because the fuzzer source file unfortunately isn't included in the CMake build.

Having the local address/port is useful if the server is bound to
all interfaces, e.g. to serve different content for developers
on localhost only.
@ibauersachs
Copy link
Contributor Author

@yhirose Updated. The workflows passed in my fork.

As a side note, when running the tests locally in WSL2 - which doesn't properly support AF_UNIX - the UnixSocketTest hangs (it waits forever until the server is running, which becomes true). A possible workaround is to assert on a future, e.g. similar to this:

TEST_F(UnixSocketTest, pathname) {
  httplib::Server svr;
  svr.Get(pattern_, [&](const httplib::Request &, httplib::Response &res) {
    res.set_content(content_, "text/plain");
  });

  std::packaged_task<bool()> listen_task([&]{
    return svr.set_address_family(AF_UNIX).listen(pathname_, 80);
  });
  auto future = listen_task.get_future();

  std::thread t(std::move(listen_task));
  t.detach();
  ASSERT_EQ(future.wait_for(std::chrono::milliseconds(1000)), std::future_status::ready);
  ASSERT_TRUE(future.get());
  ASSERT_TRUE(svr.is_running());

  client_GET(pathname_);

  svr.stop();
}

@yhirose yhirose merged commit 8f32271 into yhirose:master Dec 6, 2022
@yhirose
Copy link
Owner

yhirose commented Dec 6, 2022

@ibauersachs thanks for your fine contribution! As for UnixSocketTest.pathname, I tested it on my WSL2 and the test passed successfully. So I am leaving it as it is until it becomes an issue on my machine.

@ibauersachs
Copy link
Contributor Author

Awesome, thank you!

@ibauersachs ibauersachs deleted the local-addr branch December 6, 2022 18:22
ExclusiveOrange pushed a commit to ExclusiveOrange/cpp-httplib-exor that referenced this pull request May 2, 2023
)

Having the local address/port is useful if the server is bound to
all interfaces, e.g. to serve different content for developers
on localhost only.
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 this pull request may close these issues.

2 participants