Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#5467 defect closed duplicate (duplicate)

Fix BindAuthority For Bind files with Absolute Names

Reported by: Bob Novas Owned by: Bob Novas
Priority: normal Milestone:
Component: names Keywords:
Cc: Branch:
Author: Bob Novas

Description

BindAuthority doesn't handle bind format zone files with absolute names (names that end in .). This patch provides a fix to BindAuthority and test code that demonstrates the patch.

Attachments (1)

01-FixBindAuthorityForBindFilesWithAbsoluteNames.patch (3.3 KB) - added by Bob Novas 6 years ago.
patch for ticket 5467- fix BindAuthority for absolute names and test code.

Download all attachments as: .zip

Change History (7)

Changed 6 years ago by Bob Novas

patch for ticket 5467- fix BindAuthority for absolute names and test code.

comment:1 Changed 6 years ago by Itamar Turner-Trauring

Keywords: BindAuthority removed
Type: enhancementdefect

I don't understand zone files yet to finish a review, but:

  1. The test should use self.mktemp() to create a temporary file path - no need to delete it when you're done, either, they hang out in _trial_temp and are useful for debugging.
  2. As long as you're testing one domain, having assertions for the other domains seems like a good idea, especially since there don't appear to be any tests for parsing zone files.
  3. Imports always go at the top of the module.
  4. Is this a general issue in the Authority classes, or is it specific to BINDAuthority, in which case maybe the fix should go there?

comment:2 Changed 6 years ago by Itamar Turner-Trauring

I've ordered a copy of "DNS and Bind" from the library, so worst case if someone who knows about DNS doesn't get around to a review I should be able to do one soon.

comment:3 Changed 6 years ago by Itamar Turner-Trauring

Keywords: review removed
Owner: set to Bob Novas

OK, to expand on my last review point: the FileAuthority class seems presume it is storing absolute domain names. This is reasonable. Dealing with converting relative to absolute domain names, and cleaning up file-format specific things like "." at the end of the domain should therefore be in BindAuthority, instead of allowing the domain ending with "." to end up inside the object.

Please fix the above comments and resubmit for review.

comment:4 Changed 6 years ago by Bob Novas

Resolution: duplicate
Status: newclosed

I'm closing this ticket as a duplicate as I've now fixed and tested this in BindAuthority as part of ticket 5470.

comment:5 Changed 6 years ago by Itamar Turner-Trauring

So you've added the whitespace thing to #5470?

comment:6 Changed 6 years ago by Bob Novas

well, yes, the whitespace thing was always part of 5470. It turns out that I fixed the period thing (which this fix fixed poorly by adding a period to the end of names stored in records dict) in 5470 as well. So this fix was overtaken by events, so to speak. Although, there is more to be said on the subject of periods at the end of names - take a careful look at the test cases for 5470 for periods at the end of names. I'm not positive I like what I did there. I'll have 5470 out shortly.

Note: See TracTickets for help on using tickets.