Opened 20 months ago

Last modified 14 months ago

#6724 needs_revision enhancement

Vidalia complains about bridge line starting with "bridge"

Reported by: karsten Owned by: chiiph
Priority: normal Milestone:
Component: Vidalia Version: Vidalia: 0.2.20
Keywords: Cc: asn, fstinglmayr@…
Actual Points: Parent ID: #6434
Points:

Description

When I fetch new bridges from bridges.tpo and copy-paste a line into Vidalia's Network tab (including the "bridge" part), I get the following error messages:

Aug 29 08:37:14.573 [Warning] Error parsing Bridge address 'bridge'
Aug 29 08:37:14.574 [Warning] Controller gave us config lines that
  didn't validate: Bridge line did not parse. See logs for details.

The obfsproxy TBB has the following error message for me:

Aug 29 08:33:55.497 [Warning] We were supposed to connect to bridge
  '(scrubbed)' using pluggable transport 'bridge', but we can't find
  a pluggable transport proxy supporting 'bridge'. This can happen
  if you haven't provided a ClientTransportPlugin line, or if your
  pluggable transport proxy stopped running.

Can we make Vidalia smarter by ignoring the "bridge" part in bridge lines? We can probably get asn's promise to never call a pluggable transport "bridge". ;)

Child Tickets

Change History (10)

comment:1 Changed 20 months ago by karsten

Related to this enhancement, how about changing the text field for adding a bridge into a text area, labeled "Add one or more bridges:"? Users could copy-paste whatever bridge lines they get from bridges.tpo and add them in a single step.

comment:2 in reply to: ↑ description ; follow-up: Changed 20 months ago by asn

Replying to karsten:

When I fetch new bridges from bridges.tpo and copy-paste a line into Vidalia's Network tab (including the "bridge" part), I get the following error messages:

Aug 29 08:37:14.573 [Warning] Error parsing Bridge address 'bridge'
Aug 29 08:37:14.574 [Warning] Controller gave us config lines that
  didn't validate: Bridge line did not parse. See logs for details.

The obfsproxy TBB has the following error message for me:

Aug 29 08:33:55.497 [Warning] We were supposed to connect to bridge
  '(scrubbed)' using pluggable transport 'bridge', but we can't find
  a pluggable transport proxy supporting 'bridge'. This can happen
  if you haven't provided a ClientTransportPlugin line, or if your
  pluggable transport proxy stopped running.

Can we make Vidalia smarter by ignoring the "bridge" part in bridge lines? We can probably get asn's promise to never call a pluggable transport "bridge". ;)

I'm OK with this. I promise. Although maybe it would be better to spec this?

However this would look quite stupid on the spec: maybe we should instead have Vidalia warn the user when the pluggable transport is called 'bridge', saying that this is a common mistake and Vidalia expects the name of the PT or the bridge addrport as the first input to the text box?
Or maybe we should have Vidalia document on the GUI what it expects to see in that text box?

I also like your other idea about switching to a text area!

comment:3 in reply to: ↑ 2 Changed 20 months ago by karsten

Replying to asn:

Replying to karsten:

Can we make Vidalia smarter by ignoring the "bridge" part in bridge lines? We can probably get asn's promise to never call a pluggable transport "bridge". ;)

I'm OK with this. I promise. Although maybe it would be better to spec this?

However this would look quite stupid on the spec: maybe we should instead have Vidalia warn the user when the pluggable transport is called 'bridge', saying that this is a common mistake and Vidalia expects the name of the PT or the bridge addrport as the first input to the text box?

I think the default should be that Vidalia expects the bridge line including the "bridge" part as its input. With addresses and ports only I can see how users can be expected only to input those two things. But with longer bridge lines, it's hard for (new) users to understand why they're supposed to leave out just the "bridge" part when they paste a line into Vidalia.

Or maybe we should have Vidalia document on the GUI what it expects to see in that text box?

Nah, adding more documentation to Vidalia only means that we failed to design it intuitively.

I also like your other idea about switching to a text area!

Cool. :)

comment:4 Changed 18 months ago by asn

  • Parent ID set to #6434

comment:5 Changed 16 months ago by nola

  • Cc fstinglmayr@… added

I implemented a fix for this at:
https://github.com/n0la/vidalia/commit/3e64bcb37ef6981ae9da229ceb451b1714f6dd1e

o The bridge in front of any bridge line is now simply stripped.
o The input box is now accepts more than one bridge to be added at a time, simply separate them with new lines. This allows for all bridge lines from bridges.tpo to be pasted into the box in one swoop.
o Also fixed a bug (?) where deleted bridges where not committed to the configuration.

I'd love to hear some feedback on this change as it's my first committing something to Vidalia.

comment:6 follow-up: Changed 14 months ago by chiiph

  • Status changed from new to needs_revision

Looks good!

I have just two comments:

comment:7 in reply to: ↑ 6 ; follow-up: Changed 14 months ago by nola

Replying to chiiph:

Looks good!

I have just two comments:

Done:
https://github.com/n0la/vidalia/commit/81c91acd068f93fe996f215c806223dca078b378

  • Would you like to backport this to master? I'll merge/push to alpha in the meantime.

I have a backport now:
https://github.com/n0la/vidalia/commit/842be751f252ece0b9a0f3fe7d98851b26cf4acc

But I'd like to test it a bit more before you merge it back into master.

comment:8 in reply to: ↑ 7 ; follow-up: Changed 14 months ago by chiiph

Replying to nola:

Done:
https://github.com/n0la/vidalia/commit/81c91acd068f93fe996f215c806223dca078b378

Merged.

  • Would you like to backport this to master? I'll merge/push to alpha in the meantime.

I have a backport now:
https://github.com/n0la/vidalia/commit/842be751f252ece0b9a0f3fe7d98851b26cf4acc

But I'd like to test it a bit more before you merge it back into master.

I've just tested this. Given that the code is the same as for alpha, and only the UI parts are slightly different I think we this is good to be merged. You should add the changes file.

comment:9 in reply to: ↑ 8 Changed 14 months ago by nola

Replying to chiiph:

I've just tested this. Given that the code is the same as for alpha, and only the UI parts are slightly different I think we this is good to be merged. You should add the changes file.

I have also used it now for some days and it works well here. But that doesn't mean much if the developer of the feature tests his own code ;-).
I have also added the changes file to the master branch.

comment:10 Changed 14 months ago by nola

https://github.com/n0la/vidalia/commit/b007ebc7b3283503d7409b32bda70e43feea5a42

Here is the direct link. Forgot that one in the previous post.

Note: See TracTickets for help on using tickets.