DnsUdpMessageHandler keeps duplicate responses in socket and reused by later queries, causes header ID mismatch

I had similar issue with #140 and reviewed #142. I agree the approach of retry on header ID mismatch can increase the possibility of success. And I also think the implementation of DnsUdpMessageHandler needs to be improved in the case we can have duplicate responses.

The class maintains two queues for UDP clients to be reused. As the code shows:

DnsClient.NET/src/DnsClient/DnsUdpMessageHandler.cs

Lines 112 to 130 in d56cb12

    
                           using (var memory = new PooledBytes(readSize)) 
        
                           { 
        
           #if !NET45 
        
                               int received = await udpClient.Client.ReceiveAsync(new ArraySegment<byte>(memory.Buffer), SocketFlags.None).ConfigureAwait(false); 
        
                               var response = GetResponseMessage(new ArraySegment<byte>(memory.Buffer, 0, received)); 
        
           #else 
        
                               var result = await udpClient.ReceiveAsync().ConfigureAwait(false); 
        
                               var response = GetResponseMessage(new ArraySegment<byte>(result.Buffer, 0, result.Buffer.Length)); 
        
           #endif 
        
                               ValidateResponse(request, response); 
        
                               Enqueue(endpoint.AddressFamily, udpClient); 
        
                               return response; 
        
                           }

A UDP client will be enqueued for later reuse after receiving data from it. However, based on the fact that we have duplicate responses, a UDP client can still receive data even after it has been enqueued. So it will later get reused by another query, and in that query it will read some nonsense response.

I think that is (one of) the root cause(s) why LookupClient got confused as described in #140. So I propose to improve the class DnsUdpMessageHandler to better support the duplicate response scenario.

@euyuil

Hi @euyuil
Thanks for taking the time to test and replicate that behavior. I hope you can help me test the changes later on, too ;)

But for this issue, I'd consider it a duplicate of #132.
I'm already working on removing the pooling for UDP traffic as mentioned in that issue. (almost done)

Removing UDP client pooling (#146)

Navigation Menu

Repository navigation

DnsUdpMessageHandler keeps duplicate responses in socket and reused by later queries, causes header ID mismatch #145

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	using (var memory = new PooledBytes(readSize))
	{
	#if !NET45
	int received = await udpClient.Client.ReceiveAsync(new ArraySegment<byte>(memory.Buffer), SocketFlags.None).ConfigureAwait(false);

	var response = GetResponseMessage(new ArraySegment<byte>(memory.Buffer, 0, received));

	#else
	var result = await udpClient.ReceiveAsync().ConfigureAwait(false);

	var response = GetResponseMessage(new ArraySegment<byte>(result.Buffer, 0, result.Buffer.Length));
	#endif

	ValidateResponse(request, response);

	Enqueue(endpoint.AddressFamily, udpClient);

	return response;
	}

DnsUdpMessageHandler keeps duplicate responses in socket and reused by later queries, causes header ID mismatch #145

Description

Activity

MichaConrad commented on Jan 30, 2022

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions