Opened 6 years ago

Closed 5 years ago

#5637 closed defect (fixed)

Ignore carriage returns when parsing descriptors

Reported by: karsten Owned by: karsten
Priority: Low Milestone:
Component: Metrics Utilities Version:
Severity: Keywords:
Cc: atagar Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

There are some (very old) server descriptors containing carriage returns in their contact line, more precisely in a full PGP key block. I'm attaching one such descriptor to this ticket.

It's unclear if carriage returns are permitted by dir-spec.txt. It does say "ArgumentChar ::= any printing ASCII character except NL.", but we know that Tor accepts non-ASCII characters in contact or platform lines, too. So, I guess the only non-permitted character is NL.

In metrics-lib, we parse descriptors using BufferedReader.readLine() which treats \n, \r, and \r\n all the same. We may have to write our own readLine() replacement that only accepts \n as line end.

This may also affect stem's descriptor parser.

Child Tickets

Attachments (1)

def5878c5fe864cbe48510e85327e1d30f7aa971 (4.2 KB) - added by karsten 6 years ago.
Server descriptor from 2007-09 that contains carriage returns in the contact line

Download all attachments as: .zip

Change History (4)

Changed 6 years ago by karsten

Server descriptor from 2007-09 that contains carriage returns in the contact line

comment:1 Changed 6 years ago by atagar

This may also affect stem's descriptor parser.

Nice catch and ewww that's gross. I just tried running stem's descriptor parser over it and it got the right value (retaining the '\r'). Interesting use case though - I'll add a test for it.

comment:2 Changed 6 years ago by atagar

Test added...
https://gitweb.torproject.org/stem.git/commitdiff/725beac5437d85e8da5baa8a6fce90ce76fa9f9f

Btw, you left a comment a while back suggesting a factory descriptor constructor for testing like metrics-lib. Stem actually does that too...
https://gitweb.torproject.org/stem.git/blob/HEAD:/test/unit/descriptor/server_descriptor.py#l35

In general I favor running against an actual descriptor when we've seen one in the wild, and using the factory when we haven't.

Cheers! -Damian

comment:3 Changed 5 years ago by karsten

Resolution: fixed
Status: newclosed

Fixed in metrics-lib. Closing.

Note: See TracTickets for help on using tickets.