Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#1810 closed task (complete)

support NO_WILDCARD option in InMemoryZoneFinder::find().

Reported by: jinmei Owned by: jelte
Priority: medium Milestone: Sprint-20120515
Component: data source Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DNS Feature Depending on Ticket: in-memory NSEC
Estimated Difficulty: 3 Add Hours to Ticket: 0
Total Hours: 1.15 Internal?: no

Description (last modified by jinmei)

As the summary says.

In some sense it's independent but probably better to hold off until
#1809 is completed because in practice it'd be used with DNSSEC
required and the expected result is NXDOMAIN. So for effective tests
it would be better to have #1809 ready.

REVISED: we should start this now, without waiting for #1809 to
maximize development concurrency.

Subtickets

Change History (13)

comment:1 Changed 6 years ago by jelte

  • Estimated Difficulty changed from 0 to 3

comment:2 Changed 6 years ago by jelte

  • Milestone changed from New Tasks to Sprint-20120515

comment:3 Changed 6 years ago by jinmei

  • Description modified (diff)

comment:4 Changed 6 years ago by jinmei

  • Owner set to jinmei
  • Status changed from new to accepted

comment:5 Changed 6 years ago by jinmei

trac1810 is ready for review.

Like others, it first merged #1803 and #1805. It also merged #1809.
The real changes to be reviewed are the last two commits.

The main change doesn't depend on other branch, but in practice
it will make more sense to hold off merging until these precedent
branches are merged.

comment:6 Changed 6 years ago by jinmei

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

comment:7 Changed 6 years ago by jelte

  • Owner changed from UnAssigned to jelte

comment:8 follow-up: Changed 6 years ago by jelte

  • Owner changed from jelte to jinmei

The change looks simple & okay.

I am wondering if there is a hidden semantical conflict with 1808 though :) Or maybe just an oversight; the DATASRC_MEM_NXRRSET either also needs some form of NO_WILDCARD check, or maybe it simply should never rename its result; when I merge all branches together I get a wildcard-expanded NSEC record in my responses.

comment:9 in reply to: ↑ 8 ; follow-up: Changed 6 years ago by jinmei

Replying to jelte:

The change looks simple & okay.

I am wondering if there is a hidden semantical conflict with 1808 though :) Or maybe just an oversight; the DATASRC_MEM_NXRRSET either also needs some form of NO_WILDCARD check, or maybe it simply should never rename its result; when I merge all branches together I get a wildcard-expanded NSEC record in my responses.

Do you mean what if rename is true while NO_WILDCARD is specified?

        const bool rename = ((node_result.flags &
                              ZoneData::FindNodeResult::FIND_WILDCARD) != 0);

This should be impossible, because findNode() should never set the
FIND_WILDCARD flag if NO_WILDCARD is specified. It's a bit indirect,
but I guess it's pretty clear from the findNode() implementation.

But we could also make that 100% sure by explicitly asserting it:

        const bool rename = ((node_result.flags &
                              ZoneData::FindNodeResult::FIND_WILDCARD) != 0);
        // findNode() must ensure wildcard isn't explored with NO_WILDCARD
        assert((options & ZoneFinder::NO_WILDCARD) == 0 || !rename);

Would you like to do that?

comment:10 Changed 6 years ago by jinmei

  • Owner changed from jinmei to jelte

comment:11 in reply to: ↑ 9 Changed 6 years ago by jinmei

Replying to jinmei:

Replying to jelte:

The change looks simple & okay.

I am wondering if there is a hidden semantical conflict with 1808 though :) Or maybe just an oversight; the DATASRC_MEM_NXRRSET either also needs some form of NO_WILDCARD check, or maybe it simply should never rename its result; when I merge all branches together I get a wildcard-expanded NSEC record in my responses.

Okay, I now (believe I) understand what you meant, which should be this:
http://bind10.isc.org/ticket/1808#comment:12
and you're right. We should address it in #1808.

comment:12 Changed 6 years ago by jelte

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

Merged, closing ticket.

comment:13 Changed 6 years ago by jinmei

  • Total Hours changed from 0 to 1.15
Note: See TracTickets for help on using tickets.