Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
euyuil opened this issue Jan 30, 2022 · 1 comment · Fixed by #146

Comments

@euyuil
Copy link

euyuil commented Jan 30, 2022

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:

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 euyuil changed the title DnsUdpMessageHandler keeps duplicate responses in socket and used by later queries, causes header ID mismatch DnsUdpMessageHandler keeps duplicate responses in socket and reused by later queries, causes header ID mismatch Jan 30, 2022
@MichaCo
Copy link
Owner

MichaCo commented Jan 30, 2022

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)

MichaCo added a commit that referenced this issue Jan 30, 2022
Removing UDP client pooling 
+ some bug fixes: #132, #145, #136, #133
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants