| Subject: | UniqueID is suspicious to race conditions |
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 |
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);
}