Opened 7 months ago

Closed 4 months ago

#26112 closed defect (invalid)

Stem should not use human-readable message of GETCONF 552 response

Reported by: dmr Owned by: atagar
Priority: Low Milestone:
Component: Core Tor/Stem Version:
Severity: Minor Keywords: controller easy
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Both tor and stem use a message format for GETCONF 552 errors that differs from the wording of the control spec's `GETCONF` response.

GETCONF output via tor-prompt:

>>> GETCONF blah
552 Unrecognized configuration key "blah"

Stem expects this format; see `GetConfResponse`:

      unrecognized_keywords = []
      for code, _, line in self.content():
        if code == '552' and line.startswith('Unrecognized configuration key "') and line.endswith('"'):
          unrecognized_keywords.append(line[32:-1])

      if unrecognized_keywords:
        raise stem.InvalidArguments('552', 'GETCONF request contained unrecognized keywords: %s' % ', '.join(unrecognized_keywords), unrecognized_keywords)
      else:
        raise stem.ProtocolError('GETCONF response contained a non-OK status code:\n%s' % self)

My tor version, for reference:

>>> GETINFO version
250-version=0.3.2.10 (git-0edaa32732ec8930)
250 OK

Historical note

In searching (for potential duplicates) to file this ticket, I happened upon the original ticket: #6114

Priority, Severity

I'm marking this as low,minor because it appears the code will work decently well (still raise an exception) if the message format changes.

Expected behavior

In discussion over #tor-dev, it was agreed that stem (and generally any controller) shouldn't rely on a specific message. Only the numeric code 552 should be relied upon.

Child Tickets

Change History (5)

comment:1 Changed 7 months ago by dmr

Related ticket(s)

See also tor-spec ticket #26113 which serves to reduce the ambiguity in the spec about whether a human-readable message is specified.

comment:2 Changed 7 months ago by atagar

Hi dmr. If tor changed its message format then the result for Stem users is that stem.InvalidArguments exceptions become stem.ProtocolError instead.

If there's a unique 5xx status for 'that config option doesn't exist' then great. If not then the message is the only game in town for differentiating errors.

comment:3 Changed 7 months ago by dmr

Keywords: controller added; control removed

Slight change to the keyword.

comment:4 Changed 7 months ago by dmr

Keywords: controller, easycontroller easy

For stem, keywords are conventionally separated by a space ( ) only, no comma (,).

comment:5 Changed 4 months ago by atagar

Resolution: invalid
Status: newclosed

If Tor doesn't want us to parse exception messages then it needs to provide unique error codes for its different errors. Resolving in favor of...

https://trac.torproject.org/projects/tor/ticket/27034#comment:4

Note: See TracTickets for help on using tickets.