Address @urso review of DNS over TCP

* Use printf format in all debug messages
* Rename err in handleDecode
* Remove a useless pointer assignement and move another one into a if statement

packetbeat/protos/dns/dns.go

@@ -268,14 +268,14 @@ func (dns *Dns) ConnectionTimeout() time.Duration {
 }
 
 func (dns *Dns) receivedDnsRequest(tuple *DnsTuple, msg *DnsMessage) {
-	logp.Debug("dns", "Processing query. %s", tuple)
+	logp.Debug("dns", "Processing query. %s", tuple.String())
 
 	trans := dns.deleteTransaction(tuple.Hashable())
 	if trans != nil {
 		// This happens if a client puts multiple requests in flight
 		// with the same ID.
 		trans.Notes = append(trans.Notes, DuplicateQueryMsg.Error())
-		logp.Debug("dns", DuplicateQueryMsg.Error()+" %s", tuple)
+		logp.Debug("dns", "%s %s", DuplicateQueryMsg.Error(), tuple.String())
 		dns.publishTransaction(trans)
 		dns.deleteTransaction(trans.tuple.Hashable())
 	}
@@ -286,14 +286,14 @@ func (dns *Dns) receivedDnsRequest(tuple *DnsTuple, msg *DnsMessage) {
 }
 
 func (dns *Dns) receivedDnsResponse(tuple *DnsTuple, msg *DnsMessage) {
-	logp.Debug("dns", "Processing response. %s", tuple)
+	logp.Debug("dns", "Processing response. %s", tuple.String())
 
 	trans := dns.getTransaction(tuple.RevHashable())
 	if trans == nil {
 		trans = newTransaction(msg.Ts, tuple.Reverse(), common.CmdlineTuple{
 			Src: msg.CmdlineTuple.Dst, Dst: msg.CmdlineTuple.Src})
 		trans.Notes = append(trans.Notes, OrphanedResponse.Error())
-		logp.Debug("dns", OrphanedResponse.Error()+" %s", tuple)
+		logp.Debug("dns", "%s %s", OrphanedResponse.Error(), tuple.String())
 	}
 
 	trans.Response = msg
@@ -378,7 +378,7 @@ func (dns *Dns) publishTransaction(t *DnsTransaction) {
 
 func (dns *Dns) expireTransaction(t *DnsTransaction) {
 	t.Notes = append(t.Notes, NoResponse.Error())
-	logp.Debug("dns", NoResponse.Error()+" %s", t.tuple.String())
+	logp.Debug("dns", "%s %s", NoResponse.Error(), t.tuple.String())
 	dns.publishTransaction(t)
 }
 

packetbeat/protos/dns/dns_tcp.go

@@ -87,6 +87,7 @@ func (dns *Dns) doParse(conn *dnsConnectionData, pkt *protos.Packet, tcpTuple *c
 
 	if stream == nil {
 		stream = newStream(pkt, tcpTuple)
+		conn.Data[dir] = stream
 	} else {
 		if stream.message == nil { // nth message of the same stream
 			stream.message = &DnsMessage{Ts: pkt.Ts, Tuple: pkt.Tuple}
@@ -99,12 +100,9 @@ func (dns *Dns) doParse(conn *dnsConnectionData, pkt *protos.Packet, tcpTuple *c
 			return conn
 		}
 	}
-	conn.Data[dir] = stream
-	decodedData, err := conn.Data[dir].handleTcpRawData()
+	decodedData, err := stream.handleTcpRawData()
 
 	if err != nil {
-		logp.Debug("dns", err.Error()+" addresses %s, length %d",
-			tcpTuple.String(), len(stream.rawData))
 
 		if err == IncompleteMsg {
 			logp.Debug("dns", "Waiting for more raw data")
@@ -115,14 +113,17 @@ func (dns *Dns) doParse(conn *dnsConnectionData, pkt *protos.Packet, tcpTuple *c
 			dns.publishResponseError(conn, err)
 		}
 
+		logp.Debug("dns", "%s addresses %s, length %d", err.Error(),
+			tcpTuple.String(), len(stream.rawData))
+
 		// This means that malformed requests or responses are being sent...
 		// TODO: publish the situation also if Request
 		conn.Data[dir] = nil
 		return conn
 	}
 
 	dns.messageComplete(conn, tcpTuple, dir, decodedData)
-	conn.Data[dir].PrepareForNewMessage()
+	stream.PrepareForNewMessage()
 	return conn
 }
 
@@ -175,20 +176,20 @@ func (dns *Dns) ReceivedFin(tcpTuple *common.TcpTuple, dir uint8, private protos
 		return conn
 	}
 
-	decodedData, err := conn.Data[dir].handleTcpRawData()
+	decodedData, err := stream.handleTcpRawData()
 
 	if err == nil {
 		dns.messageComplete(conn, tcpTuple, dir, decodedData)
 		return conn
 	}
 
-	logp.Debug("dns", err.Error()+" addresses %s, length %d",
-		tcpTuple.String(), len(stream.rawData))
-
 	if dir == tcp.TcpDirectionReverse {
 		dns.publishResponseError(conn, err)
 	}
 
+	logp.Debug("dns", "%s addresses %s, length %d", err.Error(),
+		tcpTuple.String(), len(stream.rawData))
+
 	return conn
 }
 
@@ -206,7 +207,7 @@ func (dns *Dns) GapInStream(tcpTuple *common.TcpTuple, dir uint8, nbytes int, pr
 		return private, false
 	}
 
-	decodedData, err := conn.Data[dir].handleTcpRawData()
+	decodedData, err := stream.handleTcpRawData()
 
 	if err == nil {
 		dns.messageComplete(conn, tcpTuple, dir, decodedData)
@@ -217,9 +218,8 @@ func (dns *Dns) GapInStream(tcpTuple *common.TcpTuple, dir uint8, nbytes int, pr
 		dns.publishResponseError(conn, err)
 	}
 
-	logp.Debug("dns", err.Error()+" addresses %s, length %d",
+	logp.Debug("dns", "%s addresses %s, length %d", err.Error(),
 		tcpTuple.String(), len(stream.rawData))
-
 	logp.Debug("dns", "Dropping the stream %s", tcpTuple.String())
 
 	// drop the stream because it is binary Data and it would be unexpected to have a decodable message later
@@ -261,7 +261,7 @@ func (dns *Dns) publishResponseError(conn *dnsConnectionData, err error) {
 }
 
 // Manages data length prior to decoding the data and manages errors after decoding
-func (stream *DnsStream) handleTcpRawData() (dns *layers.DNS, err error) {
+func (stream *DnsStream) handleTcpRawData() (*layers.DNS, error) {
 	rawData := stream.rawData
 	messageLength := len(rawData)
 
@@ -292,10 +292,10 @@ func (stream *DnsStream) handleTcpRawData() (dns *layers.DNS, err error) {
 		return nil, IncompleteMsg
 	}
 
-	decodedData, err_ := decodeDnsData(TransportTcp, rawData[:stream.parseOffset])
+	decodedData, err := decodeDnsData(TransportTcp, rawData[:stream.parseOffset])
 
-	if err_ != nil {
-		return nil, err_
+	if err != nil {
+		return nil, err
 	}
 
 	return decodedData, nil

packetbeat/protos/dns/dns_udp.go

@@ -18,7 +18,7 @@ func (dns *Dns) ParseUdp(pkt *protos.Packet) {
 		// This means that malformed requests or responses are being sent or
 		// that someone is attempting to the DNS port for non-DNS traffic. Both
 		// are issues that a monitoring system should report.
-		logp.Debug("dns", err.Error())
+		logp.Debug("dns", "%s", err.Error())
 		return
 	}