[irods/irods#8653] imiscsvrinfo: Do not segfault when connected to 4.3.3 and earlier (main) - #590
Conversation
alanking
left a comment
alanking
left a comment
There was a problem hiding this comment.
While I appreciate that we are trying to accommodate earlier server versions in the iCommands... do we make any compatibility claims between 5.x iCommands and 4.x iRODS servers? Doesn't hurt to have this, of course, but this cross-version use of iCommands is unlikely and possibly officially "unsupported" in 5.x, right? In other words, I wonder if this should only be in 4-3-stable.
We don't have any official wording around compatibility for cross-version icommands yet, but it's in the works and is likely to land with the release of 5.0.2 or 5.1.0. Can cover that as part of irods/irods_docs#388.
You're proposing we catch the incompatibility and report it to the user? That works and likely aligns better with the idea of only allowing changes to packing instructions in major versions. Alternatively, we never change packing instructions once they are made public. We only add new APIs and packing instructions. What are your thoughts on end-users upgrading and getting 5.x icommands and interacting with 4.3 systems? Consider the cases:
|
|
I think I'm just confused about when we try to support backward compatibility and when we just let it go. How do we test this? We are only fixing this one because it causes the client to segfault. What about more subtle/serious issues brought about by compatibility issues? I guess all of this is going to be covered in irods/irods_docs#388, like you said. |
|
I agree. We lack rules around these kinds of challenges. irods/irods_docs#388 is the space for defining those rules. Postponing this work until we've formed a plan around API stability and backward compatibility. |
|
Given that 4.3.5 will include this fix and will be released before 5.1.0, let's go ahead and get this in for 5.1.0. It's a small change that leads to a better set of icommands. We will define concrete rules around compatibility in irods/irods_docs#388. |
|
Squashed. |
|
Holding off on adding the pound until the irods/irods PR is done/approved. |
…3.3 and earlier Before this commit, imiscsvrinfo would segfault when attempting to deserialize the response from a server older than 4.3.4. This was due to 4.3.4 servers and later expecting the byte stream to include data for certificate information. This problem is fixed by overriding the packing instruction used for deserialization of the response. This only applies when connected to a server that's older than 4.3.4.
|
Added pound. |
2 participants
Companion PR at irods/irods#8672.
This change is limited to the main branch and 4-3-stable branch.
Manually tested against 4.3.3. It resolves the segfault issue.