Opened 6 years ago

Closed 6 years ago

#11246 closed defect (fixed)

Count data cells with stream-id 0 as delivered for SENDME purposes

Reported by: nickm Owned by:
Priority: Medium Milestone: Tor: 0.2.5.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: tor-relay, 025-triaged andrea-review-0255
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

From discussions in #8093, I made a branch called "not_8093" which looks like a fine idea to merge.

Its commit message was:

count DATA cells with stream ID 0 as delivered for SENDME purposes

Found while investigating 8093, but probably not the cause of it,
since this bug would result in us sending too few SENDMEs, not in us
receiving SENDMEs unexpectedly.

Bugfix on the fix for 7889, which has appeared in 0.2.4.10-alpha, but
not yet in any released 0.2.3.x version.

Child Tickets

Change History (8)

comment:1 Changed 6 years ago by nickm

Status: newneeds_review

comment:2 Changed 6 years ago by nickm

Keywords: 025-triaged added

comment:3 Changed 6 years ago by andrea

Keywords: andrea-review-0255 added

comment:4 Changed 6 years ago by nickm

Still looks okay, but I would like a review. The branch name is actually "not_bug8093"

comment:5 Changed 6 years ago by andrea

So, if I understand correctly, all we're doing here is moving the log and return down below the circuit window code for the RELAY_COMMAND_DATA case?

I think this is okay, but it makes it possible for connection_edge_process_relay_cell_not_open() to get called in the stream_id == 0 case. I *think* that's harmless, but adding a stream_id != 0 test to that if clause as well may be the slightly more cautious route.

comment:6 Changed 6 years ago by nickm

I've added a check in "not_bug8093", and corrected the bug number in the changes file. Better now?

comment:7 Changed 6 years ago by andrea

Looks merge-worthy to me.

comment:8 Changed 6 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Merged to 0.2.5. Thanks for the review!

Note: See TracTickets for help on using tickets.