Opened 8 years ago

Closed 8 years ago

#1502 closed defect (fixed)

xfrin.diff module should distinguish RRSIGs for different covered

Reported by: jinmei Owned by: jinmei
Priority: high Milestone: Sprint-20120110
Component: xfrin Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DNS Feature Depending on Ticket: none
Estimated Difficulty: 5 Add Hours to Ticket: 0
Total Hours: 1 Internal?: no

Description

See this at the bind10-users ML:
https://lists.isc.org/pipermail/bind10-users/2011-December/000156.html

xfrin.diff should handle these RRSIGs separately.

Subtickets

Change History (12)

comment:1 Changed 8 years ago by jinmei

  • Summary changed from xfrin.Diff module should distinguish RRSIGs for different covered to xfrin.diff module should distinguish RRSIGs for different covered

comment:2 Changed 8 years ago by jelte

  • Estimated Difficulty changed from 0 to 5

comment:3 Changed 8 years ago by jelte

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

comment:4 Changed 8 years ago by jelte

  • Priority changed from major to critical

comment:5 Changed 8 years ago by shane

  • Feature Depending on Ticket set to none

comment:6 Changed 8 years ago by jinmei

  • Owner set to UnAssigned
  • Status changed from new to reviewing

trac1502 is ready for review.

I also fixed the log format issue reported on #1471. So when we close
this ticket we can also close #1471.

Proposed changelog entry:

361.?	[bug]		jinmei
	Python xfrin.diff module incorrectly combined RRSIGs of different
	type covered, possibly merging different TTLs.  As a result a
	secondary server could store different RRSIGs than those at the
	primary server if it gets these records via IXFR.
	(Trac #1502, git TBD)

comment:7 follow-up: Changed 8 years ago by vorner

  • Owner changed from UnAssigned to vorner

Hello

I believe the code does what it should. However, the logic of same_type seems hard to follow. May I propose replacing the first two conditions with something like this?

        if rrset1.get_type() != isc.dns.RRType.RRSIG() or \
                rrset2.get_type != isc.dns.RRType.RRSIG():
                return rrset1.get_type() == rrset2.get_type()

Thanks

comment:8 Changed 8 years ago by vorner

  • Owner changed from vorner to jinmei

I shouldn't assign it to myself after the review O:-)

comment:9 in reply to: ↑ 7 Changed 8 years ago by jinmei

Replying to vorner:

I believe the code does what it should. However, the logic of same_type seems hard to follow. May I propose replacing the first two conditions with something like this?

        if rrset1.get_type() != isc.dns.RRType.RRSIG() or \
                rrset2.get_type != isc.dns.RRType.RRSIG():
                return rrset1.get_type() == rrset2.get_type()

Okay, applied. I guess it's now ready for merge based on the tone of
the comment, but will give the ticket back to you just in case.

comment:10 Changed 8 years ago by jinmei

  • Owner changed from jinmei to vorner

comment:11 follow-up: Changed 8 years ago by vorner

  • Owner changed from vorner to jinmei

Yes, it looks nice. Please, merge.

comment:12 in reply to: ↑ 11 Changed 8 years ago by jinmei

  • Resolution set to fixed
  • Status changed from reviewing to closed
  • Total Hours changed from 0 to 1

Replying to vorner:

Yes, it looks nice. Please, merge.

Merge done, closing ticket. Thanks for the review.

I'll also close #1471.

Note: See TracTickets for help on using tickets.