Skip to content

Conversation

@arturobernalg
Copy link
Member

Implement RFC 6874 zone ID handling for IPv6 literals.
Round-trip: parse [fe80::1%25eth0] → "fe80::1%eth0", format → "[fe80::1%25eth0]"; validates ZoneID.
Touches URIAuthority/URIBuilder/HttpHost; includes tests; preserves compatibility.

@arturobernalg arturobernalg requested a review from ok2c October 10, 2025 14:02

// Bracket only real IPv6 literals; decide using the address part only (ignore zone)
if (ZoneIdSupport.isIPv6AddressPart(hostname)) {
ZoneIdSupport.appendBracketedIPv6(buffer, hostname);
Copy link
Member

Choose a reason for hiding this comment

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

@arturobernalg If I understand it right #isIPv6AddressPart will be executed twice. Is this needed?

Copy link
Member

Choose a reason for hiding this comment

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

@arturobernalg #isIPv6AddressPart still gets called twice. Once here, and another time inside #appendBracketedIPv6

}
}

public static boolean isIPv6AddressPart(final String host) {
Copy link
Member

Choose a reason for hiding this comment

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

@arturobernalg Please use CharSequence instead of String whenever possible.

Copy link
Member Author

Choose a reason for hiding this comment

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

@ok2c My bad. changed.

cursor.updatePos(initPos);
}

if (!cursor.atEnd() && s.charAt(cursor.getPos()) == '[') {
Copy link
Member

@ok2c ok2c Oct 11, 2025

Choose a reason for hiding this comment

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

@arturobernalg God Merciful. Do we really need to do all this? I have been trying to reduce our security footprint outside of our direct area of responsibility. This is just a matter of time some "security professionals" or "security researchers" will start crawling up your rectum claiming this code is potentially vulnerable to exploits by green men from Mars and demand an CVE with their name on it. I understand, we all have to make a living, so do security researches, but do we really need all that?

Again, why are we doing this? Do we really need to parse IPv6 addresses? Really.

Copy link
Member Author

Choose a reason for hiding this comment

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

@ok2c No IPv6 parsing—just bracket check + RFC 6874 zone encode/decode in one tiny helper; minimal surface, CONNECT untouched.

Copy link
Member

Choose a reason for hiding this comment

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

@arturobernalg Do not see those changes. There are also conflicts in the branch. Please double-check.

Copy link
Member Author

Choose a reason for hiding this comment

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

@ok2c should be fine now

… literal opaquely, decode/validate ZoneID; keep colon-count heuristic.
@ok2c
Copy link
Member

ok2c commented Oct 11, 2025

@arturobernalg I do not know what I am missing but I see no difference with the previous revision.

Generally I am not in favor of this change. IPv6 zones are none of our business.

@arturobernalg
Copy link
Member Author

@arturobernalg I do not know what I am missing but I see no difference with the previous revision.

Generally I am not in favor of this change. IPv6 zones are none of our business.

@ok2c I think i already remove the v6 parse. please take a another look

@ok2c
Copy link
Member

ok2c commented Oct 11, 2025

@arturobernalg I do not know what I am missing but I see no difference with the previous revision.
Generally I am not in favor of this change. IPv6 zones are none of our business.

@ok2c I think i already remove the v6 parse. please take a another look

@arturobernalg I still see no difference. I am sorry, but I do not understand what you are doing and more importantly why you are doing at all. I cannot approve this change-set.

@arturobernalg
Copy link
Member Author

@arturobernalg I do not know what I am missing but I see no difference with the previous revision.
Generally I am not in favor of this change. IPv6 zones are none of our business.

@ok2c I think i already remove the v6 parse. please take a another look

@arturobernalg I still see no difference. I am sorry, but I do not understand what you are doing and more importantly why you are doing at all. I cannot approve this change-set.

So i guess this pr has no sense anymore.

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.

2 participants