Fix multicast build and validation logic by PeimonBot · Pull Request #82 · adityarao2005/WebCraft

PeimonBot · 2026-02-16T06:00:44Z

Fixes declaration order and missing includes for multicast build:

socket.hpp: Define detail::is_multicast_address before multicast_group::resolve so the name is in scope (was declared later, causing universal compile failure).
async_udp.cpp: Add #include <stdexcept> for std::invalid_argument in join_group/leave_group and related code.

Made with Cursor

vercel · 2026-02-16T06:00:48Z

@PeimonBot is attempting to deploy a commit to the Aditya's projects Team on Vercel.

A member of the Team first needs to authorize it.

…alid_argument - Test resolve() directly for invalid and non-multicast addresses instead of relying on socket.join() and WEBCRAFT_UDP_MOCK. - Expect std::invalid_argument for 'not.an.ip.address' and '192.168.1.1'. Co-authored-by: Cursor <cursoragent@cursor.com>

…Pv6 multicast Windows Winsock defines IPV6_ADD_MEMBERSHIP and IPV6_DROP_MEMBERSHIP; POSIX uses IPV6_JOIN_GROUP and IPV6_LEAVE_GROUP. Add compatibility defines so the same code compiles on both. Co-authored-by: Cursor <cursoragent@cursor.com>

…entation - socket.hpp: tcp_listener::accept() now wraps descriptor->accept() result in tcp_socket; it returns task<tcp_socket> but was returning the raw task<std::shared_ptr<tcp_socket_descriptor>>, breaking GCC/Clang/MSVC. - async_udp.cpp: Normalize indentation of io_uring_udp_socket_descriptor sendto/join_group/leave_group to match rest of class (4 spaces). Co-authored-by: Cursor <cursoragent@cursor.com>

…::invalid_argument - socket.hpp: define detail::is_multicast_address before multicast_group::resolve so the name is in scope (was declared later, causing universal compile failure). - async_udp.cpp: add #include <stdexcept> for std::invalid_argument in join_group/leave_group and related code. Co-authored-by: Cursor <cursoragent@cursor.com>

…n rebase) Co-authored-by: Cursor <cursoragent@cursor.com>

- Add multicast_socket type alias (= udp_socket) and make_multicast_socket() - Expose join()/leave() on udp_socket delegating to descriptor join_group/leave_group - Add sendto(span, multicast_group) overload building connection_info from group Fixes compilation of test_async_io_multicast on all platforms. Co-authored-by: Cursor <cursoragent@cursor.com>

Copilot

Pull request overview

This PR adds multicast socket support to the webcraft async I/O library and fixes several build issues. The changes introduce multicast group address validation, join/leave operations, and corresponding test coverage. However, the implementation is incomplete as multicast operations are only implemented for Linux, leaving Windows and macOS with non-functional no-op implementations.

Changes:

Added multicast group address validation and join/leave operations to the UDP socket API
Fixed declaration order of detail::is_multicast_address to resolve compilation failures
Fixed tcp_listener::accept() to properly construct tcp_socket instances
Added comprehensive multicast test suite

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.

File	Description
include/webcraft/async/io/socket.hpp	Adds multicast_group struct with validation, multicast_join_options, and extends udp_socket with join/leave methods; fixes tcp_listener::accept() constructor call
src/webcraft/async_udp.cpp	Adds `<stdexcept>` include, Windows IPv6 compatibility macros, and Linux-only join_group/leave_group implementations
tests/src/test_async_io_multicast.cpp	Adds comprehensive multicast test suite covering group resolution, join/leave operations, send/receive, and invalid address handling

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-17T00:48:07Z

tests/src/test_async_io_multicast.cpp

+#define TEST_SUITE_NAME MulticastSocketTestSuite
+
+#ifndef WEBCRAFT_HAS_MULTICAST
+#define WEBCRAFT_HAS_MULTICAST 1


The WEBCRAFT_HAS_MULTICAST macro is unconditionally set to 1 at the top of this test file (line 9), which means the tests will always attempt to run multicast operations even on platforms (Windows, macOS) where join_group and leave_group are not implemented.

Since these methods fall back to no-op default implementations on unsupported platforms, the tests will pass but won't actually test multicast functionality on Windows/macOS. Consider either:

Setting WEBCRAFT_HAS_MULTICAST based on the actual platform support (e.g., only set to 1 on Linux), or

Making the tests verify that multicast operations actually work rather than just checking that they don't throw exceptions

Suggested change

#define WEBCRAFT_HAS_MULTICAST 1

# if defined(__linux__)

# define WEBCRAFT_HAS_MULTICAST 1

# else

# define WEBCRAFT_HAS_MULTICAST 0

# endif

Copilot · 2026-02-17T00:48:08Z

include/webcraft/async/io/socket.hpp

        task<tcp_socket> accept()
        {
-            co_return co_await descriptor->accept();
+            co_return tcp_socket(co_await descriptor->accept());


This fix to tcp_listener::accept() is not mentioned in the PR description. The change adds an explicit tcp_socket constructor call that was missing, which would have caused a compilation error (cannot convert std::shared_ptr<tcp_socket_descriptor> to tcp_socket without calling the constructor).

This appears to be an unrelated bug fix that was included in this PR. Consider either:

Updating the PR description to mention this fix, or

Moving this fix to a separate PR if it's not directly related to the multicast functionality

Copilot · 2026-02-17T00:48:08Z

include/webcraft/async/io/socket.hpp

+            if (addr.find(':') != std::string::npos)
+            {
+                // IPv6: ff00::/8 — first 16-bit segment must be 0xff00–0xffff
+                std::size_t i = 0;
+                while (i < addr.size() && addr[i] == ':') ++i;
+                while (i < addr.size())
+                {
+                    std::size_t start = i;
+                    while (i < addr.size() && addr[i] != ':')
+                    {
+                        char ch = addr[i];
+                        if ((ch >= '0' && ch <= '9') || (ch >= 'a' && ch <= 'f') || (ch >= 'A' && ch <= 'F'))
+                            ++i;
+                        else
+                            return false;
+                    }
+                    if (i > start)
+                    {
+                        if (i - start >= 2u && i - start <= 4u)
+                        {
+                            if ((addr[start] == 'f' || addr[start] == 'F') && (addr[start + 1] == 'f' || addr[start + 1] == 'F'))
+                                return true;
+                        }
+                        return false;
+                    }
+                    if (i < addr.size()) ++i;
+                }
+                return false;
+            }


The IPv6 multicast address validation logic is incomplete and will incorrectly validate many IPv6 addresses. The current implementation only checks if the first hexadecimal segment starts with 'ff', but this has several issues:

It only checks the first non-empty segment encountered, not necessarily the first segment of the address

It returns true immediately upon finding 'ff', without validating the rest of the address format

It doesn't handle IPv6 compression (::) correctly

Valid multicast addresses like "ff02::1" would fail because the implementation doesn't properly handle the :: compression

For correct validation, consider using inet_pton(AF_INET6, ...) followed by IN6_IS_ADDR_MULTICAST() macro (which checks if the first byte is 0xff), similar to how it's done in the join_group implementation in async_udp.cpp. This would be more robust and consistent with the runtime validation.

Copilot · 2026-02-17T00:48:08Z

src/webcraft/async_udp.cpp

+    void join_group(const webcraft::async::io::socket::multicast_group &group, const webcraft::async::io::socket::multicast_join_options &) override
+    {
+        if (socket < 0) return;
+        in_addr maddr4;
+        if (inet_pton(AF_INET, group.host.c_str(), &maddr4) == 1)
+        {
+            if (!IN_MULTICAST(ntohl(maddr4.s_addr)))
+                throw std::invalid_argument("Not a multicast address: " + group.host);
+            struct ip_mreq mreq{};
+            mreq.imr_multiaddr = maddr4;
+            mreq.imr_interface.s_addr = INADDR_ANY;
+            if (setsockopt(socket, IPPROTO_IP, IP_ADD_MEMBERSHIP, &mreq, sizeof(mreq)) < 0)
+            {
+                std::error_code ec(errno, std::system_category());
+                throw std::system_error(ec, "Failed to join IPv4 multicast group");
+            }
+            return;
+        }
+        struct in6_addr maddr6;
+        if (inet_pton(AF_INET6, group.host.c_str(), &maddr6) == 1)
+        {
+            if (!IN6_IS_ADDR_MULTICAST(&maddr6))
+                throw std::invalid_argument("Not a multicast address: " + group.host);
+            struct ipv6_mreq mreq6{};
+            mreq6.ipv6mr_multiaddr = maddr6;
+            mreq6.ipv6mr_interface = 0;
+            if (setsockopt(socket, IPPROTO_IPV6, IPV6_JOIN_GROUP, &mreq6, sizeof(mreq6)) < 0)
+            {
+                std::error_code ec(errno, std::system_category());
+                throw std::system_error(ec, "Failed to join IPv6 multicast group");
+            }
+            return;
+        }
+        throw std::invalid_argument("Invalid multicast address: " + group.host);
+    }
+
+    void leave_group(const webcraft::async::io::socket::multicast_group &group) override
+    {
+        if (socket < 0) return;
+        in_addr maddr4;
+        if (inet_pton(AF_INET, group.host.c_str(), &maddr4) == 1)
+        {
+            struct ip_mreq mreq{};
+            mreq.imr_multiaddr = maddr4;
+            mreq.imr_interface.s_addr = INADDR_ANY;
+            setsockopt(socket, IPPROTO_IP, IP_DROP_MEMBERSHIP, &mreq, sizeof(mreq));
+            return;
+        }
+        struct in6_addr maddr6;
+        if (inet_pton(AF_INET6, group.host.c_str(), &maddr6) == 1)
+        {
+            struct ipv6_mreq mreq6{};
+            mreq6.ipv6mr_multiaddr = maddr6;
+            mreq6.ipv6mr_interface = 0;
+            setsockopt(socket, IPPROTO_IPV6, IPV6_LEAVE_GROUP, &mreq6, sizeof(mreq6));
+        }
+    }


The join_group and leave_group methods are only implemented for the Linux io_uring_udp_socket_descriptor class. The Windows (iocp_udp_socket_descriptor) and macOS (kqueue_udp_socket_descriptor) implementations do not have these methods, which means they will fall back to the no-op default implementation in the base class. This means multicast functionality will silently fail on Windows and macOS platforms.

Either:

Implement join_group and leave_group for Windows and macOS (recommended), or

Document this platform limitation clearly in the multicast_group API documentation and throw an appropriate exception from the default implementations rather than silently doing nothing.

Copilot · 2026-02-17T00:48:08Z

src/webcraft/async_udp.cpp

+    void leave_group(const webcraft::async::io::socket::multicast_group &group) override
+    {
+        if (socket < 0) return;
+        in_addr maddr4;
+        if (inet_pton(AF_INET, group.host.c_str(), &maddr4) == 1)
+        {
+            struct ip_mreq mreq{};
+            mreq.imr_multiaddr = maddr4;
+            mreq.imr_interface.s_addr = INADDR_ANY;
+            setsockopt(socket, IPPROTO_IP, IP_DROP_MEMBERSHIP, &mreq, sizeof(mreq));
+            return;
+        }
+        struct in6_addr maddr6;
+        if (inet_pton(AF_INET6, group.host.c_str(), &maddr6) == 1)
+        {
+            struct ipv6_mreq mreq6{};
+            mreq6.ipv6mr_multiaddr = maddr6;
+            mreq6.ipv6mr_interface = 0;
+            setsockopt(socket, IPPROTO_IPV6, IPV6_LEAVE_GROUP, &mreq6, sizeof(mreq6));
+        }


The leave_group implementation does not validate that the provided address is a valid multicast address before attempting to leave. While join_group validates using IN_MULTICAST and IN6_IS_ADDR_MULTICAST, leave_group silently ignores invalid addresses. This inconsistency could mask programming errors where invalid addresses are passed to leave_group.

For consistency and to help catch errors early, consider adding the same validation in leave_group that exists in join_group (lines 813-814 and 828-829).

Copilot · 2026-02-17T00:48:09Z

tests/src/test_async_io_multicast.cpp

+#if WEBCRAFT_HAS_MULTICAST
+TEST_CASE(TestMulticastInvalidAddressThrows)
+{
+    // As of PR #79, resolve() validates and throws std::invalid_argument for non-multicast/invalid addresses.


The comment references "PR #79" but this appears to be part of this current PR introducing the multicast functionality. This reference should either be removed or updated to reflect the actual PR number once merged, as it could be confusing for future readers.

Suggested change

// As of PR #79, resolve() validates and throws std::invalid_argument for non-multicast/invalid addresses.

// resolve() validates and throws std::invalid_argument for non-multicast/invalid addresses.

Copilot · 2026-02-17T00:48:09Z

src/webcraft/async_udp.cpp

+    void leave_group(const webcraft::async::io::socket::multicast_group &group) override
+    {
+        if (socket < 0) return;
+        in_addr maddr4;
+        if (inet_pton(AF_INET, group.host.c_str(), &maddr4) == 1)
+        {
+            struct ip_mreq mreq{};
+            mreq.imr_multiaddr = maddr4;
+            mreq.imr_interface.s_addr = INADDR_ANY;
+            setsockopt(socket, IPPROTO_IP, IP_DROP_MEMBERSHIP, &mreq, sizeof(mreq));
+            return;
+        }
+        struct in6_addr maddr6;
+        if (inet_pton(AF_INET6, group.host.c_str(), &maddr6) == 1)
+        {
+            struct ipv6_mreq mreq6{};
+            mreq6.ipv6mr_multiaddr = maddr6;
+            mreq6.ipv6mr_interface = 0;
+            setsockopt(socket, IPPROTO_IPV6, IPV6_LEAVE_GROUP, &mreq6, sizeof(mreq6));
+        }


The error handling is inconsistent between join_group and leave_group. The join_group method throws std::system_error when setsockopt fails (lines 818-822, 833-837), but leave_group silently ignores setsockopt failures (lines 852, 861).

This inconsistency could make debugging difficult. If leaving a multicast group fails, the application should likely be notified, especially since the join operation is careful to report errors. Consider either:

Making leave_group throw on setsockopt failure for consistency with join_group, or

Documenting why failures are acceptable in leave_group (e.g., if the socket is being closed anyway)

PeimonBot requested a review from adityarao2005 as a code owner February 16, 2026 06:00

PeimonBot and others added 5 commits February 16, 2026 06:03

fix: add join_group/leave_group to udp_socket_descriptor base (lost i…

d0c81a7

…n rebase) Co-authored-by: Cursor <cursoragent@cursor.com>

PeimonBot force-pushed the fix/multicast-build branch from 3b0d9de to d0c81a7 Compare February 16, 2026 06:05

adityarao2005 requested review from Copilot and removed request for Copilot February 16, 2026 06:05

Copilot started reviewing on behalf of adityarao2005 February 16, 2026 06:05 View session

adityarao2005 requested a review from Copilot February 17, 2026 00:42

Copilot started reviewing on behalf of adityarao2005 February 17, 2026 00:42 View session

Copilot AI reviewed Feb 17, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix multicast build and validation logic#82

Fix multicast build and validation logic#82
PeimonBot wants to merge 6 commits intoadityarao2005:mainfrom
PeimonBot:fix/multicast-build

PeimonBot commented Feb 16, 2026

Uh oh!

vercel bot commented Feb 16, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 17, 2026

Uh oh!

Copilot AI Feb 17, 2026

Uh oh!

Copilot AI Feb 17, 2026

Uh oh!

Copilot AI Feb 17, 2026

Uh oh!

Copilot AI Feb 17, 2026

Uh oh!

Copilot AI Feb 17, 2026

Uh oh!

Copilot AI Feb 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-#define WEBCRAFT_HAS_MULTICAST 1
+#  if defined(__linux__)
+#    define WEBCRAFT_HAS_MULTICAST 1
+#  else
+#    define WEBCRAFT_HAS_MULTICAST 0
+#  endif

	// As of PR #79, resolve() validates and throws std::invalid_argument for non-multicast/invalid addresses.
	// resolve() validates and throws std::invalid_argument for non-multicast/invalid addresses.

Conversation

PeimonBot commented Feb 16, 2026

Uh oh!

vercel bot commented Feb 16, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants