Skip to content

fix(toolbox): Correctly print unix sockets with no path #310

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.

Sign up for GitHub

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

Closed
ItsLifeJim wants to merge 1 commit into from
Closed

fix(toolbox): Correctly print unix sockets with no path #310

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions toolbox/net/Endpoint.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,17 @@ template <typename StreamT, typename T>
StreamT& print_unix_endpoint(StreamT& os, const T& ep)
{
constexpr const char* scheme = "unix://";
// abstract unix socket's path starts with '\0' and not null-terminated
const size_t family_size = sizeof(std::declval<sockaddr_un>().sun_family);
if (ep.size() <= family_size) {
// No path
os << scheme;
return os;
}
const auto* path = reinterpret_cast<const sockaddr_un*>(ep.data())->sun_path;
if (path[0] == '\0') {
size_t size = ep.size() - sizeof(std::declval<sockaddr_un>().sun_family) - 1;
os << scheme << '|' << std::string_view{path + 1, size};
// abstract unix socket's path starts with '\0' and not null-terminated
const size_t path_size = ep.size() - family_size;
os << scheme << '|' << std::string_view{path + 1, path_size - 1};
return os;
}
os << scheme << *ep.data();
Copy link
Contributor

@bnbajwa bnbajwa Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm not sure null terminator is guaranteed for non-abstract unix sockets -- in which case this could print a very large sequence of bytes. I think this should be bounded by path_size at least. Maybe something like this?

auto path_view = std::string_view{path, path_size};
if (const auto n = path_view.find('\0'); n != std::string_view::npos) {
    path_view = path_view.substr(0, n);
}
os << scheme << path_view;

Copy link
Contributor

@bnbajwa bnbajwa Mar 3, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm actually i don't even know what << *ep.data() would print?

Copy link
Contributor

@bnbajwa bnbajwa Mar 3, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, so it actually invokes this function at bottom, which then calls first function:

template <typename StreamT>
    requires toolbox::util::Streamable<StreamT>
StreamT& operator<<(StreamT& os, const sockaddr_un& sa)
{
    os << sa.sun_path;
    return os;
}

template <typename StreamT>
    requires toolbox::util::Streamable<StreamT>
StreamT& operator<<(StreamT& os, const sockaddr& sa)
{
    if (sa.sa_family == AF_INET) {
        os << reinterpret_cast<const sockaddr_in&>(sa);
    } else if (sa.sa_family == AF_INET6) {
        os << reinterpret_cast<const sockaddr_in6&>(sa);
    } else if (sa.sa_family == AF_UNIX) {
        os << reinterpret_cast<const sockaddr_un&>(sa);
    } else {
        os << "<sockaddr>";
    }
    return os;
}

os << sa.sun_path
yeh don't think this is right for reason explained above -- i don't think null terminator is guaranteed -- in which case this will print a large number of bytes. this should be fixed as well?

Expand Down
22 changes: 22 additions & 0 deletions toolbox/net/Endpoint.ut.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,28 @@ BOOST_AUTO_TEST_CASE(ParseStreamUnixAbstractCase)
BOOST_CHECK_EQUAL(to_string(ep), uri);
}

BOOST_AUTO_TEST_CASE(ParseStreamUnixAbstractEmptyCase)
{
const auto uri = "unix://|"s;
const auto ep = parse_stream_endpoint(uri);
BOOST_CHECK_EQUAL(ep.protocol().family(), AF_UNIX);
BOOST_CHECK_EQUAL(ep.protocol().type(), SOCK_STREAM);
BOOST_CHECK_EQUAL(ep.protocol().protocol(), 0);
BOOST_CHECK_EQUAL(to_string(ep), uri);
}

BOOST_AUTO_TEST_CASE(ParseStreamUnixEmptyCase)
{
auto sock = os::socket(net::UnixStreamProtocol{});
StreamEndpoint ep;
get_sock_name(sock.get(), ep);

BOOST_CHECK_EQUAL(ep.protocol().family(), AF_UNIX);
BOOST_CHECK_EQUAL(ep.protocol().type(), SOCK_STREAM);
BOOST_CHECK_EQUAL(ep.protocol().protocol(), 0);
BOOST_CHECK_EQUAL(to_string(ep), "unix://");
}

BOOST_AUTO_TEST_CASE(ParseStreamBindCase)
{
const auto ep = parse_stream_endpoint("tcp4://:80");
Expand Down