Opened 4 months ago

Last modified 6 weeks ago

#29432 new defect

QuotedString and CString in control-spec.txt technically require escaping ascii 32 (space)

Reported by: dcf Owned by:
Priority: Medium Milestone: Tor: unspecified
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-spec 041-proposed
Cc: catalyst Actual Points:
Parent ID: Points:
Reviewer: Sponsor:


control-spec.txt 2.1 says:

2.1. Description format

We use the following nonterminals from RFC 2822: atom, qcontent
QuotedString = DQUOTE *qcontent DQUOTE

2.1.1. Notes on an escaping bug

CString = DQUOTE *qcontent DQUOTE

RFC 2822 defines qcontent thus:

qtext           =       NO-WS-CTL /     ; Non white space controls

                        %d33 /          ; The rest of the US-ASCII
                        %d35-91 /       ;  characters not including "\"
                        %d93-126        ;  or the quote character

qcontent        =       qtext / quoted-pair

where NO-WS-CTL expands to

NO-WS-CTL       =       %d1-8 /         ; US-ASCII control characters
                        %d11 /          ;  that do not include the
                        %d12 /          ;  carriage return, line feed,
                        %d14-31 /       ;  and white space characters

In short, qcontent does not include the space character (ascii 32), and so according to a strict reading of the spec, anything that produces a QuotedString or CString has to escape spaces as \ or \040.

The reason why RFC 2822 does not require space to be escaped is that the definition of quoted-string is not DQUOTE *qcontent DQUOTE as in control-spec.txt, but also allows whitespace as part of the [FWS] production:

quoted-string   =       [CFWS]
                        DQUOTE *([FWS] qcontent) [FWS] DQUOTE

I notice that tor doesn't escape the space character (in esc_for_log and unescape_string for example). IMO tor is doing the right, expected thing and the spec should be clarified.

Child Tickets

Change History (4)

comment:1 Changed 3 months ago by nickm

Keywords: 041-proposed added

I'd be happy to take a patch here; would anybody like to write one?

comment:2 Changed 3 months ago by nickm

(I agree that a spec patch is the correct fix here)

comment:3 Changed 7 weeks ago by nickm

Milestone: Tor: unspecified

comment:4 Changed 6 weeks ago by catalyst

Cc: catalyst added
Note: See TracTickets for help on using tickets.