msg.Len() disagrees with Pack()

[gpaul]

Here's a test that fails:

func TestMsgCompressLength2(t *testing.T) {
	msg := new(Msg)
	msg.Compress = true
	msg.SetQuestion(Fqdn("_foo._tcp.bar."), TypeANY)
	msg.Answer = append(msg.Answer, &SRV{Hdr: RR_Header{Name: "_foo._tcp.bar.", Rrtype: 0x21, Class: 0x1, Ttl: 0x3c}, Port: 0x4c57, Target: "foo.bar."})
	msg.Answer = append(msg.Answer, &SRV{Hdr: RR_Header{Name: "_foo._tcp.bar.", Rrtype: 0x21, Class: 0x1, Ttl: 0x3c}, Port: 0x3307, Target: "foo.bar."})
	msg.Answer = append(msg.Answer, &SRV{Hdr: RR_Header{Name: "_foo._tcp.bar.", Rrtype: 0x21, Class: 0x1, Ttl: 0x3c}, Port: 0x35b0, Target: "foo.bar."})
	msg.Answer = append(msg.Answer, &SRV{Hdr: RR_Header{Name: "_foo._tcp.bar.", Rrtype: 0x21, Class: 0x1, Ttl: 0x3c}, Port: 0x5a7a, Target: "foo.bar."})
	msg.Answer = append(msg.Answer, &SRV{Hdr: RR_Header{Name: "_foo._tcp.bar.", Rrtype: 0x21, Class: 0x1, Ttl: 0x3c}, Port: 0x211a, Target: "foo.bar."})
	msg.Extra = append(msg.Extra, &A{Hdr: RR_Header{Name: "foo.bar.", Rrtype: 0x1, Class: 0x1, Ttl: 0x3c}, A: net.IP{0xac, 0x11, 0x0, 0x3}})
	msg.Extra = append(msg.Extra, &A{Hdr: RR_Header{Name: "foo.bar.", Rrtype: 0x1, Class: 0x1, Ttl: 0x3c}, A: net.IP{0xac, 0x11, 0x0, 0x3}})
	msg.Extra = append(msg.Extra, &A{Hdr: RR_Header{Name: "foo.bar.", Rrtype: 0x1, Class: 0x1, Ttl: 0x3c}, A: net.IP{0xac, 0x11, 0x0, 0x3}})
	msg.Extra = append(msg.Extra, &A{Hdr: RR_Header{Name: "foo.bar.", Rrtype: 0x1, Class: 0x1, Ttl: 0x3c}, A: net.IP{0xac, 0x11, 0x0, 0x3}})
	msg.Extra = append(msg.Extra, &A{Hdr: RR_Header{Name: "foo.bar.", Rrtype: 0x1, Class: 0x1, Ttl: 0x3c}, A: net.IP{0xac, 0x11, 0x0, 0x3}})
	predicted := msg.Len()
	buf, err := msg.Pack()
	if err != nil {
		t.Error(err)
	}
	if predicted != len(buf) {
		t.Errorf("predicted compressed length is wrong: predicted %s (len=%d) %d, actual %d",
			msg.Question[0].Name, len(msg.Answer), predicted, len(buf))
	}
}

Results in

=== RUN   TestMsgCompressLength2
--- FAIL: TestMsgCompressLength2 (0.00s)
	dns_test.go:215: predicted compressed length is wrong: predicted _foo._tcp.bar. (len=5) 246, actual 250

[miekg]

Thanks for the test case.

This is tricky to get right as Len and Pack use 2 diff. algorithms, i.e. you don't want to Pack for every Len you do, and then throw away the buffer.

[gpaul]

Totally, this is why I posted a test case and didn't submit a PR :)

[gpaul]

Do you think you'll get around to this in the near future? If not I could have a look.

[miekg]

This is the problem: https://github.com/miekg/dns/blob/master/msg.go#L994

Didn't want to use reflection at the time, but now that we "go gen" the packs and unpacks this needs to be generated as well. Although looking through the list, SRV is in there

[miekg]

Can you make your example smaller?

[miekg]

func TestMsgCompressLength2(t *testing.T) {
	msg := new(Msg)
	msg.Compress = true
	msg.SetQuestion(Fqdn("_foo._tcp.bar."), TypeANY)
	msg.Answer = append(msg.Answer, &SRV{Hdr: RR_Header{Name: "_foo._tcp.bar.", Rrtype: 0x21, Class: 0x1, Ttl: 0x3c}, Port: 0x4c57, Target: "foo.bar."})
	msg.Extra = append(msg.Extra, &A{Hdr: RR_Header{Name: "foo.bar.", Rrtype: 0x1, Class: 0x1, Ttl: 0x3c}, A: net.IP{0xac, 0x11, 0x0, 0x3}})
	predicted := msg.Len()
	buf, err := msg.Pack()
	if err != nil {
		t.Error(err)
	}
	if predicted != len(buf) {
		t.Errorf("predicted compressed length is wrong: predicted %s (len=%d) %d, actual %d",
			msg.Question[0].Name, len(msg.Answer), predicted, len(buf))
	}
}

shows the problem as well.

[miekg]

compressionLenSearchType does not have SRV; i.e. yes we should gen these functions.

[miekg]

This is prolly also: skynetservices/skydns#313

[miekg]

#454 does not fix it, but is still good to have.

[miekg]

Updated that PR which fixes the issue. Does that work/help you?

[gpaul]

I'll have a look asap - thanks for the fast turn-around

[gpaul]

I confirm that this PR fixes the issue for me.

[miekg]

Fixed by #454