Preserve port when computing the login URL #6711

Palleas · 2023-07-14T21:04:17Z

When using a registry URL that contains a port, ensure the port is preserved when computing the login URL.

Motivation:

Currently if you run the following

swift package-registry set https://packages.example.com:8081
swift package-registry login --token XYZ

You might get something like

Error: Unable to connect to server: https://packages.example.com

That's because the login url is built like this and ignores the port: URL(string: "https://\(host)\(loginAPIPath ?? "/login")")

Modifications:

Use URLComponents to build the login URL (which maintains the port)
Add a testing target for SwiftPackageRegistryTool

Result:

SPM can use registry URLs with ports.

I do have a question though, the registry authentication configuration dingus currently uses the host as the key:
https://github.com/apple/swift-package-manager/blob/main/Sources/PackageRegistryTool/PackageRegistryTool%2BAuth.swift#L234-L237

I suspect it's pretty unlikely that someone would use multiple registries available at the same host on different ports, but should I update that, as well?

yim-lee · 2023-07-14T21:08:16Z

@swift-ci please test

yim-lee · 2023-07-14T21:10:29Z

Tests/PackageRegistryToolTests/PackageRegistryToolTests.swift

+import XCTest
+@testable import PackageRegistryTool
+
+final class PackageRegistryToolTests: XCTestCase {


These new tests can go into CommandsTests/PackageRegistryToolTests.swift.

Roger that, done!

yim-lee · 2023-07-14T21:14:25Z

@swift-ci please smoke test

yim-lee · 2023-07-14T21:34:52Z

I suspect it's pretty unlikely that someone would use multiple registries available at the same host on different ports, but should I update that, as well?

I agree that it's unlikely to have multiple registries on the same host.

If we were to support/fix this however, I don't know if using host:port instead of just host is good enough. What if instead of different ports, different paths (e.g., /staging-1, /staging-2) on the same host are used for multiple registries? Using (registry) URLs as keys could make things tricky in RegistryClient. But then I think distinguishing registries by paths is even less likely to happen than ports.

I think changing from host to host:port is pretty straight-forward--the :port portion should only be added if registryURL.port is non-nil, so I am open to us making that change. I don't think the change is a MUST though.

Palleas · 2023-07-14T22:22:39Z

I think changing from host to host:port is pretty straight-forward--the :port portion should only be added if registryURL.port is non-nil, so I am open to us making that change. I don't think the change is a MUST though.

Done!

Sources/PackageRegistryTool/PackageRegistryTool+Auth.swift

yim-lee · 2023-07-17T19:27:56Z

@swift-ci please smoke test

yim-lee · 2023-07-17T19:28:27Z

@swift-ci please test Windows

yim-lee · 2023-07-17T19:29:56Z