fix(http): reject out-of-range listening ports

[noob339]

Fixes #4305

Changes

• Added range validation to SocketAddr(u_long addr, int port) so ports below 0 or above 65535 are rejected instead of wrapping during the uint16_t conversion.
• Updated HttpServer::addListeningPort(int port) to return -1 for an invalid port before creating or binding a socket.
• Added a unit test covering invalid values including -1, 65536, and 99999.

Tested with:

bazel test //ext/test/http:curl_http_test \
  --test_filter=HttpServerTest.RejectsOutOfRangeListeningPorts

• CHANGELOG.md updated for non-trivial changes
• Unit tests have been added
• Changes in public API reviewed

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: noob339 / name: noob339 (d47ebf3)

[thc1006]

Thanks for picking this up, and the fix reads right to me. Two things that are easier to sort out now than after review.

The description says Fixes #4287, which is the audit tracker holding a dozen or so separate defects. Merging with that keyword would close the whole tracker. Fixes #4305 is the one this resolves.

#4292 is approved and touches the same part of socket_tools.h, so this will want a rebase once it lands. It gives the string constructor the same contract you are using here, an invalid address left as AF_UNSPEC with port() returning -1, so the two constructors end up consistent. It also adds ext/test/http/socket_tools_test.cc, which may be a more natural home for the new test than curl_http_test.cc.

I can answer anything about the issue if that helps.

[noob339]

Thanks for catching that. I updated the description to use Fixes #4305 instead of closing the audit tracker.

I’ll rebase once #4292 lands and resolve any overlap in socket_tools.h. I’ll also move the port validation coverage into ext/test/http/socket_tools_test.cc after the rebase, since that sounds like the better location for the constructor-level tests.

[codecov]

Codecov Report

❌ Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 80.86%. Comparing base (4f8c843) to head (4de6266).

Files with missing lines	Patch %	Lines
...nclude/opentelemetry/ext/http/server/http_server.h	66.67%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4320      +/-   ##
==========================================
- Coverage   80.87%   80.86%   -0.01%     
==========================================
  Files         450      450              
  Lines       19186    19190       +4     
==========================================
+ Hits        15515    15516       +1     
- Misses       3671     3674       +3     

Files with missing lines	Coverage Δ
...clude/opentelemetry/ext/http/server/socket_tools.h	95.73% <100.00%> (+0.05%)	⬆️
...nclude/opentelemetry/ext/http/server/http_server.h	66.59% <66.67%> (-0.08%)	⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[noob339]

Thanks again for the guidance. I rebased onto the merged #4292 changes, moved the integer SocketAddr coverage into ext/test/http/socket_tools_test.cc, and updated the PR to reference Fixes #4305.

The focused Bazel test passes locally. Let me know if any other suggestions arise please!

[dbarker]

Thanks for the pr. This impacts internal http server used in testing and the http example. Not for production use.

[marcalff]

Instead of doing all this, how about changing port to uint16_t ?

[marcalff]

Thanks for the contribution.

Please see comments for a simpler solution.