Opened 8 years ago

Closed 8 years ago

#1138 closed task (complete)

RR type implementation: DHCID

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

Description

Implement the DHCID type, from RFC 4701. Since this is commonly in use in production systems today, I'm making this "major" priority.

See ticket #809 for more discussion.

Subtickets

Change History (18)

comment:1 Changed 8 years ago by stephen

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

comment:2 Changed 8 years ago by dvv

  • Owner changed from UnAssigned to dvv
  • Status changed from new to accepted

comment:3 Changed 8 years ago by stephen

  • Estimated Difficulty changed from 0.0 to 4

comment:4 Changed 8 years ago by dvv

  • Owner changed from dvv to UnAssigned
  • Status changed from accepted to reviewing

Initial implementation. Still working on documenting the code and testing.

Question: do we need to hide the implementation details the way we have it in DS and elsewhere?

comment:5 Changed 8 years ago by jelte

  • Owner changed from UnAssigned to dvv

Honestly, I don't see the need to use PIMPL in these classes, we can do it for consistency.

We do want accessors to the rdata fields though, which do not appear to be present yet.

As for documentation, the any_255/tsig implementation is a great example (though that one is VERY elaborate).

One initial comment on the code; there's tabs in the compare method, which should be spaces

comment:6 Changed 8 years ago by dvv

  • Owner changed from dvv to jelte

style fixes and docs in dhcid_49.h and dhcid_49.cc

comment:7 Changed 8 years ago by jelte

  • Owner changed from jelte to dvv

Looks good. I have taken the liberty to commit one addition, it implicitly depended on one of the other type files to include the hex encoder/decoder (and to use the namespace), so I added them explicitely.

Please verify my change and go ahead and merge if it's ok.

comment:8 Changed 8 years ago by dvv

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

merged into master

comment:9 Changed 8 years ago by dvv

  • Resolution fixed deleted
  • Status changed from closed to reopened

{encode,decode}Hex() is erroneously used instead of {encode,decode}Base64()

comment:10 Changed 8 years ago by dvv

  • Status changed from reopened to accepted

comment:11 Changed 8 years ago by dvv

  • Owner changed from dvv to jelte
  • Status changed from accepted to reviewing

Hex/Base64 snafu fixed

comment:12 Changed 8 years ago by jelte

  • Owner changed from jelte to dvv

Change looks OK, but this does remind me i don't see any tests...

comment:13 Changed 8 years ago by jinmei

  • Type changed from enhancement to task

comment:14 Changed 8 years ago by jelte

  • Milestone changed from Sprint-20110927 to Sprint-20111011

comment:15 Changed 8 years ago by dvv

  • Owner changed from dvv to jelte

unit tests created

comment:16 Changed 8 years ago by jelte

  • Owner changed from jelte to dvv

it seems to suffer from the same issues as master right now, so i'll assume those arent caused by this branch, in which case this can be merged.

comment:17 Changed 8 years ago by jelte

  • Milestone changed from Sprint-20111011 to Sprint-20111025

comment:18 Changed 8 years ago by dvv

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

merged into master

Note: See TracTickets for help on using tickets.