Question compression

[tmthrgd]

The following new test case fails:

func TestMsgCompressionMultipleQuestions(t *testing.T) {
	msg := new(Msg)
	msg.Compress = true
	msg.SetQuestion("www.example.org.", TypeA)
	msg.Question = append(msg.Question, Question{"other.example.org.", TypeA, ClassINET})

	predicted := msg.Len()
	buf, err := msg.Pack()
	if err != nil {
		t.Error(err)
	}

	if predicted != len(buf) {
		t.Fatalf("predicted compressed length is wrong: predicted %d, actual %d", predicted, len(buf))
	}
}

with:

--- FAIL: TestMsgCompressionMultipleQuestions (0.00s)
    length_test.go:387: predicted compressed length is wrong: predicted 56, actual 45

This fails because compressedLen doesn't compress any names in dns.Question:

dns/msg.go

Lines 909 to 912 in 1c9c9bf

	for _, r := range dns.Question {
	compressionLenHelper(compression, r.Name, l)
	l += r.len()
	}

but the packing code does compress names in Question:

dns/msg.go

Lines 1093 to 1094 in 1c9c9bf

	func (q *Question) pack(msg []byte, off int, compression map[string]int, compress bool) (int, error) {
	off, err := PackDomainName(q.Name, msg, off, compression, compress)

dns/msg.go

Line 750 in 1c9c9bf

off, err = question[i].pack(msg, off, compression, dns.Compress)

Although I know that multiple questions are very rare and largely unsupported, this is still a bug. It's very easy to fix, but I just don't know what the correct behaviour is here.

Can QNAMEs be compressed and if so, do we actually want to compress them? I can't seem to find an answer in the RFCs, but I suspect we do want to skip compressing them either way.

/cc @miekg

[tmthrgd]

@miekg Back in 2012 you added 13f1f5b titled "Question is compressible IMO".

[miekg]

Well, yes and no. Compression *should* only use forward pointers and question is the first thing you see, so it can't be compressed. However backward pointers are seen in the wild, so it can be compressed (just like any other name). Every name in the binary message can have a compression pointer; should *we* compress a question; most likely not. Can the question name be used as a compression target: yes.

[tmthrgd]

But in the case of multiple questions, a reference from one question to a previous one can occur and it breaks the test case above because Len and Pack handle it differently.

The only resource I can find that has anything explicit, says that QNAMEs can't be compressed, but it was hardly an authoritative resource. The RFCs arn't clear and multiple questions are rare.

I understand they can be used as a target, this is specifically about using pointers within QNAMEs.

[miekg]

Qnames are no different than other names so they can be compressed.

…

On Tue, 27 Nov 2018, 10:54 Tom Thorogood ***@***.*** wrote: But in the case of multiple questions, a reference from one question to a previous one can occur and it breaks the test case above because Len and Pack handle it differently. The only resource I can find that has anything explicit, says that QNAMEs can't be compressed, but it was hardly an authoritative resource. The RFCs arn't clear and multiple questions are rare. I understand they can be used as a target, this is specifically about using pointers within QNAMEs. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#821 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAVkW_jrJZAfTNUrzvS8VxaAqgTR20G8ks5uzRn8gaJpZM4Y0SLb> .

[tmthrgd]

Ack, I'll fix the bug in compressedLen instead then.

[miekg]

been answered.

[tmthrgd]

The test case still fails. #833 will fix and close this. I'll reopen until then.