#32753 closed enhancement (fixed)

Tor should lower-case its BridgeDistribution string

Reported by: arma Owned by:
Priority: Medium Milestone: Tor: 0.4.2.x-final
Component: Core Tor/Tor Version: Tor: 0.3.2.3-alpha
Severity: Normal Keywords: consider-backport-after-0432, 035-backport 041-backport 042-backport easy anticensorship-wants fast-fix
Cc: phw Actual Points:
Parent ID: Points:
Reviewer: dgoulet Sponsor: Sponsor28

Description

See comment:8 in #32547, where phw tells a bridge operator that they need to set BridgeDistribution to "none", because using "None" is not right.

That isn't a good user experience, and it's easy to fix.

One option is that inside check_bridge_distribution_setting() when we do if (!strcmp(bd, RECOGNIZED[i])) we should do strcasecmp instead. That is, don't complain if it's a recognized value and the only issue is that it's not all lowercase.

But a better option imo is that we lowercase it in place first, so that if the user types None, bridgedb still sees none.

Child Tickets

Change History (16)

comment:1 Changed 12 months ago by nickm

Keywords: backport easy anticensorship-wants added
Milestone: Tor: 0.4.3.x-final

comment:2 Changed 11 months ago by nickm

Keywords: fast-fix 043-should added

comment:3 Changed 11 months ago by ahf

Status: newneeds_review

Patches in https://github.com/torproject/tor/pull/1668

phw, does this work like what you expected?

comment:4 Changed 11 months ago by phw

Looks good to me, thanks ahf!

comment:5 Changed 11 months ago by ahf

Cool. Thanks for taking a look. I'll await review from a network team person.

comment:6 Changed 11 months ago by dgoulet

Keywords: 035-backport 041-backport 042-backport added; backport removed
Milestone: Tor: 0.4.3.x-finalTor: 0.4.2.x-final
Reviewer: dgoulet
Status: needs_reviewmerge_ready

Merged!

Moving to 042 component for backport consideration.

comment:7 Changed 11 months ago by ahf

Sponsor: Sponsor28

comment:8 Changed 11 months ago by arma

I would vote 'no backport' for this one, since it's not a security fix and other components are going to have to tolerate the old behavior for a while anyway.

(Also because my high level backport policy is "by default, no backport", with the goal of making the changelogs in new Tor stables small enough that Debian won't be horrified.)

comment:9 Changed 10 months ago by teor

Keywords: consider-backport-after-0432 added

This fixes a serious usability issue for bridge operators, so we should backport it to all supported versions.

comment:10 Changed 10 months ago by teor

Is there some minimal version of this patch that we could backport?

comment:11 Changed 10 months ago by ahf

https://github.com/torproject/tor/pull/1694 contains a backported patch to 0.4.2 maint. Awaiting CI.

comment:12 Changed 10 months ago by teor

I think you removed these lines because they are redundant?

    if (strchr(bd, '\n') || strchr(bd, '\r'))
      bd = escaped(bd);

Maybe we should log a BUG() instead?

comment:13 Changed 10 months ago by teor

Status: merge_readyneeds_revision

comment:14 Changed 10 months ago by teor

Keywords: 043-should removed

comment:15 Changed 10 months ago by teor

Status: needs_revisionmerge_ready

Actually, I'm just going to backport this as-is.

comment:16 Changed 10 months ago by teor

Resolution: fixed
Status: merge_readyclosed
Version: Tor: 0.3.2.3-alpha

Cherry-picked to 0.3.5, and merged forwards.
Used an "ours" merge from 0.4.2 to 0.4.3, because we already merged PR 1668 to 0.4.3.

Note: See TracTickets for help on using tickets.