rec: improve dnssec record skipping for non dnssec queries #4119

Merged
merged 2 commits into from Jul 21, 2016

Projects

None yet

3 participants

@mind04
Contributor
mind04 commented Jul 6, 2016

successor of #4096

closes #4096
fixes #4084

@pieterlexis pieterlexis added this to the rec-4.0.0 milestone Jul 6, 2016
@Habbie Habbie commented on an outdated diff Jul 6, 2016
pdns/pdns_recursor.cc
@@ -1006,7 +1006,8 @@ void startDoResolve(void *p)
bool needCommit = false;
for(auto i=ret.cbegin(); i!=ret.cend(); ++i) {
- if(!DNSSECOK && (i->d_type == QType::RRSIG || i->d_type==QType::NSEC || i->d_type==QType::NSEC3))
+ if(!DNSSECOK && ((((i->d_type == QType::RRSIG || i->d_type==QType::NSEC) && i->d_place != DNSResourceRecord::ANSWER) || i->d_type==QType::NSEC3) ||
+ (i->d_type == QType::RRSIG && dc->d_mdp.d_qtype != i->d_type && dc->d_mdp.d_qtype != QType::ANY)))
@Habbie
Habbie Jul 6, 2016 Member

Can we reformat this to more lines so the () levels are clearer? And can you replace the last i->d_type with QType::RRSIG? I think this would make the code easier to understand.

Otherwise LGTM.

@Habbie
Member
Habbie commented Jul 7, 2016

One formatting nit, one clarity nit, both in the second commit. Otherwise whole PR looks good to me.

@Habbie Habbie modified the milestone: rec-4.0.x, rec-4.0.0 Jul 8, 2016
@Habbie
Member
Habbie commented Jul 8, 2016

Note: first half already merged as #4096

@Habbie Habbie merged commit f18acf0 into PowerDNS:master Jul 21, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@mind04 mind04 deleted the mind04:recursor branch Jul 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment