- rewrites: fix invalid question in response

#1746

for a rule:

cname -> domain

with IP of 'domain' resolved by upstream

Squashed commit of the following:

commit fb3ad25ac8a7963a8fa6587cdc33e2fc35f79dfb
Merge: a6794989 67dacdf8
Author: Simon Zolin <s.zolin@adguard.com>
Date:   Mon Jun 1 15:15:23 2020 +0300

    Merge remote-tracking branch 'origin/master' into 1746-rewrites

commit a679498904a817011b55c58ee579e55f27fa0bc8
Author: Simon Zolin <s.zolin@adguard.com>
Date:   Mon Jun 1 10:42:16 2020 +0300

    test: check question

commit 7491e753c5eb6df54c9c050b0fa198c81dded2a0
Author: Simon Zolin <s.zolin@adguard.com>
Date:   Fri May 29 18:22:32 2020 +0300

    test

commit 12cb2e194191ca489c9025b55f8571ae2dd7c33d
Author: Simon Zolin <s.zolin@adguard.com>
Date:   Fri May 29 14:51:06 2020 +0300

    - rewrites: fix invalid question in response

    for a rule:

    cname -> domain

    with IP of 'domain' resolved by upstream

dnsforward/dnsforward_test.go

@@ -627,6 +627,66 @@ func TestBlockedBySafeBrowsing(t *testing.T) {
 	}
 }
 
+func TestRewrite(t *testing.T) {
+	c := dnsfilter.Config{}
+	c.Rewrites = []dnsfilter.RewriteEntry{
+		dnsfilter.RewriteEntry{
+			Domain: "test.com",
+			Answer: "1.2.3.4",
+			Type:   dns.TypeA,
+		},
+		dnsfilter.RewriteEntry{
+			Domain: "alias.test.com",
+			Answer: "test.com",
+			Type:   dns.TypeCNAME,
+		},
+		dnsfilter.RewriteEntry{
+			Domain: "my.alias.example.org",
+			Answer: "example.org",
+			Type:   dns.TypeCNAME,
+		},
+	}
+
+	f := dnsfilter.New(&c, nil)
+	s := NewServer(f, nil, nil)
+	conf := ServerConfig{}
+	conf.UDPListenAddr = &net.UDPAddr{Port: 0}
+	conf.TCPListenAddr = &net.TCPAddr{Port: 0}
+	conf.ProtectionEnabled = true
+	conf.UpstreamDNS = []string{"8.8.8.8:53"}
+
+	err := s.Prepare(&conf)
+	assert.Nil(t, err)
+	err = s.Start()
+	assert.Nil(t, err)
+	addr := s.dnsProxy.Addr(proxy.ProtoUDP)
+
+	req := createTestMessageWithType("test.com.", dns.TypeA)
+	reply, err := dns.Exchange(req, addr.String())
+	assert.Nil(t, err)
+	assert.Equal(t, 1, len(reply.Answer))
+	a, ok := reply.Answer[0].(*dns.A)
+	assert.True(t, ok)
+	assert.Equal(t, "1.2.3.4", a.A.String())
+
+	req = createTestMessageWithType("alias.test.com.", dns.TypeA)
+	reply, err = dns.Exchange(req, addr.String())
+	assert.Nil(t, err)
+	assert.Equal(t, 2, len(reply.Answer))
+	assert.Equal(t, "test.com.", reply.Answer[0].(*dns.CNAME).Target)
+	assert.Equal(t, "1.2.3.4", reply.Answer[1].(*dns.A).A.String())
+
+	req = createTestMessageWithType("my.alias.example.org.", dns.TypeA)
+	reply, err = dns.Exchange(req, addr.String())
+	assert.Nil(t, err)
+	assert.Equal(t, "my.alias.example.org.", reply.Question[0].Name) // the original question is restored
+	assert.Equal(t, 2, len(reply.Answer))
+	assert.Equal(t, "example.org.", reply.Answer[0].(*dns.CNAME).Target)
+	assert.Equal(t, dns.TypeA, reply.Answer[1].Header().Rrtype)
+
+	_ = s.Stop()
+}
+
 func createTestServer(t *testing.T) *Server {
 	rules := `||nxdomain.example.org
 ||null.example.org^

dnsforward/handle_dns.go

@@ -211,12 +211,11 @@ func processFilteringAfterResponse(ctx *dnsContext) int {
 
 	switch res.Reason {
 	case dnsfilter.ReasonRewrite:
-		if d.Res != nil {
+		if len(ctx.origQuestion.Name) == 0 {
-			break // response is already prepared
+			// origQuestion is set in case we get only CNAME without IP from rewrites table
-		}
-		if len(res.CanonName) == 0 {
 			break
 		}
+
 		d.Req.Question[0] = ctx.origQuestion
 		d.Res.Question[0] = ctx.origQuestion