dCache / xrootd4j

Implementation of the xrootd data access protocol in Java
Other
3 stars 8 forks source link

xrootd4j: do not overwrite EndSession messages with stored session #118

Closed alrossi closed 2 years ago

alrossi commented 2 years ago

Motivation:

See GitHub Xroot "Session not found" https://github.com/dCache/dcache/issues/6246

For internal purposes, the authentication handler has always added the session associated with the original login to its channel to all other request messages before processing them or passing them down the pipeline. This is so that the sessionId and associated information would be available, since this is not always sent with every message by the client.

However, in the case of EndSession, where the message is sent with the id, it does not make sense to clobber it with the stored session object.

This never got us into trouble before the changes made to the xrdcp client in 5.0+, where it is not necessarily the case that an end session request will be sent on the same channel (i.e., socket) as the original login. When this occurs, it is possible that the original channel has already been closed and its session id removed from the map, resulting in "session not found", when in fact the session which the client is trying to close is not the one stored.

Notwithstanding the change in xrootd client implementation, it still appears that the overwrite in this case is actually a bug, given that the EndSession request is explictly bound to a specific session id.

Modification:

Do not overwrite the message's data structure, but use the id that is sent.

Handle id = all binary zeros (this session).

Do not raise exception for missing or unknown session, but just return OK.

Result:

This may not solve this issue entirely (see testing notes below). But I would deem what the code is doing to be a bug nonetheless.

Target: master Request: 4.2 Request: 4.1 Request: 4.0 Patch: https://rb.dcache.org/r/13264/ Requires-notes: yes Requires-book: no Closes: #6246 Acked-by: Tigran Acked-by: Paul