=act Stash dns query if there is a collision. #379
Conversation
|
Generally, this looks ok to me. I'd like to have some more eyes on this change before we merge it. |
|
@He-Pin would it be possible to add a test or 2 to this PR - for regression testing but also I think a test might make it clearer what this PR is fixing? Could you also provide the link to the Akka issue - or a Pekko issue - that describes the problem? I'm pretty sure I've seen the related Akka but I can't find it today. |
|
@He-Pin did you see the comment in #371 (comment) about 'To protect DNS poisoning you should also add a check to the response handler in DnsClient to ensure the question in the response matches the question asked.' - fyi @IainHull I don't know if that is something that should be done in addition to this change or instead of this change. It seems useful to have both. |
Signed-off-by: He-Pin <hepin1989@gmail.com>
| } | ||
| replyTo ! response | ||
| inflightRequests -= response.id | ||
| unstashAll() |
| if (questions.head != previousQuestion.head) { | ||
| log.warning( | ||
| "Dns response referenced to different question:{} expected:{} for id:{},maybe an error.", | ||
| questions.head, previousQuestion.head, id) |
There was a problem hiding this comment.
Check the question in the response.
This is a behavior change, it doesn't check before.
|
@pjfanning @mdedetrich I have updated the PR ,with additional checking, |
pjfanning
added
the
async-dns-resolver
If there is already an onging query with same id, stash the query, otherwise send it directly.
When receive a response, unstash all and process.
refs: akka/akka#31905
Modification: