[tor-bugs] #30721 [Core Tor/Tor]: tor_addr_port_lookup() is overly permissive

[Tor Bug Tracker & Wiki]

#30721: tor_addr_port_lookup() is overly permissive
-------------------------------------------------+-------------------------
 Reporter:  teor                                 |          Owner:  teor
     Type:  defect                               |         Status:
                                                 |  merge_ready
 Priority:  Medium                               |      Milestone:  Tor:
                                                 |  0.4.2.x-final
Component:  Core Tor/Tor                         |        Version:  Tor:
                                                 |  unspecified
 Severity:  Normal                               |     Resolution:
 Keywords:  technical-debt, tor-addr, refactor,  |  Actual Points:  1.5
  practracker-improvement                        |
Parent ID:                                       |         Points:  0.5
 Reviewer:  catalyst                             |        Sponsor:
                                                 |  Sponsor31-can
-------------------------------------------------+-------------------------
Changes (by catalyst):

 * status:  needs_review => merge_ready


Comment:

 Replying to [comment:8 teor]:
 > Replying to [comment:7 catalyst]:
 > > Thanks! The new tests look reasonable. I wonder if it's necessary to
 make all those multi-statement macro definitions, instead of helper
 functions (and maybe much smaller macros that call those functions)? I
 think helper function would be a little cleaner. Please let me know what
 you think.
 >
 > The problem with helper functions is that failures are attributed to the
 test assertions in the helper function, without any context. So it's very
 hard to tell which test data caused the failure.
 >
 > I'm not sure if there is some way of providing context as part of the
 test macros. I suggest that we merge this code as-is, because it's
 functional, adds coverage, and makes sure we won't introduce future bugs.
 Then we can open a ticket for follow-up.
 I see. I agree we should open a new ticket to follow up, in that case.

 I think I see that the problem is buried in the `TT_DECLARE()` macro in
 `tinytest_macros.h`. I think it's possible to work around it, but it might
 be nontrivial. (Rough sketch: redefine `TT_DECLARE()` in helper functions
 to read file and line info from function parameters, and make file and
 line numbers explicit parameters to helper functions.)

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/30721#comment:9>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online