[go: up one dir, main page]
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

parsing breaks with IPv4-mapped IPv6 addresses #8

Open
rogpeppe opened this issue Oct 17, 2023 · 3 comments
Open

parsing breaks with IPv4-mapped IPv6 addresses #8

rogpeppe opened this issue Oct 17, 2023 · 3 comments

Comments

@rogpeppe
Copy link
rogpeppe commented Oct 17, 2023 •
edited
Loading

The IPv6 address syntax allows forms such as ::ffff:192.0.2.128, but the reference parsing code, despite allowing other forms of IPv6 address, doesn't seem to allow these, even though the Go net package does.

For example, running against commit 8507c7f, this code fails:

package main

import (
	"fmt"

	"github.com/distribution/reference"
)

func main() {
	_, err := reference.ParseAnyReference("[::ffff:192.0.2.128]/bar")
	if err != nil {
		fmt.Println("error:", err)
	}
}

The error it prints is:

error: invalid reference format

Although the code says "Special addresses such as IPv4-Mapped are deliberately excluded", ISTM that that exclusion is not entirely warranted, as it's reasonable to specify an address in this way. The same cannot be said of zone identifiers.
@milosgajdos
Copy link
Member

@thaJeztah is there a reason we've changed this behaviour 🤔

@thaJeztah
Copy link
Member
  • Handling of IPv6 IP-addresses was added in Ipv6 registries distribution#3489, before that IPv6 was not handled in any form, so this looks to be a feature request / enhancement.

I don't think there's a specific reason, other than potentially to reduce complexity of the regex; I recall I suggested reducing the scope of the regex for this purpose (and handle further processing after this) in distribution/distribution#3489 (comment)

So, I think the current implementation was done to provide the (minimum) requirements for IPv6 addresses at that time, and the documentation was added to reflect what's supported to prevent false expectations; see distribution/distribution#3489 (comment)

We must make sure that there's no ambiguity in such references, as the "is it a domain?", "is it an IP?" or "is it a namespace on the default registry?" is already complex, and allowed formats of IPv6 addresses is a bit of a wild-west, but I THINK with the requirement to have these addresses using the notation with square brackets ([<address>]) should help with that, so perhaps not a concern. Performance is definitely still something to keep in mind though, so if we would update, we should consider what part to handle as part of the regex, and what part to handle in code after that.

@milosgajdos
Copy link
Member

Aha! I remember that PR now. Solid git archaeology @thaJeztah Thanks for refreshing my memory and for adding the context 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@rogpeppe @milosgajdos @thaJeztah