[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
-------------------------------------------------+-------------------------

Comment (by teor):

 Replying to [comment:9 catalyst]:
 > 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.)

 I opened #30968 with your suggestion, and my alternate suggestion (a
 format string that allows us to print the data being tested).

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