Re: [tor-bugs] #7478 [Core Tor/Tor]: Allow routersets to include/exclude nodes by IPv6 address

["Tor Bug Tracker & Wiki" <blackhole@xxxxxxxxxxxxxx>]

#7478: Allow routersets to include/exclude nodes by IPv6 address
-------------------------------------------------+-------------------------
 Reporter:  nickm                                |          Owner:  teor
     Type:  enhancement                          |         Status:
 Priority:  High                                 |  needs_review
Component:  Core Tor/Tor                         |      Milestone:  Tor:
 Severity:  Normal                               |  0.2.9.x-final
 Keywords:  tor-client, ipv6,                    |        Version:
  TorCoreTeam201605, TorCoreTeam-                |     Resolution:
  postponed-201604, review-group-2               |  Actual Points:
Parent ID:                                       |         Points:  1
 Reviewer:  mikeperry                            |        Sponsor:
-------------------------------------------------+-------------------------

Comment (by andrea):

 Begin review of branch at https://github.com/fergus-
 dall/tor/commits/ticket-7478-2:

 3ed42323dbe225cb8cc6b26eac8b9f535065f1cc:

  - I believe this code is correct, but I'd feel better about it if it made
    routerset_contains_router() call routerset_contains() twice and || the
    results rather than adding a second addr/orport to
 routerset_contains(),
    for the sake of keeping interfaces clean.  This approach will
 generalize
    much more cleanly if we ever have cases where routers have more than
 two
    addresses to check as well.

 0e4a2063d6e476d91386a67c633899052ffd708a:

  - New unit tests for routerset_contains(); looks okay.

 276c53829c50197491ceb18f917c2adc36cc499d:

  - This just renames some parameters

 9c4a7c174955666240dfa9605d9e5ffa64a2dbc0:

  - Adds null checks to routerset_contains().  This is important given the
    new interface to make it possible to disable checking one set of args,
    but I really prefer keeping routerset_contains() clean and calling
 multiple
    times when there's more than one address to check.

 6dd42e85ea26d3b026417c8b3088c7b539bfadac:

  - Unit test fixups after 9c4a7c174955666240dfa9605d9e5ffa64a2dbc0

 End code review

 Summary: I think this patch will work but I don't really like crufting up
 routerset_contains() this way - but I also don't have a totally good feel
 about routerset_contains() in general.  Perhaps we should have a simpler
 routerset_contains_addr() or something like that for clarity, since we
 have
 other queries against a routerset like routerset_contains_router().

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/7478#comment:37>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs