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

Ensure ALIAS answers over TCP have correct name #6659

Merged
merged 3 commits into from May 24, 2018

Conversation

Projects
None yet
2 participants
@pieterlexis
Member

pieterlexis commented May 23, 2018

Short description

Closes #6654

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled this code
  • tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added or modified regression test(s)
  • added or modified unit test(s)

pieterlexis added some commits May 23, 2018

@pieterlexis pieterlexis requested a review from Habbie May 23, 2018

@pieterlexis pieterlexis referenced this pull request May 23, 2018

Merged

Auth 4.1.3 backports #6656

4 of 8 tasks complete
@@ -105,7 +105,7 @@ bool DNSProxy::completePacket(DNSPacket *r, const DNSName& target,const DNSName&
for (auto &ip : ips)
{
ip.dr.d_name = target;
ip.dr.d_name = aname;

This comment has been minimized.

@rgacogne

rgacogne May 23, 2018

Member

Change looks good, the comment above the function is wrong (and has been for a long time).

This comment has been minimized.

@pieterlexis

pieterlexis May 23, 2018

Member

The SERVFAIL one? It looked off to me as well, I was planning to see what's up with that later

This comment has been minimized.

@rgacogne

rgacogne May 23, 2018

Member

I was thinking of

//! look up qname aname with r->qtype, plonk it in the answer section of 'r' with name target

but the other one may be wrong too.

@pieterlexis

This comment has been minimized.

Member

pieterlexis commented May 23, 2018

It seemed we did not SERVFAIL on e.g. NXDOMAIN from the resolver. This PR now also contains fixes and a test for this situation.

@pieterlexis

This comment has been minimized.

Member

pieterlexis commented May 24, 2018

It seemed we did not SERVFAIL on e.g. NXDOMAIN from the resolver. This PR now also contains fixes and a test for this situation.

There are some side effects here that requiring some more thinking, I have removed these commits and will add them to a separate PR.

@pieterlexis pieterlexis merged commit 653e822 into PowerDNS:master May 24, 2018

4 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
lgtm analysis: C/C++ No alert changes
Details
lgtm analysis: JavaScript No alert changes
Details
lgtm analysis: Python No alert changes
Details

@pieterlexis pieterlexis deleted the pieterlexis:auth-alias-tcp branch May 24, 2018

pieterlexis added a commit to pieterlexis/pdns that referenced this pull request May 24, 2018

@pieterlexis pieterlexis referenced this pull request Jun 11, 2018

Merged

ALIAS: Respond SERVFAIL on non-NOERRORs from resolver #6727

5 of 7 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment