Ignore responses with unexpected IDs #1155
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #1155 +/- ##
=======================================
Coverage 55.89% 55.89%
=======================================
Files 41 41
Lines 10141 10142 +1
=======================================
+ Hits 5668 5669 +1
Misses 3468 3468
Partials 1005 1005
Continue to review full report at Codecov.
|
|
[ Quoting <[email protected]> in "[miekg/dns] Ignore responses with u..." ]
This fixes the following problem:
ErrID is returned in this case and the caller needs to check for this. There are other
checks that can be done as well, such as double checking if the case of the domain names
matches what was sent.
nacking this pr, as it may be useful for debug programs to actually ignore the ID.
/close
|
tmthrgd
requested changes
Oct 11, 2020
Codecov Report
@@ Coverage Diff @@
## master #1155 +/- ##
==========================================
- Coverage 55.89% 55.12% -0.77%
==========================================
Files 41 42 +1
Lines 10141 9581 -560
==========================================
- Hits 5668 5282 -386
+ Misses 3468 3274 -194
- Partials 1005 1025 +20
Continue to review full report at Codecov.
|
tmthrgd
requested changes
Oct 13, 2020
tmthrgd
requested changes
Oct 13, 2020
|
I've made the requested changes. Please take another look. |
tmthrgd
approved these changes
Oct 14, 2020
|
|
|
this needs a rebase to pick up the travis changes |
tmthrgd
requested changes
Oct 14, 2020
This fixes the following problem: At time 0, we send a query with ID X from port P. At time T, we time out the query due to lack of response, and then send a different query with ID Y. By coincidence, the new query is sent from the same port number P (since port numbers are only 16 bits, this can happen with non-negligible probability when making queries at a high rate). At time T+epsilon, we receive a response to the original query. Since the ID in this response is X, not Y, we would previously return ErrId, preventing the second query from succeeding. With this commit, we simply ignore the response with the mismatched ID and return once we receive the response with the correct ID.
The new test sends two replies: the first one has a bad ID, which should be ignored, and the second one has the correct ID.
|
Should be good now. Please take another look. |
aanm
pushed a commit
to cilium/dns
that referenced
this pull request
Jul 29, 2022
* Ignore replies with unexpected IDs This fixes the following problem: At time 0, we send a query with ID X from port P. At time T, we time out the query due to lack of response, and then send a different query with ID Y. By coincidence, the new query is sent from the same port number P (since port numbers are only 16 bits, this can happen with non-negligible probability when making queries at a high rate). At time T+epsilon, we receive a response to the original query. Since the ID in this response is X, not Y, we would previously return ErrId, preventing the second query from succeeding. With this commit, we simply ignore the response with the mismatched ID and return once we receive the response with the correct ID. * Update test for bad ID The new test sends two replies: the first one has a bad ID, which should be ignored, and the second one has the correct ID. * Add test to ensure query times out when server returns bad ID * Avoid use of error string matching in test case * Check for mismatched query IDs when using TCP * Reduce timeout in TestClientSyncBadID
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes the following problem:
At time 0, we send a query with ID X from port P.
At time T, we time out the query due to lack of response, and then send a different query with ID Y. By coincidence, the new query is sent from the same port number P (since port numbers are only 16 bits, this can happen with non-negligible probability when making queries at a high rate).
At time T+epsilon, we receive a response to the original query. Since the ID in this response is X, not Y,
exchangereturnsErrId, preventing the second query from succeeding.Since UDP does not have connections, there's no guarantee that a response that we receive is actually associated with the query that we sent - the ID is needed to make the association. Therefore, it's more appropriate to ignore replies with unexpected IDs that to consider them an error. Accordingly, this pull request changes
exchangeto ignore responses with mismatched IDs.exchangecontinues waiting for a response with the expected ID and returns it when it arrives.