data (4th) argument of enet_host_connect() is not set in the CONNECT event received on the CLIENT side.

[dennisjenkins75]

(Updated after stand-alone repro showed the the SERVER side sees the value, but the CLIENT side does not.)

The docs for enet_host_connect() state that the final argument, enet_uint32 data is "user data supplied to the receiving host". Sadly when the sending (client) host receives the ENET_EVENT_TYPE_CONNECT event, that data value is NOT set in the ENetEvent->data member; the value is always zero. In fact, none of the data members reachable from the event (such as event->peer->data or event->peer->eventData) have the magic 32-bit value that I specified in my call to enet_host_connect().

My intent was to use this value on the server side to allow the server to reject a client that did not specify the expected 32-bit value. However, my own project uses common code (both the client and server end) to run the ENet "while / switch" event pump.

Confirmed w/ GDB:

// 32-bit value sent from client to server when connecting.
// The server end will reject any client that fails to provide this value.
// This is to reduce errant connection attempts from other ENet clients.
constexpr uint32_t kConnectionSentinal = 0x50564500; // "PVE\000"
...
  std::unique_lock<std::mutex> enet_lock(enet_mutex_);
  enet_host_connect(enet_host_.get(), &address, kMaxChannels, kConnectionSentinal);
...

Thread 3 "unit_tests" hit Breakpoint 1, pve::NetworkDriver::HandleConnect (this=0x7fffffffd5f0, event=0x7ffff6f2bc20) at ../src/lib/common/net_driver.cc:336
336     void NetworkDriver::HandleConnect(ENetEvent *event) {
(gdb) p event
$1 = (ENetEvent *) 0x7ffff6f2bc20
(gdb) p *event
$2 = {type = ENET_EVENT_TYPE_CONNECT, peer = 0x5555558e94f0, channelID = 0 '\000', data = 0, packet = 0x0}

Application logs:

[2022-03-29 21:38:35.262] [warning] NetworkDriver::HandleConnect 127.0.0.1:58676 Client provided invalid sentinal value of 0, expecting 1347831040
<onehittoaster> ../src/lib/common/net_driver_test.cc:90: Failure

I can confirm w/ wireshark that my uint32 is present in the UDP packet going from the client to the server. The last 4 bytes of the outbound packet (wireshark capture is from a different run of the same test, so the UDP port numbers are different, as is the connectionID; but its all the same code):

00000000  8f ff b0 0c 82 ff 00 01  00 00 ff ff 00 00 05 78   ........ .......x
00000010  00 01 00 00 00 00 00 02  00 00 00 00 00 00 00 00   ........ ........
00000020  00 00 13 88 00 00 00 02  00 00 00 02 25 a4 f6 38   ........ ....%..8
00000030  50 56 45 00                                        PVE.
    00000000  80 00 b0 0c 83 ff 00 01  00 00 00 00 00 00 05 78   ........ .......x
    00000010  00 01 00 00 00 00 00 02  00 00 00 00 00 00 00 00   ........ ........
    00000020  00 00 13 88 00 00 00 02  00 00 00 02 25 a4 f6 38   ........ ....%..8
00000034  80 00 b0 0c 01 ff 00 01  00 01 b0 0c 85 ff 00 02   ........ ........
    00000030  00 00 01 ff 00 02 00 02  b0 0c                     ........ ..

[dennisjenkins75]

I suppose that I should have stated what I expected. I expect that 32-bit unsigned integer value of the 4th argument to enet_host_connect() would appear in the event->data on the sending and receiving end with the ENET_EVENT_TYPE_CONNECT event.

[dennisjenkins75]

Update: The data value is present on the SERVER (receiving) side in the CONNECT event ENetEvent::data member. However, it is always ZERO on the CLIENT (sending) side CONNECT event. Technically, this does match the documentation.

However, I suggest (maybe I'll send a PR) that the value be present in the CLIENT CONNECT event also. My own code uses one common "while() / switch (event.type)" block, so server vs client distinction is thin at best.

Repro:

https://gist.github.com/dennisjenkins75/2d8e1cc5591a877583336ddfa62fa758

$ g++ -Wall -O0 -ggdb --std=c++11 -Ienet ./src/utils/enet_demo.cc -o ./enet_demo -lenet
$ ./enet_demo 
ENet version: 66321
ServerThread() entry.
ClientThread() entry.
client Event: type:1, data:0, channel:, addr.port:49999, packet:NULL
CONNECT: event.data = 0 has the wrong value.
server Event: type:1, data:1347831040, channel:, addr.port:58296, packet:NULL
ServerThread() exit.
ClientThread() exit.

[mman]

@dennisjenkins75 I can confirm the behavior, the data parameter is correctly transmitted to the server side, but is not present on the client side.

[dennisjenkins75]

So, I'd like to convert this "issue" from a "bug report" to a "feature request" and ask that the data parameter be set in the CONNECT event on the client side of a connection. The example use-case would be to simplify (eg, not need to special-case handling) of ENetEvent messages. In my use-case, I have one C++ method (in my ENet wrapper) to handle all ENetEvents that is used by both the client-side and server-side. A second use-case would be if ENet were used in a peer-to-peer fashion, where the distinction between client and server is nebulous b/c N peers have N-1 connections to all other peers, or some reduced spanning-tree routing mechanism.

[mman]

@dennisjenkins75 I have looked into the code, and it should be fairly trivial.

Right now it only copies the data supplied to enet_host_connect directly into the outgoing packet, and forgets it. It should probably store it in the user peer's eventData member.

I would add it here: https://github.com/lsalzman/enet/blob/3340d1cf85f2c917eba29d179854c24a31dd37e2/host.c#L212.

Then when the CONNECT command is sent and acknowledged with VERIFY_CONNECT back, the peer eventData is already picked up and passed to the event.

This is (if I understand it right) done here https://github.com/lsalzman/enet/blob/3340d1cf85f2c917eba29d179854c24a31dd37e2/protocol.c#L130.

Note, I have not verified this hypothesis, but it should be trivial to test.

[lsalzman]

The sending end leaves eventData at 0 so that you can distinguish whether you are initiating the connection or whether you are receiving the connection. This is intentional behavior. If you want something on the sending side, then use peer->data.

[mman]

@lsalzman Right, that's a good point, I have worked around that limitation in a similar way but I remember being confused during my early days the same way as @dennisjenkins75. Perhaps this may need a bit of clarification in enet_host_connect documentation?

[dennisjenkins75]

The sending end leaves eventData at 0 so that you can distinguish whether you are initiating the connection or whether you are receiving the connection. This is intentional behavior. If you want something on the sending side, then use peer->data.

Will modifying the peer returned from enet_host_connect() lead to a race condition? I'll need to check the source. My concern is how much work does enet_host_connect() do before returning to its caller?