chrysn / aiocoap

The Python CoAP library
Other
264 stars 119 forks source link

Token variable in Message seems to get ignored. #331

Open javlands opened 10 months ago

javlands commented 10 months ago

When running the CoAP client example, adding a token to the request doesn't seem to influence the actual token used when sending the message (I captured it using Wireshark). After some digging I found that the self.token=random.randint(0, 65535) (line 21) inside the TokenManager class overrides the token. Manually adapting the self.token in TokenManager for each project is a bit cumbersome, so I'm wondering if I'm missing anything?

Thanks in advance!

Python version: 3.10.12 (main, Jun 11 2023, 05:26:28) [GCC 11.4.0]
aiocoap version: 0.4.7
Modules missing for subsystems:
    dtls: missing DTLSSocket
    oscore: missing filelock, ge25519
    linkheader: everything there
    prettyprint: everything there
Python platform: linux
Default server transports:  tcpserver:tcpclient:tlsserver:tlsclient:udp6
Selected server transports: tcpserver:tcpclient:tlsserver:tlsclient:udp6
Default client transports:  tcpclient:tlsclient:udp6
Selected client transports: tcpclient:tlsclient:udp6
SO_REUSEPORT available (default, selected): True, True
chrysn commented 10 months ago

This is intentional, and the fact that the application even gets an opportunity to set a token is more of a vestige of old decisions than intentional. The application usually does not have enough information to set the token, and not all CoAP transports would even have a meaningful token (let alone message Id, for which the same goes) in the first place.

So far my experience has been that every time there have been requests for customization around MID or token, there was an XY problem -- what is it you'd need to set the token for?

javlands commented 10 months ago

Hi

Thanks for the quick response. We are running a client-server implementation where a client can observe a certain resource. When the client no longer needs the observation, it explicitly sends a deregister request. Following the RFC, this request needs to be send using the same token as the initial observe request, hence the need to set a token.

chrysn commented 10 months ago

Cancellation needs to be handled through aiocoap -- otherwise the library will not know whether the observation is still supposed to be active.

Sadly, it's a long-standing shortcoming in aiocoap's client side of observation that it doesn't deregister actively -- mostly due to most application working equally well when instead of one active cancellation, there is an RST or an ICMP error when the next notification is sent instead.

I'll have a look to see if the more recent changes made this substantially easier; if not, this may need to wait for PRs or me to find more time to fix this properly. If this is needed in a commercial project, I can also offer expedited handling as that'd move it to working times (for that, you can reach me directly at c@amsuess.com).

chrysn commented 10 months ago

Looking into the current state I've found that even properly cancelled observations (as in clientGET-observe.py: request.observation.cancel() is necessary as long as the old callback style is supported) would not eventually wind up in a state where RSTs are sent (that then cancel the observation in the server, even if it needs 1-2 extra round trips).

Those are awaiting CI to merge, and will restore what is probably a practical behavior (while not conforming to the observe specification, this is also not putting undue stress on any peer). The full cancellation can be added, but probably needs a day or two to hammer out the corner cases and suitable tests.