[tor-bugs] #24392 [Core Tor/Tor]: Ignore cached bridge descriptors until we check if they are running

[Tor Bug Tracker & Wiki]

#24392: Ignore cached bridge descriptors until we check if they are running
-------------------------------------------------+-------------------------
 Reporter:  teor                                 |          Owner:  teor
     Type:  defect                               |         Status:
                                                 |  needs_review
 Priority:  Medium                               |      Milestone:  Tor:
                                                 |  0.3.2.x-final
Component:  Core Tor/Tor                         |        Version:  Tor:
                                                 |  0.3.0.1-alpha
 Severity:  Normal                               |     Resolution:
 Keywords:  030-backport, 031-backport,          |  Actual Points:  0.5
  regression, tor-bridge-client, s8-errors,      |
  review-group-27                                |
Parent ID:  #24367                               |         Points:  0.3
 Reviewer:                                       |        Sponsor:
-------------------------------------------------+-------------------------
Changes (by teor):

 * status:  needs_revision => needs_review


Comment:

 Replying to [comment:19 teor]:
 > Replying to [comment:18 nickm]:
 > > Hi! I have some questions, that are not necessarily blockers.
 > >
 > > ...
 >
 > > Second question: In e9cb0cd7c8fe03dbd7e0fef91ab868cb56f280b0, when we
 do
 > > {{{
 > > -      } else if (!options->UseBridges || num_bridges_usable() > 0) {
 > > +      } else {
 > > }}}
 > > can we add a `tor_nonfatal_assert()` there to make sure that the
 second case really and truly only happens when we think it does?
 >
 > Yes, I'll do this in the morning.

 Done and pushed in:
 {{{
 [bug24367_032 42a77862ab] fixup! Simplify some conditionals in
 circuit_get_open_circ_or_launch()
 }}}

 Waiting for it to compile.

 > > Last:
 > > > I am no longer convinced that this bug is restricted to 0.3.2, we
 should consider backporting these changes to 0.3.0 as a precautionary
 measure.
 > >
 > > This is a pretty complex set of changes, and the rebase isn't clean.
 Is there a minimal set of changes that, we could put into a branch based
 on 0.3.0?
 >
 > I'll look into this in the morning.
 > I think we only need the `int first = num_bridges_usable(0) < 1;`
 change.
 >
 > Because `} else if (!options->UseBridges || num_bridges_usable() > 0) {`
 is redundant, and the final change is on code that didn't exist before
 0.3.2.

 I will work on this now.

 > > And final question: how is the testing on this?
 >
 > It passes all of `make test-network-all`, and gk likes it better than
 any of the previous versions when testing bridge switches in Tor Browser.

 Now the testing also has Travis CI™.base

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