code review 6675043: net: Add function to expose NS records for a domain

net: add LookupNS(domain string)
Fixes issue 4224.

[smcquay, 2012-10-13 04:10:11]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go

[dfc, 2012-10-13 04:20:21]

Fixes issue 4224.

On Sat, Oct 13, 2012 at 3:10 PM,  <stephen@mcquay.me> wrote:
> Reviewers: golang-dev_googlegroups.com,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go
>
>
> Description:
> net: Add function to expose NS records for a domain
>
>     Fixes issue 4424.
>
> Please review this at http://codereview.appspot.com/6675043/
>
> Affected files:
>   M src/pkg/net/dnsclient.go
>   M src/pkg/net/lookup.go
>   M src/pkg/net/lookup_test.go
>   M src/pkg/net/lookup_unix.go
>
>
> Index: src/pkg/net/dnsclient.go
> ===================================================================
> --- a/src/pkg/net/dnsclient.go
> +++ b/src/pkg/net/dnsclient.go
> @@ -227,6 +227,11 @@
>         Pref uint16
>  }
>
> +// An NS represents a single DNS NS record.
> +type NS struct {
> +       Host string
> +}
> +
>  // byPref implements sort.Interface to sort MX records by preference
>  type byPref []*MX
>
> Index: src/pkg/net/lookup.go
> ===================================================================
> --- a/src/pkg/net/lookup.go
> +++ b/src/pkg/net/lookup.go
> @@ -47,6 +47,11 @@
>         return lookupMX(name)
>  }
>
> +// LookupNS returns the DNS NS records for the given domain name
> +func LookupNS(name string) (mx []*NS, err error) {
> +       return lookupNS(name)
> +}
> +
>  // LookupTXT returns the DNS TXT records for the given domain name.
>  func LookupTXT(name string) (txt []string, err error) {
>         return lookupTXT(name)
> Index: src/pkg/net/lookup_test.go
> ===================================================================
> --- a/src/pkg/net/lookup_test.go
> +++ b/src/pkg/net/lookup_test.go
> @@ -52,6 +52,20 @@
>         }
>  }
>
> +func TestGmailNS(t *testing.T) {
> +       if testing.Short() || !*testExternal {
> +               t.Logf("skipping test to avoid external network")
> +               return
> +       }
> +       mx, err := LookupMX("gmail.com")
> +       if err != nil {
> +               t.Errorf("failed: %s", err)
> +       }
> +       if len(mx) == 0 {
> +               t.Errorf("no results")
> +       }
> +}
> +
>  func TestGmailTXT(t *testing.T) {
>         if testing.Short() || !*testExternal {
>                 t.Logf("skipping test to avoid external network")
> Index: src/pkg/net/lookup_unix.go
> ===================================================================
> --- a/src/pkg/net/lookup_unix.go
> +++ b/src/pkg/net/lookup_unix.go
> @@ -119,6 +119,19 @@
>         return
>  }
>
> +func lookupNS(name string) (ns []*NS, err error) {
> +       _, records, err := lookup(name, dnsTypeNS)
> +       if err != nil {
> +               return
> +       }
> +       ns = make([]*NS, len(records))
> +       for i, r := range records {
> +               r := r.(*dnsRR_NS)
> +               ns[i] = &NS{r.Ns}
> +       }
> +       return
> +}
> +
>  func lookupTXT(name string) (txt []string, err error) {
>         _, records, err := lookup(name, dnsTypeTXT)
>         if err != nil {
>
>

[minux1, 2012-10-13 08:03:06]

https://codereview.appspot.com/6675043/diff/5001/src/pkg/net/dnsclient.go
File src/pkg/net/dnsclient.go (right):

https://codereview.appspot.com/6675043/diff/5001/src/pkg/net/dnsclient.go#new...
src/pkg/net/dnsclient.go:230: // An NS represents a single DNS NS record.
please move this struct to the end of file to keep definition of struct
MX close to type byPref and its methods.

https://codereview.appspot.com/6675043/diff/5001/src/pkg/net/lookup.go
File src/pkg/net/lookup.go (right):

https://codereview.appspot.com/6675043/diff/5001/src/pkg/net/lookup.go#newcode51
src/pkg/net/lookup.go:51: func LookupNS(name string) (mx []*NS, err error) {
s/mx/ns/

[smcquay, 2012-10-13 14:45:33]

Hello golang-dev@googlegroups.com, dave@cheney.net, minux.ma@gmail.com (cc:
golang-dev@googlegroups.com),

Please take another look.

[mikio, 2012-10-13 16:11:08]

please add lookupNS func for plan9, windows too.

https://codereview.appspot.com/6675043/diff/6005/src/pkg/net/lookup.go
File src/pkg/net/lookup.go (right):

https://codereview.appspot.com/6675043/diff/6005/src/pkg/net/lookup.go#newcode50
src/pkg/net/lookup.go:50: // LookupNS returns the DNS NS records for the given
domain name
full stop.

https://codereview.appspot.com/6675043/diff/6005/src/pkg/net/lookup_test.go
File src/pkg/net/lookup_test.go (right):

https://codereview.appspot.com/6675043/diff/6005/src/pkg/net/lookup_test.go#n...
src/pkg/net/lookup_test.go:60: mx, err := LookupMX("gmail.com")
LookupNS?

[smcquay, 2012-10-14 02:26:10]

Hello golang-dev@googlegroups.com, dave@cheney.net, minux.ma@gmail.com,
mikioh.mikioh@gmail.com (cc: golang-dev@googlegroups.com),

Please take another look.

[smcquay, 2012-10-14 02:30:09]

On 2012/10/13 16:11:08, mikio wrote:
> please add lookupNS func for plan9, windows too.

I did my best to implement the func for plan9 and windows, but don't have access
to either os to test things.

What do people usually do in my circumstance?

[smcquay, 2012-10-14 02:40:52]

Hello golang-dev@googlegroups.com, dave@cheney.net, minux.ma@gmail.com,
mikioh.mikioh@gmail.com (cc: golang-dev@googlegroups.com),

Please take another look.

[dfc, 2012-10-14 02:46:16]

Lucio and Alex/Mikio can help with plan9 / windows respectively.

Please correct the issue fixed in the description (hg change).

On Sun, Oct 14, 2012 at 1:40 PM,  <stephen@mcquay.me> wrote:
> Hello golang-dev@googlegroups.com, dave@cheney.net, minux.ma@gmail.com,
> mikioh.mikioh@gmail.com (cc: golang-dev@googlegroups.com),
>
> Please take another look.
>
>
> http://codereview.appspot.com/6675043/

[smcquay, 2012-10-14 03:03:54]

On Sun, Oct 14, 2012 at 01:46:15PM +1100, Dave Cheney wrote:
>Lucio and Alex/Mikio can help with plan9 / windows respectively.

If it helps, I have been using the following command to test things:

package main

import (
	"fmt"
	"net"
	"os"
)

const usage = "main <hostname>\n"

func main() {
	if len(os.Args) != 2 {
		fmt.Fprintf(os.Stderr, usage)
		os.Exit(1)
	}
	n, _ := net.LookupNS(os.Args[1])
	for i, v := range n {
		fmt.Printf("%v %v %v\n", i, v, v.Host)
	}
}

And I compare with the output for dig against gmail.com, mcquay.me, yahoo.com.

Thanks,

-- 
Stephen M. McQuay
http://mcquay.me/vcf

[minux1, 2012-10-14 16:47:15]

You'd better simplify the description a bit, i suggest this:
net: add LookupNS(domain string)
Fixes issue 4224.

For windows, you need to do more, as syscall doesn't define
syscall.DNSNSData, you need to use syscall.DNSPTRData directly.
Take lookupCNAME as an example.

[smcquay, 2012-10-14 20:28:51]

https://codereview.appspot.com/6675043/diff/6005/src/pkg/net/lookup.go
File src/pkg/net/lookup.go (right):

https://codereview.appspot.com/6675043/diff/6005/src/pkg/net/lookup.go#newcode50
src/pkg/net/lookup.go:50: // LookupNS returns the DNS NS records for the given
domain name
On 2012/10/13 16:11:09, mikio wrote:
> full stop.

Addressed.

https://codereview.appspot.com/6675043/diff/6005/src/pkg/net/lookup_test.go
File src/pkg/net/lookup_test.go (right):

https://codereview.appspot.com/6675043/diff/6005/src/pkg/net/lookup_test.go#n...
src/pkg/net/lookup_test.go:60: mx, err := LookupMX("gmail.com")
On 2012/10/13 16:11:09, mikio wrote:
> LookupNS?

Good point. Updated.

[smcquay, 2012-10-14 20:32:52]

On 2012/10/14 16:47:15, minux wrote:
> For windows, you need to do more, as syscall doesn't define
> syscall.DNSNSData, you need to use syscall.DNSPTRData directly.
> Take lookupCNAME as an example.

I tried to bring parity with lookupCNAME. From my inspection of DNSPTRData and
DnsQuery it seems I may be closer.

[brainman, 2012-10-15 03:24:37]

LGTM

Nicely done. Thank you.

You are creating some new public names. Lets leave it for a few days to give Go
team members chance to comment.

You need to sign the agreement http://golang.org/doc/contribute.html#copyright
before you change can be submitted.

Alex

https://codereview.appspot.com/6675043/diff/11007/src/pkg/net/lookup_test.go
File src/pkg/net/lookup_test.go (right):

https://codereview.appspot.com/6675043/diff/11007/src/pkg/net/lookup_test.go#...
src/pkg/net/lookup_test.go:60: mx, err := LookupNS("gmail.com")
s/mx/ns/

Same everywhere.

[smcquay, 2012-10-15 04:04:33]

On Mon, Oct 15, 2012 at 03:24:37AM +0000, alex.brainman@gmail.com wrote:
>LGTM
>
>Nicely done. Thank you.

Thanks!


>You are creating some new public names. Lets leave it for a few days to
>give Go team members chance to comment.

Sounds reasonable.


>You need to sign the agreement 
>http://golang.org/doc/contribute.html#copyright before you change can 
>be submitted.

I believe I've filled it out before, but just did for good measure. How 
can I figure out if it's been recorded?

Thanks, Alex,


-- 
Stephen M. McQuay
http://mcquay.me/vcf

[brainman, 2012-10-15 04:08:00]

On 2012/10/15 04:04:33, smcquay wrote:
> ...
> I believe I've filled it out before, but just did for good measure. How 
> can I figure out if it's been recorded?
> 

I do not see these myself. Only golang team gophers do. But, do not worry, we
will find your record once we are ready to submit.

Alex

[smcquay, 2012-10-15 04:47:54]

https://codereview.appspot.com/6675043/diff/11007/src/pkg/net/lookup_test.go
File src/pkg/net/lookup_test.go (right):

https://codereview.appspot.com/6675043/diff/11007/src/pkg/net/lookup_test.go#...
src/pkg/net/lookup_test.go:60: mx, err := LookupNS("gmail.com")
On 2012/10/15 03:24:37, brainman wrote:
> s/mx/ns/
> 
> Same everywhere.

Addressed.

[rsc, 2012-10-15 17:43:20]

Sorry for being late to this game, but why is this useful?

LookupMX returns an address you can use with smtp.Dial.
LookupNS returns an address that can't be used by any standard API that I'm
aware of.

[smcquay, 2012-10-15 18:25:00]

rsc --

On Mon, Oct 15, 2012 at 05:43:21PM +0000, rsc@golang.org wrote:
>Sorry for being late to this game, but why is this useful?

You'll have to forgive my enthusiasm; I saw an issue (4224) that 
I thought I could knock out and hacked it up before I put much thought 
into the 'why useful'.


>LookupMX returns an address you can use with smtp.Dial.
>LookupNS returns an address that can't be used by any standard API that
>I'm aware of.

I found in my tests that the Host field of the NS struct can be used in 
calls to Lookup*, for what it's worth. Also, when I've needed NS info in 
the past, I run dig and the string hostname is typically what I'm after.

I am guessing that the reporter had reason to post, and would be happy 
to follow up in the issue's comments.

Respectfully,

-- 
Stephen M. McQuay
http://mcquay.me/vcf

[hgfischer, 2012-10-15 23:38:36]

On 2012/10/15 18:25:00, smcquay wrote:

> I am guessing that the reporter had reason to post, and would be happy 
> to follow up in the issue's comments.

Hi,

I am the reporter of the issue 4224.

I issued this when starting to develop something that depends on checking the
DNS status headers of a domain (NOERROR, NXDOMAIN, etc) by querying it's NS.
Something able to get information like the "dig" command does.

 I've ended up using a 3rd party lib for now, but I think some of the DNS
internal functions in Go's net pkg could solve my problem if they were exported.

IMHO I should note that I net.Lookup* functions is a strange mix of simple
basic/general resolv/DNS functions (LookupAddr, LookupHost, LookupIP) and more
DNS specific functions (LookupCNAME, ...MX, ...SRV, ...TXT), but this set of
specific functions is incomplete. Maybe Go should have a net/dns package with
these features in it, maybe not. Maybe Go could have some way to return DNS
headers of each query. I'm not sure what is the best approach.

Thank you all!

[rsc, 2012-10-16 17:39:17]

LGTM

Thanks for the explanation. Leaving for Mikio.

[mikio, 2012-10-18 00:42:49]

https://codereview.appspot.com/6675043/diff/12022/src/pkg/net/lookup_test.go
File src/pkg/net/lookup_test.go (right):

https://codereview.appspot.com/6675043/diff/12022/src/pkg/net/lookup_test.go#...
src/pkg/net/lookup_test.go:60: mx, err := LookupNS("gmail.com")
s/mx/ns/

https://codereview.appspot.com/6675043/diff/12022/src/pkg/net/lookup_test.go#...
src/pkg/net/lookup_test.go:64: if len(mx) == 0 {
s/mx/ns/

[smcquay, 2012-10-18 03:06:09]

Sorry, I should've caught these; changed.

[mikio, 2012-10-18 06:38:47]

LGTM

[mikio, 2012-10-18 06:39:26]

*** Submitted as http://code.google.com/p/go/source/detail?r=eb854c88d4a6 ***

net: add LookupNS(domain string)
Fixes issue 4224.

R=golang-dev, dave, minux.ma, mikioh.mikioh, alex.brainman, rsc, herbert.fischer
CC=golang-dev
http://codereview.appspot.com/6675043

Committer: Mikio Hara <mikioh.mikioh@gmail.com>