Opened 5 weeks ago

Closed 4 weeks ago

#24230 closed defect (fixed)

control: HS_DESC event failed upload sends back the wrong Action

Reported by: dgoulet Owned by: dgoulet
Priority: Medium Milestone: Tor: 0.3.2.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: easy, tor-control, tor-hs
Cc: atagar Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

When a descriptor upload fails, tor sends on the control port a UPLOAD_FAILED event where in fact it should be sending a FAILED event as the action with the reason: UPLOAD_REJECTED or UNEXPECTED.

The control_event_hs_descriptor_upload_failed() is the offending function.

The control spec does not specify at all the Action=UPLOAD_FAILED nor Stem for instance knows about it so this is a problem on tor side.

Child Tickets

Change History (6)

comment:1 Changed 5 weeks ago by dgoulet

Status: assignedneeds_review

See branch: bug24230_032_01.

Not sure this worth a backport according to our policy? Maybe 031. With this, since 027, tor simply reports an unspecified Action for the HS_DESC event and never the right action for a failed HS descriptor upload.

comment:2 Changed 4 weeks ago by nickm

I'm fine merging this. But would it make more sense to update the documentation in control-spec?

comment:3 Changed 4 weeks ago by dgoulet

No spec changes needed, the problem was that the code wasn't doing what the spec specifies.

comment:4 Changed 4 weeks ago by nickm

What I'm wondering is, should we change the spec to match the code instead?

comment:5 in reply to:  4 Changed 4 weeks ago by dgoulet

Replying to nickm:

What I'm wondering is, should we change the spec to match the code instead?

I'm worried about that because Stem is implemented the way the spec does it, not the code. So right now, Stem will never pick up a failed upload event.

And I can only imagine that other control port library are also doing the same. So, updating the code just seems more straight forward. Second, this would mean adding a new Action to the control spec which is a bit more work and nullifying a Reason that is used with the Action=Failed.

comment:6 Changed 4 weeks ago by nickm

Resolution: fixed
Status: needs_reviewclosed

ok. sounds persuasive. merging to maint-0.3.2

Note: See TracTickets for help on using tickets.