Skip Menu |
 

This queue is for tickets about the Net-XMPP CPAN distribution.

Report information
The Basics
Id: 74244
Status: new
Priority: 0/
Queue: Net-XMPP

People
Owner: Nobody in particular
Requestors: whynot [...] pozharski.name
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: 1.02_04
Fixed in: (no value)



Subject: UniqueID is suspicious to race conditions
Download (untitled) / with headers
text/plain 2.7k
Putting long story short: the way Net-XMPP creates 'id' attribs is a disaster waiting to happen. Let me show you a little example. Two entities based on Net-XMPP are talking a custom kinda RPC protocol to each other. '^^>' means packet going through initial-stream, 'vv>' through responce-stream, and '++' isa twilight interserver zone. Side Alpha | | Side Bravo ====================================================================== | ++ | $c->SendAndReceiveWithID( | ++ | $xml ) | ++ | $c->SendXML( | ++ | $c->SendAndReceiveWithID( '<iq type="set" | ++ | $xml ) id="netjabber-95"/>' )^^95^^>\+ | $c->WaitForID( | |+ | $c->SendXML( 'netjabber-95' ) | |+ | '<iq type="get" | |/<^^95^^ id="netjabber-95"/>' ) | || | $c->WaitForID( | \+vv95vv> 'netjabber-95' ) <vv95vv-/ | | ++ | ## [1] ############################################################### | ++ | | ++ | $c->Send( $c->Send( | ++ | '<iq type="error" '<iq type="error" | ++/<^^95^^ id="netjabber-95"/>' ) id="netjabber-95"/>' ) ^^95^^>\| | | || | ## [2] ###########################||################################## | || | <vv95vv+/ | | \-vv95vv> | ++ | ## [3] ############################################################### [1] That's the WTF point. Alpha expects <iq type="result"/> or <iq type="error"/>. And Bravo is no different. [2] At that point both initial-streams become invalid. Packet with 'id="netjabber-95"' was already seen, thus either server can just drop any or both of them. And I think, I'd experienced such spourious timeouts couple of times. [3] Now responce-streams are invalid too. Not to say it's possible that point wouldn't ever be reached. And anyway, now what? Going through 'id="netjabber-96"'? As a bonus, 'proper-ids.diff' enables script to generate its own IDs. The intended purpose of criptic variables in 'sha1_hex' call isn't to make ID random (then 'rand' would be just enough) but to gather as much uniqueness as possible. 'sha1_hex' is in use because Digest::SHA1 is already here. p.s. A tag (namely, 'netjabber') looks somewhat outdated, doesn't it?
Subject: proper-ids.diff
Download proper-ids.diff
text/x-diff 2k
diff -ur backup-1.002004/lib/Net/XMPP/Protocol.pm proper-ids/lib/Net/XMPP/Protocol.pm --- backup-1.002004/lib/Net/XMPP/Protocol.pm 2011-07-19 19:58:40.000000000 +0300 +++ proper-ids/lib/Net/XMPP/Protocol.pm 2012-01-21 01:24:17.000000000 +0200 @@ -148,6 +148,7 @@ $receiveObj = $Con->WaitForID($id); $receiveObj = $Con->WaitForID($id, 20); + $id = $Con->UniqueID(); =head2 Namespace Functions @@ -301,7 +302,8 @@ iq=>function, send=>function, receive=>function, - update=>function) + update=>function, + id=>function) sets the callback functions for @@ -323,6 +325,7 @@ your program while waiting for a packet with an ID to be returned (useful for GUI apps). +id is used to generate an unique ID when such is required. A major change that came with the last release is that the @@ -339,6 +342,8 @@ send and receive get passed in strings. update gets passed nothing, not even the session id. +id get passed in some Net::XMPP touch; +that touch is scalar and unique for each call. If you set the function to undef, then the callback is removed from @@ -626,6 +631,13 @@ id tag and pass it through, so both clients must support the id tag for these functions to be useful. +=item UniqueID + + UniqueID() + +provides reasonable default ID generator. +A script can change ID generation arbitrarily by setting id callback. + =back =head2 Namespace Functions @@ -1596,15 +1608,15 @@ # UniqueID - Increment and return a new unique ID. # ############################################################################### +my $currentID; sub UniqueID { my $self = shift; - my $id_num = $self->{RCVDB}->{currentID}; - - $self->{RCVDB}->{currentID}++; + $currentID++; - return "netjabber-$id_num"; + return $self->{CB}->{id}->($currentID) if exists($self->{CB}->{id}); + return Digest::SHA1::sha1_hex(time, $$, $0, caller, $currentID); }


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.