Opened 7 years ago

Closed 7 years ago

#2166 closed task (fixed)

update RRset::toWire() so it will render RRSIGs, too

Reported by: jinmei Owned by: muks
Priority: medium Milestone: Sprint-20120904
Component: libdns++ Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DNS Feature Depending on Ticket: scalable inmemory
Estimated Difficulty: 4 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

This is the final building block of #2163.

We update the abstract interface of RRset::toWire() so that if it's
associated with RRSIG it also renders the RRSIGs. Then update the
actual implementation accordingly. Finally, adjust test cases that
assume the old behavior. Also re-enable any disabled tests in #2165.

Depends on #2165.

Subtickets

Change History (7)

comment:1 Changed 7 years ago by shane

  • Milestone changed from New Tasks to Next-Sprint-Proposed

comment:2 Changed 7 years ago by jelte

  • Milestone changed from Next-Sprint-Proposed to Sprint-20120821

comment:3 Changed 7 years ago by muks

  • Owner changed from UnAssigned to muks
  • Status changed from new to assigned

Sorry I'm picking this (though I'm not finished with #2165) as I have implemented it. There was no Message::toWire() testcase before for records with RRSIGs and I added one as a part of #2164. Now there's a Message::toWire() testcase, I can test that RRset::toWire() works properly (make check passes inside src/lib/dns/) before moving on to fixing other libraries. For this, I had to implement the RRset::toWire() specializations.

Also there are many tests to update, and it's less work to fix the tests than comment/uncomment them. It's not possible for anyone to work in parallel on this bug anyway.

Last edited 7 years ago by muks (previous) (diff)

comment:4 Changed 7 years ago by muks

  • Owner changed from muks to UnAssigned
  • Status changed from assigned to reviewing

Up for review.

Please note:

  • The work is in the trac2165_2 branch.
  • Whoever picks up this bug for review will also have to assign #2165 to themselves (for review) as both are in the same branch. Please first assign both bugs to yourself before beginning review.

Please see #2165 for more details.

comment:5 Changed 7 years ago by vorner

  • Owner changed from UnAssigned to vorner

comment:6 Changed 7 years ago by vorner

  • Owner changed from vorner to muks

comment:7 Changed 7 years ago by muks

  • Resolution set to fixed
  • Status changed from reviewing to closed

Merged to master in commit 5d1e19947ce5875cd950f1b1750dad5076909446:

* 79f1c5b (origin/trac2165_2) [2165] Remove effects that were used during EDNS testing
* 1159e5b [2165] Test RRType::RRSIG() query when FIND_DNSSEC is not passed
* abcd7d9 [2165] Fix answer section count in comment
* 2fabaed [2165] Don't continue and render RRSIGs if data is truncated
* 11af4c1 [2165] Add TODO to commented out test code
* 33ab70a [2165] Update comment
* a173297 [2165] Move ZoneFinder::stripRRsigs() into a .cc file
* 3afbf54 [2165] Rename variables
* 552c578 [2165] Remove another block of redundant code
* 3c20e03 [2165] Ignore RRSIG database records if sigs argument is false
* 32dd44a [2165] Rename member variable to contain underscore
* a8d00ea [2165] Fix typo in comment
* 94d0fd2 [2165] updater.delete_rrset no longer raises (as there are no attached RRSIGs now)
* 1fbbeb3 [2165] Stop RRSIGs from being added in getRRsets() itself
* 7f00956 [2165] Don't strip RRSIGs from DNSSEC rrsets
* 9a13449 [2165] Remove RRSIG filtering code from getAdditionalAddrs()
* 2a22df2 [2165] Check for NULL rrsets in stripRRsigs() itself
* 52f1d2c [2165] Remove multiple copies of the same code
* 18324f0 [2165] Fix NSEC3 tests to expect RRSIGs in some more cases
* 98a0dad [2165] Fix NSEC3 tests to expect RRSIGs in some cases
* f060129 [2165] Strip RRSIGs in MockZoneFinder when DNSSEC find option is not specified
* ef85853 [2165] Strip RRSIGs from addditional records in DB datasource when DNSSEC is not asked
* 4ecd5fb [2165] Don't add another copy of RRSIGs to the actual rrsets
* cdfbc35 [2165] Strip RRSIGs from addditional records in memory datasource when DNSSEC is not asked
* d057be8 [2165] Add the attached RRSIG when dumping RRsets
* 525a74d [2165] Disable test code that is no longer correct
* 8cc84c6 [2165] Add back deleted dnssec flag in some places
* 5438673 [2165] Update prepareRRset() to filter RRSIGs when ZoneFinder::FIND_DNSSEC is not passed
* 4c86091 [2165] Disable some datasrc tests which cover obsolete API
* c4500a6 [2166] Update RRset::toWire() so it will render RRSIGs too
* ace794f [2165] Make Message::addRRset() to be unaware of signedness

Resolving as fixed. Thank you for the reviews Michael and Jinmei.

Note: See TracTickets for help on using tickets.