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

fix reading of ednsflags in recursor testing #5617

Merged
merged 2 commits into from Nov 28, 2017
Merged

Conversation

Habbie
Copy link
Member

@Habbie Habbie commented Aug 16, 2017

Short description

We parsed the DNS header flags section as if it were the EDNS flags section. Due to the placement of the QR and DO bit in these sections respectively, this caused all testcases to always see a DO bit.

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled and tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added or modified regression test(s)
  • added or modified unit test(s)
  • checked that this code was merged to master

@rgacogne rgacogne added this to the rec-4.1.0 milestone Aug 16, 2017
@Habbie
Copy link
Member Author

Habbie commented Aug 16, 2017

This will be red on Travis until #5618 is fixed.

@aerique aerique modified the milestones: rec-4.2.0, rec-4.1.0 Aug 21, 2017
@pieterlexis
Copy link
Contributor

Pleas rebase on master, the underlying issue is fixed.

@rgacogne rgacogne modified the milestones: rec-4.2.0, rec-4.1.0 Nov 21, 2017
@Habbie
Copy link
Member Author

Habbie commented Nov 22, 2017

Rebased but not tested

@Habbie Habbie modified the milestones: rec-4.1.0, rec-4.1.x Nov 27, 2017
@Habbie
Copy link
Member Author

Habbie commented Nov 27, 2017

Force-pushed a fix for something I was confused about cosmetically in our tests. Thanks @pieterlexis. This should pass now.

@Habbie Habbie merged commit ccbe288 into PowerDNS:master Nov 28, 2017
@Habbie Habbie deleted the ednsflags branch November 28, 2017 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants