Part of #4287. Split out of the #4292 review so it does not widen that PR.
SocketAddr(u_long addr, int port) narrows the port without checking its range:
https://github.com/open-telemetry/opentelemetry-cpp/blob/main/ext/include/opentelemetry/ext/http/server/socket_tools.h
inet4.sin_port = htons(static_cast<uint16_t>(port));
So port = -1 becomes 65535, 65536 becomes 0, 99999 becomes 34463. HttpServer::addListeningPort(int port) passes straight through to this overload, so an out-of-range port silently binds a different one.
#4292 gave the string constructor a contract where a malformed port produces an invalid SocketAddr (AF_UNSPEC, port() == -1). The two constructors are now inconsistent: one rejects a bad port, the other wraps it.
This is deliberately not folded into #4292 because the integer overload has a real caller and changing its failure behaviour means deciding how addListeningPort should report a bad port (return value, clamp, or leave the socket unbound), which is a separate design question.
Suggested direction
Validate port < 0 || port > 65535 in the integer overload, leave AF_UNSPEC on failure to match the string constructor, and have addListeningPort surface the failure rather than binding a wrapped port. Add tests for the boundary values.
Part of #4287. Split out of the #4292 review so it does not widen that PR.
SocketAddr(u_long addr, int port)narrows the port without checking its range:https://github.com/open-telemetry/opentelemetry-cpp/blob/main/ext/include/opentelemetry/ext/http/server/socket_tools.h
So
port = -1becomes 65535,65536becomes 0,99999becomes 34463.HttpServer::addListeningPort(int port)passes straight through to this overload, so an out-of-range port silently binds a different one.#4292 gave the string constructor a contract where a malformed port produces an invalid
SocketAddr(AF_UNSPEC,port() == -1). The two constructors are now inconsistent: one rejects a bad port, the other wraps it.This is deliberately not folded into #4292 because the integer overload has a real caller and changing its failure behaviour means deciding how
addListeningPortshould report a bad port (return value, clamp, or leave the socket unbound), which is a separate design question.Suggested direction
Validate
port < 0 || port > 65535in the integer overload, leaveAF_UNSPECon failure to match the string constructor, and haveaddListeningPortsurface the failure rather than binding a wrapped port. Add tests for the boundary values.