Tor Launcher strips "bridge" from the middle of bridge lines

I tried configuring a meek bridge on a URL with "bridge" in the domain name.

bridge meek 0.0.2.0:5 url=https://bridge.example.com/

When I clicked Connect, "bridge" disappeared from the URL and broke the configuration.

bridge meek 0.0.2.0:5 url=https://.example.com/

added GeorgKoppen201504R TorBrowserTeam201504 component::applications/tor launcher owner::brade priority::medium resolution::fixed status::closed tbb-4.5-alpha tbb-usability type::defect labels

Trac:
Username: yoah

0001-Bug-13576-When-parsing-bridge-defs-only-zap-leading-.patch

Fix for bug #13576 (moved)

Trac:
Username: yoah

Trac:
Cc: N/A to mcs

Thanks for the patch and sorry for the delay! A fix that also includes some additional small improvements is available here:

https://gitweb.torproject.org/user/brade/tor-launcher.git/commit/?h=bug13576-01&id=78f505c2139341e71538e77a975f7df9f81189d0

Isis and Sherief -- Kathy and I value your opinion on these changes, especially:

1. "bridge " will no longer be shown in Tor Launcher's custom bridges textbox.
2. Lines within that textbox will not wrap, which means users may need to scroll horizontally to see long bridge lines (but Kathy and I think that is less confusing than wrapping).

Trac:
Status: new to needs_review
Cc: mcs to mcs, isis, sherief
Keywords: N/A deleted, tbb-usability, TorBrowserTeam201503R added

Trac:
Keywords: N/A deleted, TorBrowserTeam201504 added

Trac:
Keywords: TorBrowserTeam201503R deleted, TorBrowserTeam201504R added

Replying to mcs:

Thanks for the patch and sorry for the delay! A fix that also includes some additional small improvements is available here:

https://gitweb.torproject.org/user/brade/tor-launcher.git/commit/?h=bug13576-01&id=78f505c2139341e71538e77a975f7df9f81189d0

Isis and Sherief -- Kathy and I value your opinion on these changes, especially:

1. "bridge " will no longer be shown in Tor Launcher's custom bridges textbox.

Shown in the textbox? Or re-added to the beginning of the line? Because if the "bridge " prefix is being stripped once from the beginning of the line in parseAndValidateBridges(), and then not re-added again in setBridgeListElemValue():

@@ -1406,11 +1407,7 @@ function setBridgeListElemValue(aBridgeArray)
     {
       var s = aBridgeArray[i].trim();
       if (s.length > 0)
-      {
-        if (s.toLowerCase().indexOf("bridge") != 0)
-          s = "bridge " + s;
         bridgeList.push(s);
-      }
     }
   }

Then tor won't be able to set the "Bridge" config line.

2. Lines within that textbox will not wrap, which means users may need to scroll horizontally to see long bridge lines (but Kathy and I think that is less confusing than wrapping).

I agree. (I also added a similar change recently in BridgeDB because of the obfs4 lines being huge.)

Trac:
Status: needs_review to needs_revision

Replying to isis:

Shown in the textbox? Or re-added to the beginning of the line? Because if the "bridge " prefix is being stripped once from the beginning of the line in parseAndValidateBridges(), and then not re-added again in setBridgeListElemValue(): ... Then tor won't be able to set the "Bridge" config line.

The lines contained in the textbox are not directly passed to the control port via SETCONF.

TorSetConf() inside src/components/tl-protocol.js uses a JS object to construct a SETCONF command.

For the bridge config., getAndValidateBridgeSettings() inside network-settings.js creates the JS object. It contains two properties: "Bridge" (which is an array that contains the lines from the textbox) and "UseBridges" (which is a Boolean). So the "Bridge" part really comes from the property name and does not depend on what is in the textbox (in the current code, parseAndValidateBridges() removes "bridge" everywhere within the text; this patch changes things to remove it on a line-by-line basis).

Does the above make sense?

2. Lines within that textbox will not wrap, which means users may need to scroll horizontally to see long bridge lines (but Kathy and I think that is less confusing than wrapping).

I agree. (I also added a similar change recently in BridgeDB because of the obfs4 lines being huge.)

I am glad our approach matches yours.

Trac:
Status: needs_revision to needs_review

We should consider taking this patch for TB 4.5, assuming it passes review.

Trac:
Keywords: N/A deleted, tbb-4.5-alpha added
Cc: mcs, isis, sherief to mcs, isis, sherief, gk

Trac:
Keywords: N/A deleted, GeorgKoppen201504R added

Replying to mcs:

The lines contained in the textbox are not directly passed to the control port via SETCONF.

[…]

Does the above make sense?

Yes, thanks for taking the time to explain; I missed where the config option name part was coming from.

This patch looks good to me.

Looks good. Merged in commit a37371ac9a866612e91a791251104184caad41ca.

Trac:
Status: needs_review to closed
Resolution: N/A to fixed

The commit for this ticket also fixes #10384 (moved).

Trac:
Keywords: TorBrowserTeam201504R deleted, N/A added

closed

moved to tpo/applications/tor-launcher#13576 (closed)