Opened 2 months ago

Last modified 4 hours ago

#29136 needs_information defect

PT_LOG and PT_STATUS event fields unspecifed

Reported by: atagar Owned by: ahf
Priority: High Milestone: Tor: 0.4.0.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-spec, tor-pt, 040-must
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Recently Tor added PT_LOG and PT_STATUS events to the spec...

https://gitweb.torproject.org/torspec.git/commit/?id=3028cf1
https://gitweb.torproject.org/torspec.git/commit/?id=b38257e

Unfortunately the 'pt-spec.txt section 3.3.5' section they mention does not exist, and in looking around I can't find anything that describes what these event fields are defined as ('PT=' 'TYPE=', 'CONNECT=', etc).

I started to write a stem parser for these but can't continue until this is done (I can't parse events without knowing what fields they include).

David is aware of this and plans to has kindly offered to add the missing info...

22:24 <+atagar> dgoulet: Your control-spec addition to descript PT_LOG and PT_STATUS
                cite a pt-spec section 3.3.4 which does not exist.
22:24 <+atagar> s/descript/describe
22:29 <+atagar> dgoulet: Huh. I'm not spotting anything that lists the keyword
                arguments ('PT=' and 'SEVERITY=') so guess the sections simply
                missing from the spec. I need that for stem support so please
                give me a nudge when the event spec's done. :)
22:59 <+dgoulet> atagar: oh hmmm I'll fix that sorry
23:17 <+atagar> Thanks! Much appreciated. :)

Child Tickets

Change History (6)

comment:1 Changed 2 months ago by atagar

On a side note I'd appreciate it if the control-spec lists all the event fields rather than instructing the reader to see the pt-spec. It's fine to cite the pt-spec for information, but when it comes the basic event format the control-spec should include everything the reader needs.

I watch the control-spec and dir-spec to keep informed about control and directory specification changes. Reshuffling event formatting under other specs makes my life harder, and arguably makes our control-spec incomplete.

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

Replying to atagar:

On a side note I'd appreciate it if the control-spec lists all the event fields rather than instructing the reader to see the pt-spec. It's fine to cite the pt-spec for information, but when it comes the basic event format the control-spec should include everything the reader needs.

I watch the control-spec and dir-spec to keep informed about control and directory specification changes. Reshuffling event formatting under other specs makes my life harder, and arguably makes our control-spec incomplete.

If we duplicate information in control-spec and pt-spec, then they will get out of sync. Here's two options to avoid that:

  1. put the relevant information in control-spec, and point pt-spec to control-spec
  2. put the basic syntax in control-spec, and then say that additional fields may be added, and point to pt-spec for details

Whatever we decide, we should make sure that we are consistent with our other specs.
For example, the control-spec directory GETINFOs have basic info like URLs and descriptions, but the exact format is in dir-spec (like option 2).

comment:3 Changed 8 weeks ago by dgoulet

Keywords: tor-spec tor-pt added
Milestone: Tor: 0.4.0.x-final
Owner: set to dgoulet
Status: newaccepted

comment:4 Changed 4 weeks ago by nickm

Keywords: 040-must added

Marking tickets as 040-must based on triage with dgoulet.

comment:5 Changed 28 hours ago by ahf

Owner: changed from dgoulet to ahf
Status: acceptedassigned

Taking this.

comment:6 Changed 4 hours ago by ahf

Status: assignedneeds_information

I'm not a big fan of specifying the same structures both places, but I think we should extend control spec with information about the SEVERITY of the message in PT_LOG. Then I think that message type would be complete.

For PT_STATUS I think you should wait with doing anything fancy handling for now. We still don't have any PT's that emits anything useful here. If you want to implement some basic handling for this in stem, I suggest you emit an event which contains a dictionary of strings to string and write a parser for the serialization format we use (see tor's kv_line implementation).

atagar: Would an acceptable fix for this ticket be to specify SEVERITY for PT_LOG? As far as I can tell that is the only piece missing in control-spec.txt to be able to implement a PT_LOG handler purely from reading the control-spec.txt itself without ever having a look at the PT specification.

Note: See TracTickets for help on using tickets.