Opened 7 months ago

Closed 5 months ago

Last modified 5 months ago

#26223 closed enhancement (implemented)

Allow longer bandwidth lines in bandwidth files

Reported by: teor Owned by: nickm
Priority: Medium Milestone: Tor: 0.3.5.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: easy, intro, fast-fix
Cc: Actual Points:
Parent ID: #25925 Points:
Reviewer: teor Sponsor:

Description

Bandwidth files have a line limit of 510 characters (512 - newline and NUL).

We should make sure this limit is high enough for future bandwidth files.

We can do this by finding out the current length of sbws and torflow lines, multiplying it by 4x, then rounding up to the nearest power of two.

We can use the examples in the spec if we need to:
https://gitweb.torproject.org/torspec.git/tree/bandwidth-file-spec.txt

Maybe we should wait until sbws has added all the new fields?

Child Tickets

Change History (6)

comment:1 in reply to:  description Changed 6 months ago by juga

Replying to teor:

We should make sure this limit is high enough for future bandwidth files.

We can do this by finding out the current length of sbws and torflow lines, multiplying it by 4x, then rounding up to the nearest power of two.

Why multiplying it by 4x?

We can use the examples in the spec if we need to:
https://gitweb.torproject.org/torspec.git/tree/bandwidth-file-spec.txt

Longest lines in the examples are 216 characters for sbws (including errors) and 237 characters for Torflow. Nearest power of 2 for 237*4 is 1024.

Maybe we should wait until sbws has added all the new fields?

216 is the number for sbws including all the current planned fields.

comment:2 Changed 5 months ago by nickm

Keywords: fast-fix added
Owner: set to nickm
Status: newaccepted

comment:3 Changed 5 months ago by nickm

Status: acceptedneeds_review

See my branch 'ticket26223', with PR at https://github.com/torproject/tor/pull/219 .

There's a posix-supported replacement for fgets that does what we want here, so it seemed best to me just to remove the upper limit.

comment:4 Changed 5 months ago by teor

Status: needs_reviewneeds_revision

I reviewed the pull request.
We can add a few extra comments if you like, or just merge it as-is.

comment:5 Changed 5 months ago by teor

Reviewer: teor

comment:6 Changed 5 months ago by nickm

Resolution: implemented
Status: needs_revisionclosed

Added comments, retabified configure.ac lists, merged!

Note: See TracTickets for help on using tickets.