Skip to content

fix: Support IPv6 bind addresses in http_server helper#5239

Open
I501307 wants to merge 9 commits intofluent:masterfrom
I501307:fix/http-server-ipv6-uri-support
Open

fix: Support IPv6 bind addresses in http_server helper#5239
I501307 wants to merge 9 commits intofluent:masterfrom
I501307:fix/http-server-ipv6-uri-support

Conversation

@I501307
Copy link

@I501307 I501307 commented Jan 27, 2026

Which issue(s) this PR fixes:
Fixes # (if there's an issue, or remove this line)

What this PR does / why we need it:
Fixes URI::InvalidURIError when binding to IPv6 addresses. The http_server
helper was generating invalid URIs like 'http://:::24231' instead of the
RFC 3986 compliant 'http://[::]:24231'.

Release Note:
http_server helper: Fix IPv6 bind address support

Fixes URI::InvalidURIError when binding to IPv6 addresses like '::' or '::1'.
The http_server helper was generating invalid URIs such as 'http://:::24231'
instead of the RFC 3986 compliant 'http://[::]:24231'.

Changes:
- Wrap IPv6 addresses in brackets when constructing URIs
- Add unit tests for IPv6 localhost (::1) and wildcard (::) binding
- Update CHANGELOG.md

This bug affected all Fluentd versions including v1.19.1 and caused
crashes when attempting to bind HTTP servers to IPv6 addresses in
dual-stack environments.

Resolves: Invalid URI generation for IPv6 bind addresses
Signed-off-by: Jesse Awan <jesse.awan@sap.com>
@I501307 I501307 force-pushed the fix/http-server-ipv6-uri-support branch from a279091 to eb72e2a Compare January 27, 2026 21:00
@I501307
Copy link
Author

I501307 commented Jan 27, 2026

Could a maintainer please add the bug label? This fixes a crash affecting all Fluentd versions when binding to IPv6 addresses.

- Updated server.rb to check if IPv6 addresses are already bracketed
- Added tests for pre-bracketed addresses ([::1] and [::])
- Improved ipv6_enabled? helper to verify both binding and resolution
- Updated CHANGELOG to document pre-bracketed address handling

Signed-off-by: Jesse Awan <jesse.awan@sap.com>
@I501307 I501307 requested a review from Watson1978 January 28, 2026 09:52
@I501307
Copy link
Author

I501307 commented Jan 28, 2026

@Watson1978 Same issue I encountered within a plugin as well here :
https://github.com/fluent/fluent-plugin-prometheus/pull/240/changes

Signed-off-by: Jesse Awan <jesse.awan@sap.com>
@I501307 I501307 force-pushed the fix/http-server-ipv6-uri-support branch from bca5979 to 77b380a Compare February 2, 2026 11:11
@I501307 I501307 requested a review from kenhys February 2, 2026 11:12
@Watson1978
Copy link
Contributor

@I501307 Seems it fails the test in CI. Can you take a look at it?

Signed-off-by: Jesse Awan <jesse.awan@sap.com>
@I501307 I501307 force-pushed the fix/http-server-ipv6-uri-support branch from 35a521a to 22e03d2 Compare February 3, 2026 10:44
@I501307
Copy link
Author

I501307 commented Feb 3, 2026

@I501307 Seems it fails the test in CI. Can you take a look at it?

Root Cause:
The original code didn't handle pre-bracketed IPv6 addresses like [::1] or [::]. When passed to Async::HTTP::Endpoint, socket binding failed on Unix systems because brackets are only for URI formatting, not socket binding.

The Fix (2 parts):

server.rb - Normalize addresses on initialization

  • Strip brackets from input addresses (e.g., [::1] → ::1)
  • Store unbracketed address for socket binding
  • Add brackets only when constructing URIs

test_http_server_helper.rb - Replace flaky binding tests

Before: Tests tried to bind to IPv6 addresses → failed on CI due to IPv6 networking issues
After: Tests only validate formatting logic → no networking required, works everywhere
Verified locally:

All 19 http_server tests pass (0 errors, 0 failures)
Full test suite: 811+ tests passing before completion
The new tests should pass on all platforms (Ubuntu, macOS, Windows) across Ruby 3.2-4.0. 🚀

@I501307
Copy link
Author

I501307 commented Feb 4, 2026

@I501307 Seems it fails the test in CI. Can you take a look at it?

@Watson1978 The Windows failures are unrelated to this PR - they're in test_out_exec_filter.rb
(expecting "5500" but getting "4184"), not in the http_server_helper tests.

All http_server_helper tests pass on all platforms. Can a maintainer re-run
the Windows CI or acknowledge these are known flaky tests?

Copy link
Contributor

@kenhys kenhys left a comment

Choose a reason for hiding this comment

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

Thanks @I501307 !!

If you afford to write, it might be safer to write with conditional networking test case additionally.

test ... do
  omit ""  unless ipv6_enabled?
  ...
end

- Rename addr_display to ip_literal per RFC 3986 terminology
- Move ipv6_enabled? helper to test/helper.rb for reusability
- Improve IPv6 detection with proper socket binding and address resolution checks

Signed-off-by: Jesse Awan <jesse.awan@sap.com>
@I501307 I501307 force-pushed the fix/http-server-ipv6-uri-support branch from b703cbd to f6d21ff Compare February 5, 2026 15:56
@I501307
Copy link
Author

I501307 commented Feb 5, 2026

Thanks @I501307 !!

If you afford to write, it might be safer to write with conditional networking test case additionally.

test ... do
  omit ""  unless ipv6_enabled?
  ...
end

@kenhys Done! I've renamed the variable to ip_literal and moved ipv6_enabled? to test/helper.rb with improved IPv6 detection logic. Thanks for the review feedback!

@I501307 I501307 requested a review from kenhys February 5, 2026 15:59
Watson1978
Watson1978 previously approved these changes Feb 6, 2026
Copy link
Contributor

@Watson1978 Watson1978 left a comment

Choose a reason for hiding this comment

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

Thanks 👍🏻

@Watson1978
Copy link
Contributor

I've tried to fix load error of ostruct at #5249

@I501307
Copy link
Author

I501307 commented Feb 6, 2026

I've tried to fix load error of ostruct at #5249

ah great.
Ruby 4.0 moved ostruct out of default gems, and filter_record_transformer.rb is trying to load it.

Copy link
Contributor

@kenhys kenhys left a comment

Choose a reason for hiding this comment

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

After: Tests only validate formatting logic → no networking required, works everywhere
Verified locally:

Thanks!

It works great without network, but if plugin helper implementation was changed (in the future), also
need to follow test code too.
If we forgot to follow (it might be accidentally happen), it becomes meaningless test cases there.
So I have concern about extracting internal logic directly to test code.
What do you think? @Watson1978

@Watson1978
Copy link
Contributor

What do you think? @Watson1978

Sorry, I didn't get to look closely at the test code.

Indeed.

It should not only test the logic itself.
it should actually run the http_server helper to verify it behaves as expected.

@Watson1978 Watson1978 self-requested a review February 6, 2026 06:06
@Watson1978 Watson1978 dismissed their stale review February 6, 2026 06:08

I need more review for test codes

Signed-off-by: Jesse Awan <jesse.awan@sap.com>
Signed-off-by: Jesse Awan <jesse.awan@sap.com>
@I501307
Copy link
Author

I501307 commented Feb 6, 2026

What do you think? @Watson1978

Sorry, I didn't get to look closely at the test code.

Indeed.

It should not only test the logic itself. it should actually run the http_server helper to verify it behaves as expected.

Integration tests are now added https://github.com/fluent/fluentd/pull/5239/changes#diff-0733209104ce038bd67b2898b481ab3b6ff2634063b71948ae8a230bb3d5bcf3R23-R29 🌻

@I501307
Copy link
Author

I501307 commented Feb 6, 2026

After: Tests only validate formatting logic → no networking required, works everywhere
Verified locally:

Thanks!

It works great without network, but if plugin helper implementation was changed (in the future), also need to follow test code too. If we forgot to follow (it might be accidentally happen), it becomes meaningless test cases there. So I have concern about extracting internal logic directly to test code. What do you think? @Watson1978

done here https://github.com/fluent/fluentd/pull/5239/changes#diff-0733209104ce038bd67b2898b481ab3b6ff2634063b71948ae8a230bb3d5bcf3R23-R29 🌻

@I501307 I501307 requested a review from kenhys February 6, 2026 09:24
@I501307
Copy link
Author

I501307 commented Feb 6, 2026

@kenhys Please let me know if more tests are required.

… tests

Signed-off-by: Jesse Awan <jesse.awan@sap.com>
@I501307 I501307 force-pushed the fix/http-server-ipv6-uri-support branch from 0280d82 to 046cc25 Compare February 6, 2026 09:40
Copy link
Contributor

@Watson1978 Watson1978 left a comment

Choose a reason for hiding this comment

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

Thanks

@Watson1978 Watson1978 added this to the v1.20.0 milestone Feb 6, 2026
@I501307 I501307 requested a review from Watson1978 February 6, 2026 12:32
@I501307
Copy link
Author

I501307 commented Feb 6, 2026

@kenhys we good to go 🚀

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.

3 participants