Opened 2 years ago

Last modified 20 months ago

#27739 new defect

rust protover_all_supported() accepts too-long protocol names

Reported by: cyberpunks Owned by:
Priority: Medium Milestone: Tor: unspecified
Component: Core Tor/Tor Version: Tor:
Severity: Normal Keywords: rust, protover, 040-deferred-20190220
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:


569b4e57e23d728969a12751afc6b45f32d0f093 was fixing #25517 but kept the old behavior of allowing protocol names of any length for protover_all_supported(). That's the reason this unit test was failing, and ended up being disabled on rust builds with a ??? comment of confusion.

The reason given in the commit for this behavior was in order to maintain compatibility with consensus methods older than 29. but the corresponding C code change never made any exception like this.

Child Tickets

#27741closednickmtoo many arguments in rust protover_compute_vote()Core Tor/Tor

Change History (7)

comment:1 Changed 2 years ago by teor

Keywords: 035-must protover memory-safety 033-backport 034-backport added
Milestone: Tor: 0.3.5.x-final
Parent ID: #27206

See my explanation in:

There's no reason for rust protover_all_supported() to accept too-long protocol names. We should remove the validated/unvalidated distinction, and backport the change to 0.3.3.

comment:2 Changed 2 years ago by teor

Parent ID: #27206

Actually, let's do this as a separate change.

comment:3 Changed 2 years ago by teor

comment:4 in reply to:  1 ; Changed 2 years ago by cyberpunks

This isn't a memory-safety issue is it? And this is missing the rust tag.

comment:5 in reply to:  4 Changed 2 years ago by teor

Keywords: rust added; 035-must memory-safety 033-backport 034-backport removed
Milestone: Tor: 0.3.5.x-finalTor: 0.3.6.x-final

Replying to cyberpunks:

This isn't a memory-safety issue is it? And this is missing the rust tag.

You're right, the memory safety issue is fixed by #27741.
But it would still be good to remove all the redundant Unvalidated rust code due to the #27741 fix.
We probably don't need to backport a refactor like this.

comment:6 Changed 2 years ago by nickm

Milestone: Tor: 0.3.6.x-finalTor: 0.4.0.x-final

Tor 0.3.6.x has been renamed to 0.4.0.x.

comment:7 Changed 20 months ago by nickm

Keywords: 040-deferred-20190220 added
Milestone: Tor: 0.4.0.x-finalTor: unspecified

Deferring 51 tickets from 0.4.0.x-final. Tagging them with 040-deferred-20190220 for visibility. These are the tickets that did not get 040-must, 040-can, or tor-ci.

Note: See TracTickets for help on using tickets.