[tor-bugs] #31611 [Core Tor/Tor]: Work out why chutney didn't fail due to #31495 cannot configure bridges

[Tor Bug Tracker & Wiki]

#31611: Work out why chutney didn't fail due to #31495 cannot configure bridges
-------------------------------------------------+-------------------------
 Reporter:  teor                                 |          Owner:  nickm
     Type:  defect                               |         Status:
                                                 |  needs_review
 Priority:  Medium                               |      Milestone:  Tor:
                                                 |  0.4.2.x-final
Component:  Core Tor/Tor                         |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  network-team-roadmap-september,      |  Actual Points:
  042-should                                     |
Parent ID:  #29211                               |         Points:
 Reviewer:  teor                                 |        Sponsor:
                                                 |  Sponsor31-must
-------------------------------------------------+-------------------------
Changes (by nickm):

 * status:  new => needs_review


Comment:

 Replying to [comment:6 teor]:
 > Replying to [comment:4 nickm]:
 > > Moving forward, here are the possible solutions I can see:
 > >
 > > 1. When we fuzz configurations, we test that if a configuration passes
 validation, it still passes validation when it is duplicated.
 >
 > Seems sensible, but fuzzing takes much longer to show failures than
 tests.
 > Let's defer this task, unless the other options don't get us what we
 want.
 >
 > > 2. (a) We can document more carefully the dangers of configuration
 values where NULL and "" mean different things.  (In the case of
 EntryNodes, NULL means "all nodes are included", and "" means "no nodes
 are included".)
 >
 > Yes, let's add comments in 0.4.2.

 Added #31907 for this.

 > > 2. (b) We avoid configuration types where NULL and "" mean different
 things.  We would have to add a new routerset type meaning "all routers",
 (maybe, "*"?) and have that be the default for EntryNodes.  [We probably
 could not make NULL mean the same as "" for all cases, since there are
 lots of string-valued configuration types.]
 >
 > Let's consider making NULL and "" mean the same thing in 0.4.3.

 Added as #31908.

 > > 3. We write a testing tool that uses stem to try assigning
 configurations and making sure that they pass and fail with the expected
 errors messages.
 >
 > Let's consider a change like this.
 >
 > Here are some specific suggestions:
 > * 0.4.2: add an extra config test to stem, so it is run by tor's "make
 test-stem"
 >   * let's try to add a stem test that would have caught this bug?

 #31909 is for this.

 > * 0.4.3: add a SETCONF step to the newly added config parsing tests run
 by "make check"
 >   * maybe? This seems like overkill.
 >
 > > 4. We change the implementation of SETCONF so that instead of starting
 with config_dup(), it remembers the actual sequence of configuration
 values, appends the controller's new configuration values, and replays all
 of them in sequence. [This solution would require arbitrary amounts of
 memory.]
 >
 > This feels like a bad idea, it seems to lock in a particular config
 model. It might make configs really hard to split or maintain in future.

 I agree, but I thought I should mention it for completeness.

 Now that the tickets above are opened, can we close this? Please close if
 you agree.

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