Opened 3 months ago

Last modified 2 months ago

#31081 needs_revision defect

GETCONF allows zero arguments, contrary to spec

Reported by: catalyst Owned by:
Priority: Medium Milestone: Tor: unspecified
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-control, tor-spec
Cc: Actual Points:
Parent ID: Points: 1
Reviewer: Sponsor: Sponsor31-can

Description

control-spec.txt says GETCONF takes at least one argument. Tor currently allows there to be zero arguments for GETCONF. If there are zero arguments, it replies with 250 OK. A reply with a keyword and no = can mean that a configuration variable has its default value. We've therefore committed to having no configuration variables named OK. I suppose that's OK.

Probably the easiest thing to do is to document the existing behavior.

Child Tickets

Change History (5)

comment:1 Changed 3 months ago by rl1987

Status: newneeds_review

I propose rejecting GETCONF commands with no arguments as they don't make sense semantically. The patch turned out to be trivial.

https://github.com/torproject/tor/pull/1166

comment:2 in reply to:  1 ; Changed 3 months ago by teor

Replying to rl1987:

I propose rejecting GETCONF commands with no arguments as they don't make sense semantically. The patch turned out to be trivial.

https://github.com/torproject/tor/pull/1166

We talked about rejecting GETCONF with no arguments, but we decided it was easier to update the spec. We were concerned that we might break controllers that depend on the incorrect behaviour. And it seems like such a trivial issue to risk breaking a controller.

We can talk about it again at our next team meeting, if you'd like.

If we do decide to reject GETCONF with no arguments, we'll also need to change the spec to document the versions that (mistakenly) accepted no arguments.

comment:3 in reply to:  2 Changed 3 months ago by catalyst

Replying to teor:

We talked about rejecting GETCONF with no arguments, but we decided it was easier to update the spec. We were concerned that we might break controllers that depend on the incorrect behaviour. And it seems like such a trivial issue to risk breaking a controller.

We can talk about it again at our next team meeting, if you'd like.

If we do decide to reject GETCONF with no arguments, we'll also need to change the spec to document the versions that (mistakenly) accepted no arguments.

We should figure out which stakeholders we should consult about this behavior change, because I think not all of them regularly attend our team meetings.

comment:4 Changed 2 months ago by dgoulet

Status: needs_reviewneeds_information

comment:5 Changed 2 months ago by catalyst

Status: needs_informationneeds_revision

We talked about it at the 2019-07-08 dev meeting. It's probably safest to document the existing behavior in the spec.

Note: See TracTickets for help on using tickets.