Bridge cards should inform users when a bridge isn't working

added Sponsor 96 label

added Roadmap::Future label

added Feature label

added Desktop label

cc @donuts @nah

Nice idea!

assigned to @donuts

added UX Team label

mentioned in issue tpo/ux/research#107 (closed)

marked this issue as related to tpo/ux/research#107 (closed)

removed the relation with tpo/ux/research#107 (closed)

changed title from [Connection settings] Bridge card should inform which bridge is not working to Bridge cards should inform users when a bridge isn't working

marked this issue as related to tpo/ux/research#109 (closed)

removed the relation with tpo/ux/research#109 (closed)

added 13.5 stable label

mentioned in merge request !695 (merged)

marked this issue as related to tor-browser-build#40848 (closed)

@pierov are we good to close this with !695 (merged) merged?

Nope, it's #41842 (closed) (my bad, I flipped the numbers in the description).

removed the relation with tor-browser-build#40848 (closed)

added Next label and removed Roadmap::Future label

removed Next label

added Roadmap::Future label

removed Sponsor 96 label

marked this issue as related to #41188 (closed)

Trying to sneak this in as part of the development for #42036 (closed), which will require a similar flag for lox bridges. My hope is that we can generalize this feature for all bridges.

removed the relation with #41188 (closed)

marked this issue as related to #42036 (closed)

marked this issue as related to #42299 (closed)

There's another case we can detect but we don't handle right now: an invalid bridge line (see also #42299 (closed)).

Since we have this issue, I've implemented just a workaround to unbreak the page for that issue.

The plan for this ticket is to add a new flag to bridge cards, which has been proposed as part of the work for Lox in #42036 (closed).

Could we handle your issue with some form validation instead, i.e. in #41913?

Yes, I also found that ticket at some point, just forgot to add details about it .

But please keep in mind that users might have old invalid strings and that some user might try to add bridge via about:config.

Also, we do some parsing when we show a card, so we're given the information about a line being invalid "for free", we could use it to display a warning, so it's related also to bridge cards.

Right, we'll go ahead and add it to the bridge card too in that case. Thanks @pierov!

@pierov I wonder if "Can't connect" (the new flag that's planned for lox, see the design at the top of the ticket) is sufficient enough to prompt users with invalid bridge lines to remove and re-add them.

My worry is that users won't understand what we mean by "invalid", or how to remedy the situation, and I'm not sure how else to describe the issue in 1-2 words.

malformed (too technical?)
cannot be parsed/understood (3 words, but maybe it's okay)
missing (some) information/data (invalid at this stage means especially that their IP address is missing or incorrect)

@pierov in the current code, we always send the saved bridge lines to the tor process, regardless of whether parsing it within tor browser fails. Are there ever situations where our parser may fail, but they still "work" for the tor process? E.g. if one line is malformed, but another isn't, would it still be used?

If we know for sure that a failing parse will mean that the tor process will not (successfully) use the configuration, then we should perhaps just not send it.

Moreover, I'm assuming it is not possible to have failing bridge lines from a non-manual bridge source? Or rather, does failing bridge lines setting mean we can safely assume it is manual, even if it was set in "about:config" and the bridge "source" is different?

I think in this situation we could just not show the invalid bridge lines and instead have an info box similar to the custom security level box, with:

<p>
Your manual bridge settings are broken.
</p>
<button>Clear manual bridges</button>
<button>Edit bridges manually</button>

with the latter button opening the manual bridge dialog with the existing setting pre-filled and the error message coming from #41913.

@pierov in the current code, we always send the saved bridge lines to the tor process, regardless of whether parsing it within tor browser fails. Are there ever situations where our parser may fail, but they still "work" for the tor process? E.g. if one line is malformed, but another isn't, would it still be used?

Yes, bridge lines are separate configuration entries, even though they all have the same key.

So, I've tried to add a malformed line (garbage characters) followed by a snowflake line, and this was the log the tor process sent:

2023-12-05 09:42:58.737 [WARN] Too few items to Bridge line.
2023-12-05 09:42:58.737 [WARN] Controller gave us config lines that didn't validate: Bridge line did not parse. See logs for details.
2023-12-05 09:42:58.813 [NOTICE] Opening Socks listener on 127.0.0.1:9150
2023-12-05 09:42:58.814 [NOTICE] Opened Socks listener connection (ready) on 127.0.0.1:9150
2023-12-05 09:42:59.169 [NOTICE] Bootstrapped 1% (conn_pt): Connecting to pluggable transport
2023-12-05 09:42:59.171 [NOTICE] Bootstrapped 2% (conn_done_pt): Connected to pluggable transport
2023-12-05 09:42:59.179 [NOTICE] Bootstrapped 10% (conn_done): Connected to a relay
2023-12-05 09:42:59.460 [NOTICE] Managed proxy "./TorBrowser/Tor/PluggableTransports/snowflake-client": offer created
[...]

So, it continued with snowflake indeed.

Moreover, I'm assuming it is not possible to have failing bridge lines from a non-manual bridge source?

We save that we received bridge lines from BridgeDB for $reasons (not sure which ones, maybe for the old UI), but as a matter of fact, BridgeDB lines and manual lines are treated in the same way.

For the builtin bridges, their lines are stored as prefs, so it's possible for a user to go to about:config and change them, but probably we don't care of such a case.

mentioned in merge request !855 (merged)

removed Roadmap::Future label

added Next label

added 1 design

@donuts please notice that right now we say vanilla bridge with invalid bridges, because we don't get a pluggable transport when the validation fails.

We could say unknown bridge/invalid bridge also there.