onions/detached and onions/current should return empty lists instead of errors

If you do a GETINFO query on onions/detached or onions/current and there aren't any, a 551 error is returned. Most other things just return an empty list/value in this case and I think these should too.

One consequence of this is that if you ask for onions/current with some other keys and there are no current onions, the whole query will fail.

So GETINFO onions/current should return onions/current= instead of an error.

For example you can try carml cmd getinfo orconn-status onions/current to see this behavior.

changed milestone to %Tor: 0.3.1.x-final

added component::archived/stem controller milestone::Tor: 0.3.1.x-final priority::medium resolution::fixed severity::normal status::closed type::defect version::tor 0.2.7.5 labels

Makes sense to me; a patch would be welcome.

Trac:
Milestone: N/A to Tor: unspecified

I have a patch in this branch:

https://github.com/meejah/tor/compare/ticket-21329-onions-current

Trac:
Milestone: Tor: unspecified to Tor: 0.3.1.x-final
Status: new to needs_review

Looks correct to me!

Trac:
Status: needs_review to merge_ready

Trac:
Keywords: N/A deleted, review-group-16 added

merged!

Trac:
Keywords: review-group-16 deleted, N/A added
Status: merge_ready to closed
Resolution: N/A to implemented

Gonna reopen. Recent push for this broke Stem's integ tests...

======================================================================
FAIL: test_without_ephemeral_hidden_services
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/srv/jenkins-workspace/workspace/stem-tor-ci/test/runner.py", line 126, in wrapped
    return func(self, *args, **kwargs)
  File "/srv/jenkins-workspace/workspace/stem-tor-ci/test/runner.py", line 143, in wrapped
    return func(self, *args, **kwargs)
  File "/srv/jenkins-workspace/workspace/stem-tor-ci/test/integ/control/controller.py", line 569, in test_without_ephemeral_hidden_services
    self.assertEqual([], controller.list_ephemeral_hidden_services())
AssertionError: Lists differ: [] != ['']

Second list contains 1 additional elements.
First extra element 0:


- []
+ ['']

Trac:
Resolution: implemented to N/A
Status: closed to reopened

Looking quickly at Stem's code, I think this is an integration test bug.

That is, I think list_ephemeral_hidden_services should check if it got an empty answer from Tor (similar to other GETINFO empty replies).

That said, I also don't see how that test ever passed since it appears it would re-raise the 552/protocol-error exception that 'getinfo onions/current' does without the patch in this ticket.

Thanks, I'll dig a little more on my end to figure out exactly what about tor's wire response changed and which side the bug lays on. Might not be able to get to this until after the weekend (visiting family, and can no longer build new tor versions on my netbook since it's pretty out of date).

Trac:
Component: Core Tor to Core Tor/Tor

Ok, took a quick peek. Change is the following...

Before:

>>> GETINFO onions/current
551 No onion services of the specified type.

After:

>>> GETINFO onions/current
250-onions/current=
250 OK

Both responses are valid according to the spec and it doesn't specify what's done when no hidden services exist. Stem specifically looks for the old 551 response, hence the discrepancy.

https://gitweb.torproject.org/stem.git/tree/stem/control.py#n2732

I wouldn't mind changing stem to recognize the new behavior but this tor change may break folks who presently call list_ephemeral_hidden_services(). So it's a tradeoff - is this behavior change worth that breakage.

Up to you, Nick.

Maybe the stem list processing could be tweaked?

(That is, why is there a semantic difference between an empty list, and a list containing an empty string? Neither is a valid ephemeral onion service.)

Hi teor. I'm more than happy to change Stem to recognize the new behavior. The point I was raising is that existing Stem releases will have an issue.

Honestly this is a pretty minor bug. It's an edge case for ephemeral hidden services so I don't see this biting many people. I'd be happy to go either way (change tor and risk a minor breakage for stem users, or keep tor as-is).

Nick: Any preference?

atagar: this breaks multi-key GETINFO queries, though; all other "empty things" in the control protocol return an empty-list like that.

So, if you do GETINFO info/names onions/current then you'll get a 551 and the whole thing fails (instead of, for all other cases, getting two answers)...

Gotcha. Personally I find that a compelling reason to go with this change. As mentioned before it's very much an edge case for Stem users and fixing this sounds worth it to me.

Reassigning this to myself to tweak Stem to handle the new responses. Thanks meejah!

Trac:
Component: Core Tor/Tor to Core Tor/Stem

Change pushed - thanks!

https://gitweb.torproject.org/stem.git/commit/?id=c5cef00

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

Trac:
Version: Tor: 0.2.9.9 to Tor: 0.2.7.5

I believe this bug first existed in 0.2.7.5 and commit 915c7438a77edfaf3103b69cb494a4f069a79a0c

closed