String() wrappers for DNS class and type values

[edmonds]

it would be nice to have wrappers for the TypeToString and ClassToString maps that return the mnemonic if known, or RFC 3597 compliant generic representations otherwise. something like this:

import "strconv"

type RRType uint16
type RRClass uint16

func (t RRType) String() string {
    if s, ok := dns.TypeToString[uint16(t)]; ok {
        return s
    } else {
        return "TYPE" + strconv.Itoa(int(t))
    }
}

func (c RRClass) String() string {
    if s, ok := dns.ClassToString[uint16(c)]; ok {
        return s
    } else {
        return "CLASS" + strconv.Itoa(int(c))
    }
}

so that one could simply format integer class and type values with e.g.

fmt.Println("the rrclass is", dns.RRClass(rrclass))
fmt.Println("the rrtype is", dns.RRType(rrtype))

instead of the ten or so lines it would take otherwise.

[miekg]

[ Quoting notifications@github.com in "[dns] String() wrappers for DNS cla..." ]

it would be nice to have wrappers for the TypeToString and ClassToString maps that return the mnemonic if known, or RFC 3597 compliant generic representations otherwise. something like this:

so that one could simply format integer class and type values with e.g.

fmt.Println("the rrclass is", dns.RRClass(rrclass))
fmt.Println("the rrtype is", dns.RRType(rrtype))

instead of the ten or so lines it would take otherwise.

I understand your use case, but how often is this needed? The reason
ClassToString and TypeToString are exported is that you can do this by
yourself.

I hesitant to add, just because it's so easy to write yourself, unless
it is need very, very often.

grtz Miek

[edmonds]

hi, miek:

yes, i know i can do it myself, but i don't want to :)

i can say that i write code that converts to and from "presentation format" fairly often and it would be very useful to be able to treat integer RRTypes and strings representing RRTypes without having to worry about whether they are defined or not. having the bare maps feels very "raw". would you be willing to accept a pull request with this functionality (+ full tests, of course)?

also, it would probably be nice to have wrappers in the other direction, e.g. convert "aaaa" or "AAAA" or "TYPE28" to uint16(28).

i know that RFC 3597 strictly speaking applies only to DNS server software, but i think having this kind of functionality in "the go dns package" would make it more likely that "idiomatic" code gets written that can handle unknown RRTypes correctly, even when written by people who have never heard of RFC 3597 :)

[miekg]

[ Quoting notifications@github.com in "Re: [dns] String() wrappers for DNS..." ]

hi, miek:

yes, i know i can do it myself, but i don't want to :)

Hehe :-) Well I'm sympathetic with your needs, but I always want to keep
Go DNS' API small. Adding an RRType and RRClass as (exported) aliases
for uint16 doesn't sit well with me. (I.e.: it is an good idea, but then
I should change all instances of uint16 in Go dns to the correct type,
and that is a bit much.

i can say that i write code that converts to and from "presentation format" fairly often and it would be very useful to be able to treat integer RRTypes and strings representing RRTypes without having to worry about whether they are defined or not. having the bare maps feels very "raw". would you be willing to accept a pull request with this functionality (+ full tests, of course)?

Still hesitant. I think (maybe) some new helper function, without a new
type might work better? And yes, pull request with running code are
preferred.

also, it would probably be nice to have wrappers in the other direction, e.g. convert "aaaa" or "AAAA" or "TYPE28" to uint16(28).

Ack, above StringToType you mean?

i know that RFC 3597 strictly speaking applies only to DNS server software, but i think having this kind of functionality in "the go dns package" would make it more likely that "idiomatic" code gets written that can handle unknown RRTypes correctly, even when written by people who have never heard of RFC 3597 :)

Note that "www.example.com. IN TYPE1 #4 0a000001", currently is not
parsed by Go DNS. "www.example.com. IN TYPE1 10.0.0.1" is.

grtz Miek

[edmonds]

i know about keeping APIs small :) http://rsfcode.isc.org/git/wdns/tree/wdns/wdns.h has served me well for a number of years, but i'm interested in writing more go code.

i've only been writing go code for a short while, but i don't think a plain helper function is right. having a type that implements the Stringer interface would be very nice. e.g., i went looking for the go equivalent of inet_ntop() and couldn't find it until i realized that net.IP was a type alias for byte[], so i just had to write:

fmt.Println("queryAddress:", net.IP(queryAddress))

it would be quite nice to similarly write:

fmt.Println("queryType:", dns.RRType(queryType))

which conceptually seems a bit better than calling a helper function.

are you sure switching from bare uint16's to RRType/RRClass type aliases would be that bad? i guess it might be a big diff and probably breaks the API, but i know i have occasionally written buggy code that went undetected (for a very brief while, admittedly) due to CLASS IN / TYPE A having the same uint16 value. wouldn't having dedicated type aliases make it a compile time error to ever use an RRType value where an RRClass value was expected or vice versa?

[miekg]

[ Quoting notifications@github.com in "Re: [dns] String() wrappers for DNS..." ]

i know about keeping APIs small :) http://rsfcode.isc.org/git/wdns/tree/wdns/wdns.h has served me well for a number of years, but i'm interested in writing more go code.

That is indeed a small api ;-)

i've only been writing go code for a short while, but i don't think a plain helper function is right. having a type that implements the Stringer interface would be very nice. e.g., i went looking for the go equivalent of inet_ntop() and couldn't find it until i realized that net.IP was a type alias for byte[], so i just had to write:

fmt.Println("queryAddress:", net.IP(queryAddress))

it would be quite nice to similarly write:

fmt.Println("queryType:", dns.RRType(queryType))

which conceptually seems a bit better than calling a helper function.

yes and yes, but you are the first with this specific requirement, hence
me pushing back a little.

are you sure switching from bare uint16's to RRType/RRClass type aliases would be that bad? i guess it might be a big diff and probably breaks the API, but i know i have occasionally written buggy code that went undetected (for a very brief while, admittedly) due to CLASS IN / TYPE A having the same uint16 value. wouldn't having dedicated type aliases make it a compile time error to ever use an RRType value where an RRClass value was expected or vice versa?

breaking the api just for this seems a bit much atm. I agree this is a
good idea, but might have to wait for a v2 version of go dns.

To turn the thing around: is it an idea to maintain this a go dns++
package and see how that fares? I have no problem updating the docs
to point to such a package. Stuff can then be promoted/demoted as we see
fit?

• Grtz,

  ---

  Miek Gieben

[edmonds]

Miek Gieben wrote:

[ Quoting notifications@github.com in "Re: [dns] String() wrappers for DNS..." ]

i've only been writing go code for a short while, but i don't think a plain helper function is right. having a type that implements the Stringer interface would be very nice. e.g., i went looking for the go equivalent of inet_ntop() and couldn't find it until i realized that net.IP was a type alias for byte[], so i just had to write:

fmt.Println("queryAddress:", net.IP(queryAddress))

it would be quite nice to similarly write:

fmt.Println("queryType:", dns.RRType(queryType))

which conceptually seems a bit better than calling a helper function.

yes and yes, but you are the first with this specific requirement, hence
me pushing back a little.

ok, maybe there are people who don't realize they need this, though. if
you write:

var uint16 t = 1
fmt.Println("the type is", dns.TypeToString[t])

this works ok and prints "the type is A". but there is a subtle bug,
if you write:

var uint16 t = 12345
fmt.Println("the type is", dns.TypeToString[t])

this will just print out "the type is". so you almost always need to
use the "comma ok" idiom when getting values from the TypeToString map
in order to use it correctly, which means 5+ lines of code. but now you
have to decide what to do here:

var uint16 = 12345
if _, ok := dns.TypeToString[t]; ok {
    fmt.Println("the type is", dns.TypeToString[t])
} else {
    // XXX: what to do here?
}

do you log an error and exit, or what? maybe you get lucky and copy an
RFC 3597 conforming snippet of code if you don't know about it :) not
to belabor the point, but i think most users ought to be using
TypeToString etc. through a wrapper function or the Stringer interface
rather than accessing the map directly. (having access to the bare map
is still useful in some cases, e.g. finding out which rrtypes the
library knows about.)

are you sure switching from bare uint16's to RRType/RRClass type aliases would be that bad? i guess it might be a big diff and probably breaks the API, but i know i have occasionally written buggy code that went undetected (for a very brief while, admittedly) due to CLASS IN / TYPE A having the same uint16 value. wouldn't having dedicated type aliases make it a compile time error to ever use an RRType value where an RRClass value was expected or vice versa?

breaking the api just for this seems a bit much atm. I agree this is a
good idea, but might have to wait for a v2 version of go dns.

yeah, better to queue up all the API breaking changes :)

To turn the thing around: is it an idea to maintain this a go dns++
package and see how that fares? I have no problem updating the docs
to point to such a package. Stuff can then be promoted/demoted as we see
fit?

i guess i might fork miekg/dns and put this on a branch so it can be
merged easily.

Robert Edmonds

[miekg]

[ Quoting notifications@github.com in "Re: [dns] String() wrappers for DNS..." ]

ok, maybe there are people who don't realize they need this, though. if
you write:

var uint16 t = 1
fmt.Println("the type is", dns.TypeToString[t])

this works ok and prints "the type is A". but there is a subtle bug,
if you write:

var uint16 t = 12345
fmt.Println("the type is", dns.TypeToString[t])

this will just print out "the type is". so you almost always need to
use the "comma ok" idiom when getting values from the TypeToString map
in order to use it correctly, which means 5+ lines of code. but now you
have to decide what to do here:

var uint16 = 12345
if _, ok := dns.TypeToString[t]; ok {
    fmt.Println("the type is", dns.TypeToString[t])
} else {
    // XXX: what to do here?
}

Sorry to quote this from the README:

It follows a lean and mean philosophy. If there is stuff you should know as a   
DNS programmer there isn't a convenience function for it.                       

So "// XXX: what to do here?" should already be answered in the
programmers mind.

i guess i might fork miekg/dns and put this on a branch so it can be
merged easily.

That is always possible :-)

So to be a bit formal, I'm going to NACK this for now. But I can always
change my mind of course.

grtz Miek

[edmonds]

Miek Gieben wrote:

[ Quoting notifications@github.com in "Re: [dns] String() wrappers for DNS..." ]

ok, maybe there are people who don't realize they need this, though. if
you write:

var uint16 t = 1
fmt.Println("the type is", dns.TypeToString[t])

this works ok and prints "the type is A". but there is a subtle bug,
if you write:

var uint16 t = 12345
fmt.Println("the type is", dns.TypeToString[t])

this will just print out "the type is". so you almost always need to
use the "comma ok" idiom when getting values from the TypeToString map
in order to use it correctly, which means 5+ lines of code. but now you
have to decide what to do here:

var uint16 = 12345
if _, ok := dns.TypeToString[t]; ok {
    fmt.Println("the type is", dns.TypeToString[t])
} else {
    // XXX: what to do here?
}

Sorry to quote this from the README:

It follows a lean and mean philosophy. If there is stuff you should know as a   
DNS programmer there isn't a convenience function for it.                       

this is a bit silly. the README says "lean and mean" but also "complete
and usable". which is it? :)

So "// XXX: what to do here?" should already be answered in the
programmers mind.

ahem. i guess you should fix the example code then. :)

edmonds@chase{0}:~/.go/src/github.com/miekg/dns/ex/q$ grep -r -C2 TypeToString
q.go-// shorten RRSIG to "miek.nl RRSIG(NS)"
q.go-func shortSig(sig *dns.RRSIG) string {
q.go:   return sig.Header().Name + " RRSIG(" + dns.TypeToString[sig.TypeCovered] + ")"
q.go-}
q.go-

edmonds@chase{0}:~/.go/src/github.com/miekg/dns/ex/q$ go build q.go           

edmonds@chase{0}:~/.go/src/github.com/miekg/dns/ex/q$ ./q rrsig rrsig-test.mycre.ws
;; opcode: QUERY, status: NOERROR, id: 12506
;; flags: qr rd ra; QUERY: 1, ANSWER: 1, AUTHORITY: 3, ADDITIONAL: 0

;; QUESTION SECTION:
;rrsig-test.mycre.ws.   IN   RRSIG

;; ANSWER SECTION:
rrsig-test.mycre.ws.    3241    IN  RRSIG    5 3 60 20130707010441 20130607010441 1234 mycre.ws. WhO3fSroNQ7k8fmRT+4niI8lgsJizT8BxzEfZklKXmVxuMuDjK8NVUDsPDlH5AY6hw1cCJ0KOE7ebNweRmRP5mXC+3PoZAos8pYW6GNajyXBRhK2NJoxtosB3yRMlo7mOZ+SjmH/SSzVw8u1WMJT7WfCse8BK36kkU1kO5afi88=

;; AUTHORITY SECTION:
mycre.ws.   3149    IN  NS  sfba.sns-pb.isc.org.
mycre.ws.   3149    IN  NS  ams.sns-pb.isc.org.
mycre.ws.   3149    IN  NS  ord.sns-pb.isc.org.

;; query time: 336 µs, server: 127.0.0.1:53(udp), size: 346 bytes

edmonds@chase{0}:~/.go/src/github.com/miekg/dns/ex/q$ dig +short rrsig rrsig-test.mycre.ws
TYPE12345 5 3 60 20130707010441 20130607010441 1234 mycre.ws. WhO3fSroNQ7k8fmRT+4niI8lgsJizT8BxzEfZklKXmVxuMuDjK8NVUDs PDlH5AY6hw1cCJ0KOE7ebNweRmRP5mXC+3PoZAos8pYW6GNajyXBRhK2 NJoxtosB3yRMlo7mOZ+SjmH/SSzVw8u1WMJT7WfCse8BK36kkU1kO5af i88=

Robert Edmonds

[edmonds]

oh, that did not format well. note the RRSIG "Type Covered" field is missing from the formatted output.

edmonds@chase{0}:~/.go/src/github.com/miekg/dns/ex/q$ grep -r -C2 TypeToString
q.go-// shorten RRSIG to "miek.nl RRSIG(NS)"
q.go-func shortSig(sig *dns.RRSIG) string {
q.go:   return sig.Header().Name + " RRSIG(" + dns.TypeToString[sig.TypeCovered] + ")"
q.go-}
q.go-

edmonds@chase{0}:~/.go/src/github.com/miekg/dns/ex/q$ go build q.go

edmonds@chase{0}:~/.go/src/github.com/miekg/dns/ex/q$ ./q rrsig rrsig-test.mycre.ws
;; opcode: QUERY, status: NOERROR, id: 12506
;; flags: qr rd ra; QUERY: 1, ANSWER: 1, AUTHORITY: 3, ADDITIONAL: 0

;; QUESTION SECTION:
;rrsig-test.mycre.ws.   IN   RRSIG

;; ANSWER SECTION:
rrsig-test.mycre.ws.    3241    IN  RRSIG    5 3 60 20130707010441 20130607010441 1234 mycre.ws. WhO3fSroNQ7k8fmRT+4niI8lgsJizT8BxzEfZklKXmVxuMuDjK8NVUDsPDlH5AY6hw1cCJ0KOE7ebNweRmRP5mXC+3PoZAos8pYW6GNajyXBRhK2NJoxtosB3yRMlo7mOZ+SjmH/SSzVw8u1WMJT7WfCse8BK36kkU1kO5afi88=

;; AUTHORITY SECTION:
mycre.ws.   3149    IN  NS  sfba.sns-pb.isc.org.
mycre.ws.   3149    IN  NS  ams.sns-pb.isc.org.
mycre.ws.   3149    IN  NS  ord.sns-pb.isc.org.

;; query time: 336 µs, server: 127.0.0.1:53(udp), size: 346 bytes

edmonds@chase{0}:~/.go/src/github.com/miekg/dns/ex/q$ dig +short rrsig rrsig-test.mycre.ws
TYPE12345 5 3 60 20130707010441 20130607010441 1234 mycre.ws. WhO3fSroNQ7k8fmRT+4niI8lgsJizT8BxzEfZklKXmVxuMuDjK8NVUDs PDlH5AY6hw1cCJ0KOE7ebNweRmRP5mXC+3PoZAos8pYW6GNajyXBRhK2 NJoxtosB3yRMlo7mOZ+SjmH/SSzVw8u1WMJT7WfCse8BK36kkU1kO5af i88=

[miekg]

[ Quoting notifications@github.com in "Re: [dns] String() wrappers for DNS..." ]

Sorry to quote this from the README:

It follows a lean and mean philosophy. If there is stuff you should know as a   
DNS programmer there isn't a convenience function for it.                       

this is a bit silly. the README says "lean and mean" but also "complete
and usable". which is it? :)

So "// XXX: what to do here?" should already be answered in the
programmers mind.

ahem. i guess you should fix the example code then. :)

Ah, yes. Point taken :-)

Let me see where fixing this takes me and what convience function should
(if any) be added.

The new types being aliases of uint16 might be handy enough to make the
change in the entire library and not effect code that handles these
things as uint16s.

Note, at the time I started ldns a lot of convience functions got
added, most moved from drill to ldns. Fast-forward a couple of years and
ldns' API is now large and lots of stuff in underdocumented. I don't
wont that to happen to go dns.

• Grtz,

  ---

  Miek Gieben

[miekg]

[ Quoting notifications@github.com in "Re: [dns] String() wrappers for DNS..." ]

oh, that did not format well.

TYPE12345 5 3 60 20130707010441 20130607010441 1234 mycre.ws. WhO3fSroNQ7k8fmRT+4niI8lgsJizT8BxzEfZklKXmVxuMuDjK8NVUDs PDlH5AY6hw1cCJ0KOE7ebNweRmRP5mXC+3PoZAos8pYW6GNajyXBRhK2 NJoxtosB3yRMlo7mOZ+SjmH/SSzVw8u1WMJT7WfCse8BK36kkU1kO5af i88=

How a little bit of programming can make you see the light :)

b35306b

• Grtz,

  ---

  Miek Gieben