reference icon indicating copy to clipboard operation
reference copied to clipboard

parsing breaks with IPv4-mapped IPv6 addresses

Open rogpeppe opened this issue 2 years ago • 3 comments

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 8507c7fcf0da9f570540c958ea7b972c30eeaeca, 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.

rogpeppe avatar Oct 17 '23 10:10 rogpeppe

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

milosgajdos avatar Oct 18 '23 09:10 milosgajdos

  • Handling of IPv6 IP-addresses was added in https://github.com/distribution/distribution/pull/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 https://github.com/distribution/distribution/pull/3489#issuecomment-905005613

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 https://github.com/distribution/distribution/pull/3489#discussion_r904431981

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.

thaJeztah avatar Oct 18 '23 10:10 thaJeztah

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

milosgajdos avatar Oct 18 '23 14:10 milosgajdos