Update golang.org/x/net/idna to handle hostname started with numeric

[outsideris]

When I try to use terraform module with a custom private registry, I found terraform doesn't handle correctly hostname started with a numeric character like 123.example.com or 1example.com

module "vpc" {
  source = "123.example.com/namespace/name/provider"
}

So, I updated idna and added a new test case.

Previously, it throws an error like:

--- FAIL: TestForComparison (0.00s)
    --- FAIL: TestForComparison/1example.com (0.00s)
      svchost_test.go:173: unexpected error; want success
        error: idna: invalid label "1example"
      svchost_test.go:177: wrong result
        input: 1example.com
        got:
        want:  1example.com

[apparentlymart]

Hi @outsideris! Thanks for looking into this.

From looking at the history of golang/x/net between the previous and new commits here I see the following:

• idna: bug fix imported from x/text
• idna: updated to Unicode 10.0.0

I assume that the behavior change here is coming from the unicode tables update in that second commit. Given that switching from Unicode 9 to Unicode 10 may have other implications beyond this change, for safety I'd like to hold this update until our next major release, which has work in progress right now. This will allow us to mark the change as a compatibility note.

Since everything under golang.org/x/net is versioned together, it might make sense to upgrade all of it together. We may also upgrade golang.org/x/text at the same time, to ensure that we're using a consistent set of Unicode tables.

I'm going to label this as a breaking change (even though in practice it might not necessarily be) just to remind us to look at it again before the next release.

Thanks again for working on this!

[outsideris]

It looks like vendor.json migrated to modules.txt. Should I close this or migrate to module?

[apparentlymart]

Hi @outsideris,

We are indeed now using Go modules to manage the vendor directory rather than govendor. If you have the time and willingness to update this with Go modules then that'd be great! However, we're also happy to do that before merging if you would rather.

The following commands should get the dependency updated, and then your new test can be cherry-picked onto it:

go get -u github.com/golang/net
go mod vendor
git add go.mod go.sum vendor

I expect this will update some transitive dependencies as well; that's okay since we can use the 0.12 prereleases cycle to reduce the risk of unexpected changes.

[outsideris]

It looks like golang.org/x/net/idna is already updated as new version. So, I only add the test.
svchost test is passed, but master is failed currently. I can't find why CI is failed now.

$ make test TEST=./svchost/
==> Checking that code complies with gofmt requirements...
go generate ./...
2018/10/26 05:10:56 Generated command/internal_plugin_list.go
go list ./svchost/ | xargs -t -n4 go test  -timeout=2m -parallel=4
go test -timeout=2m -parallel=4 github.com/hashicorp/terraform/svchost
ok  	github.com/hashicorp/terraform/svchost	(cached)

[mildwonkey]

Thanks for adding a test, @outsideris!
We're steadily working on getting tests passing in master - the failures are all a known part of the 0.12-alpha release. It's nothing to do with this :D

I'll update this PR and merge it - thanks again for adding the test, that's fantastic.

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.