GETCONF allows zero arguments, contrary to spec

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.

changed milestone to %Tor: 0.4.3.x-final

added actualpoints::0.1 component::core tor/tor milestone::Tor: 0.4.3.x-final points::1 priority::medium resolution::fixed reviewer::teor severity::normal status::closed tor-control tor-spec type::task labels

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

Trac:
Status: new to needs_review

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.

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.

Trac:
Status: needs_review to needs_information

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

Trac:
Status: needs_information to needs_revision

No more sponsor 31. All this tickets remained open after sponsor 31 ended.

Trac:
Sponsor: Sponsor31-can to N/A

https://github.com/torproject/torspec/pull/104

Trac:
Status: needs_revision to needs_review

Thanks!

I suggested a tweak to the wording of your spec patch, so that the text and the syntax match. (And the text explicitly allows zero arguments.)

Did you also want to document that zero keywords returns "250 OK", so that controllers don't think it's an option?

Trac:
Type: defect to task
Milestone: Tor: unspecified to Tor: 0.4.3.x-final
Actualpoints: N/A to 0.1
Status: needs_review to needs_revision

Trac:
Status: needs_revision to needs_review

Trac:
Reviewer: N/A to teor

Looks good, let's merge.

Trac:
Status: needs_review to merge_ready

Looks good to me too; merged to torspec.

Trac:
Status: merge_ready to closed
Resolution: N/A to fixed

closed

changed time estimate to 8h

added 48m of time spent

moved to tpo/core/tor#31081 (closed)