Opened 9 years ago

Closed 9 years ago

Last modified 8 years ago

#3407 closed defect (fixed)

connection_printf_to_buf is crazypants bizarre!

Reported by: rransom Owned by: rransom
Priority: Medium Milestone: Tor: 0.2.2.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: tor-client
Cc: nickm Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

connection_printf_to_buf's callers expect it to emit a CRLF iff the format string ends with CRLF; it actually emits a CRLF iff (a) the format string ends with CRLF or (b) the resulting string is over 1023 characters long or (c) the format string does not end with CRLF and the resulting string is 1021 characters long or longer. (I may be wrong about the exact string lengths there.)

connection_printf_to_buf should be made less flaky.

Child Tickets

Change History (7)

comment:1 Changed 9 years ago by rransom

Status: newneeds_review

See bug3407 ( git://git.torproject.org/rransom/tor.git bug3407 ).

comment:2 Changed 9 years ago by rransom

I've pushed another commit to make send_control_event_impl less goofy as well; the changes/ entry for it isn't finished yet, though.

comment:3 Changed 9 years ago by nickm

Hm. At first I was going to suggest that this go into 0.2.3, but it looks like this is actually triggerable: just GETCONF a key that's exactly the right length.

I don't think that send_control_event_impl change is right: it looks like that function actually _did_ ensure that its strings had a CRLF before sending to send_control_event_string, and that some of its callers rely on this feature: Look at conrol_event_general_status and the functions that call it.

comment:4 in reply to:  3 Changed 9 years ago by rransom

Replying to nickm:

Hm. At first I was going to suggest that this go into 0.2.3, but it looks like this is actually triggerable: just GETCONF a key that's exactly the right length.

I don't think that send_control_event_impl change is right: it looks like that function actually _did_ ensure that its strings had a CRLF before sending to send_control_event_string, and that some of its callers rely on this feature: Look at conrol_event_general_status and the functions that call it.

send_control_event_impl only ensures that a string ends with CRLF if it is given a 10061-character-or-longer string.

control_event_general_status calls control_event_status, which ensures that the format string given to send_control_event_impl ends with a CRLF.

comment:5 Changed 9 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Ah, right you are. Merging.

comment:6 Changed 8 years ago by nickm

Keywords: tor-client added

comment:7 Changed 8 years ago by nickm

Component: Tor ClientTor
Note: See TracTickets for help on using tickets.