Should we used net.JoinHostPort here, or was this on purpose to keep the old format?

If that's the case, perhaps we should have a comment here to describe that we did so on purpose.

Nope, that's an oversight. I'm not trying to preserve the old behavior because it seems buggy to me:

• You store a credential for https://registry.example.com:5000/v1/
• And you end up with https://registry.example.com/v1/:5000 in a subsequent list

Unless someone starts complaining because they're depending on that specific bug, I don't think we should preserve the old behavior. If that's the case, and we want to add an escape hatch, I think we can introduce an env var. Downside: integrations need to use client.NewShellProgramFunc to inherit the calling process' env vars, or NewShellProgramFuncWithEnv with that escape hatch specified.

And you end up with https://registry.example.com/v1/:5000 in a subsequent list

And now looking at that example, I suspect some of that is related to the assumption that we store creds per host, but I suspect the handling of that lived (or still lives) elsewhere.

I.e., the credentials helper would not show anything other than registry.example.com or registry.example.com:5000, and where needed for the credentials-store used, make that into a proper URL (with scheme); https://registry.example.com:5000.

That obviously fails if data was not normalised? And possibly part of that may have been because the "special case" for https://index.docker.io/v1/

Also see;

• Inconsistent auth keys on macOS, Windows, Linux #256
• Protocol should not be stripped out of the registry URL on login cli#190

Do we know if Path is always cleaned? Mostly because URL.String() doesn't do any cleaning up; https://go.dev/play/p/azv5LBXYlZo

package main

import (
	"fmt"
	"net/url"
)

func main() {
	u := url.URL{
		Scheme: "https",
		Host:   "::1:8080",
		Path:   "//////location/../../hello",
	}

	fmt.Println(u.String())
}

Will print https://::1:8080//////location/../../hello

Do we know if Path is always cleaned?

It's not, and never was. (I wrote a test checking if uncleaned paths are preserved, and tried to run it on this branch and on v0.8.2 -- it passes on both.)

I'm not sure if that's correct tbh, but at least this PR doesn't introduce yet another backward-incompatible change, so it should be okay from that standpoint.