Opened 4 months ago

Closed 4 months ago

Last modified 4 months ago

#27164 closed defect (fixed)

rust protover accepts "Link=1-5-foo"

Reported by: cyberpunks Owned by:
Priority: Medium Milestone: Tor: 0.3.5.x-final
Component: Core Tor/Tor Version: Tor: 0.3.3.9
Severity: Normal Keywords: rust, 033-backport, 034-backport
Cc: Actual Points:
Parent ID: Points:
Reviewer: teor Sponsor:

Description

Parsing a range of versions, it accepts and ignores the 2nd hyphen and everything after it. "Link=1-5-6 LinkAuth=1-1-foo,3" is considered valid.

Child Tickets

Change History (9)

comment:1 Changed 4 months ago by cyberpunks

Patch in branch rust-protohyphen at ​https://gitgud.io/onionk/tor.git

comment:2 Changed 4 months ago by nickm

Milestone: Tor: 0.3.5.x-final
Status: newneeds_review

comment:3 Changed 4 months ago by teor

Reviewer: teor
Status: needs_reviewneeds_revision

Thanks for this patch, it looks good to me.

Here's what needs to change before it merges:

  • code changes need a changes file that passes make check-changes
    • (doc and comment changes don't get a changes file)
  • the branch needs to be rebased onto maint-0.3.3

Let us know if you don't want to make these changes, and someone else will do them eventually.

And also:

comment:5 in reply to:  3 ; Changed 4 months ago by cyberpunks

Replying to teor:

  • code changes need a changes file that passes make check-changes
    • (doc and comment changes don't get a changes file)

Slight tangent, is that always true? Since doc26638 exists.

Branch is rebased.

comment:6 in reply to:  5 Changed 4 months ago by nickm

Resolution: fixed
Status: needs_revisionclosed

Replying to cyberpunks:

Replying to teor:

  • code changes need a changes file that passes make check-changes
    • (doc and comment changes don't get a changes file)

Slight tangent, is that always true? Since doc26638 exists.

For comments, generally not, unless it's something massive and important.

For documentation, it's optional for small fixes and suggested for big ones.

Branch is rebased.

Thanks! Looks better now; merged to 0.3.3 and forward.

comment:7 Changed 4 months ago by cyberpunks

Oh, I added more tests for this and #27177 in the branch prototest1.

comment:8 in reply to:  7 Changed 4 months ago by teor

Replying to cyberpunks:

Oh, I added more tests for this and #27177 in the branch prototest1.

Would you mind opening another ticket?

Integers are cheap, but reading long comment threads takes time.
(And mistakes cost time, too.)

comment:9 Changed 4 months ago by teor

I'm at a more capable device now, so I opened #27195.

Thanks!

Note: See TracTickets for help on using tickets.