Opened 10 years ago

Closed 10 years ago

#3933 closed Bug (fixed)

announcer.c peer parsing could be simpler

Reported by: jordan Owned by: jordan
Priority: Normal Milestone: 2.20
Component: libtransmission Version: 2.13
Severity: Normal Keywords:
Cc:

Description (last modified by jordan)

Two related issues:

(1) there's duplication of code between announcer.c's peer parsing functions and peer-mgr.c's tr_peerMgrCompactToPex() and tr_peerMgrCompact6ToPex(). The code in announcer.c could use peer-mgr.c for parsing compact ipv4 and ipv5 lists.

(2) As reported in the comments of #3931, parseOldPeers() loses some of the peers off the end of an old-style benc peer list:

array = tr_new( uint8_t, peerCount * ( sizeof( tr_address ) + 2 ) );
...
*byteCount = peerCount * sizeof( tr_address ) + 2;

Note the order of precedence between the * and + in the two lines.

Actually, even using the parenthesis in the latter line would still be wrong, because the resulting array can be shorter if part of the incoming benc list had any corrupt peers.

Change History (8)

comment:1 Changed 10 years ago by jordan

  • Description modified (diff)
  • Status changed from new to assigned

comment:2 Changed 10 years ago by jordan

...actually there's a lot of redundancy between the peer list parsing code in announcer.c and in peer-mgr.c's tr_peerMgrCompactToPex() and tr_peerMgrCompact6ToPex() functions. Maybe the former should use the latter for the parsing.

comment:3 Changed 10 years ago by jordan

  • Description modified (diff)
  • Summary changed from announcer.c drops some peers from old-style non-compact peer lists to announcer.c peer parsing could be simpler

comment:4 Changed 10 years ago by jordan

  • Keywords backport-2.0x backport-2.1x removed

comment:5 Changed 10 years ago by jordan

  • Resolution set to fixed
  • Status changed from assigned to closed

Fixed in r11750

comment:6 follow-up: Changed 10 years ago by gunzip

you may have undersold this ticket as it appears to have solved a rather obscure but nevertheless nagging problem i've previously had announcing & connecting to a certain bittorrent client/version. this fix appears to have solved it just when i had given up hope of ever figuring what was going wrong.

comment:7 in reply to: ↑ 6 ; follow-up: Changed 10 years ago by gunzip

  • Resolution fixed deleted
  • Status changed from closed to reopened

Replying to myself:

you may have undersold this ticket as it appears to have solved a rather obscure but nevertheless nagging problem i've previously had announcing & connecting to a certain bittorrent client/version. this fix appears to have solved it just when i had given up hope of ever figuring what was going wrong.

this problem is returning for me in the latest nightlies. the certain bittorrent client i had trouble with was libTorrent (Rakshasa) 0.12.7 , it wouldn't respond to my tracker announces and would never connect. after r11750 everything was fine and had no problems with 2.20, 2.21 and 2.22.

i'm guessing, based on the timing of the changes, it may be due to r12120 / r12121 as i noticed this problem cropping up again.

comment:8 in reply to: ↑ 7 Changed 10 years ago by gunzip

  • Resolution set to fixed
  • Status changed from reopened to closed

Replying to myself:

this problem is returning for me in the latest nightlies. the certain bittorrent client i had trouble with was libTorrent (Rakshasa) 0.12.7 , it wouldn't respond to my tracker announces and would never connect. after r11750 everything was fine and had no problems with 2.20, 2.21 and 2.22.

i'm guessing, based on the timing of the changes, it may be due to r12120 / r12121 as i noticed this problem cropping up again.

did several tests with r12207 and i'm no longer seeing the issue re libTorrent (Rakshasa) 0.12.7, and i can connect to it in seconds now. don't know what changed, but since i'm the only one who ever reported this, and it's no longer occuring, i should re-close this ticket.

i'm guessing that the many changes/bug-fixes resolved the issue for me.

Note: See TracTickets for help on using tickets.