[tbb-bugs] #30501 [Applications/Tor Browser]: BridgesList Preferences is an overloaded field

[Tor Bug Tracker & Wiki]

#30501: BridgesList Preferences is an overloaded field
-----------------------------------------------+---------------------------
 Reporter:  sisbell                            |          Owner:  tbb-team
     Type:  defect                             |         Status:
                                               |  needs_revision
 Priority:  Medium                             |      Milestone:
Component:  Applications/Tor Browser           |        Version:
 Severity:  Normal                             |     Resolution:
 Keywords:  tbb-mobile, TorBrowserTeam201910R  |  Actual Points:
Parent ID:  #31280                             |         Points:  2
 Reviewer:                                     |        Sponsor:
                                               |  Sponsor30-can
-----------------------------------------------+---------------------------
Changes (by sysrqb):

 * status:  needs_review => needs_revision


Comment:

 `service/src/main/java/org/torproject/android/service/AndroidTorSettings.java`:

 {{{
 -    @Override
 -    public boolean hasDormantCanceledByStartup() {
 -        return true;
 -    }
 }}}
 Don't we need this?

 `service/src/main/java/org/torproject/android/service/util/Prefs.java`

 {{{
 +    public static void setBridgeTypes(List<String> bridgeTypes) {
 +        if(bridgeTypes == null || bridgeTypes.isEmpty()) {
 }}}
 nit: "if ("

 {{{
 +    public static List<String> getBridgeTypes() {
 +        String bridgeTypes = prefs.getString(PREF_BRIDGE_TYPES, null);
 +        if(bridgeTypes == null || bridgeTypes.isEmpty()) {
 }}}
 nit: "if (" (and the other instances in the patch).

 {{{
 +            BridgeType defaultBridgeType =
 (Locale.getDefault().getLanguage().equals("fa")) ? BridgeType.MEEK_LITE :
 BridgeType.OBFS4;
 }}}
 We should check with the anti-censorship team that this is still the
 correct default. I know we inherited this from Orbot.

 {{{
 +            return
 Collections.singletonList(defaultBridgeType.name().toLowerCase());
 }}}
 I'm not opposed to this, but did you choose a singletonList for a reason?

 {{{
 +        return Arrays.asList(bridgeTypes.split("[|]"));
 }}}
 Please create a wrapper-method around `split()`, like `join()`, in case we
 need to change the separate at a later point.

 {{{
 +    private static String join(List<String> list) {
 +        StringBuilder sb = new StringBuilder();
 +        for (int i = 0; i < list.size(); i++) {
 +            sb.append(list.get(i));
 +            if (i != list.size() - 1) {
 +                sb.append("|");
 }}}
 I think `|` is a safe divider, but we should confirm with the anti-
 censorship team. In particular, I think `|` could be used in the PT
 arguments, but I don't think any of the current transports use an encoding
 that could include `|`.

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