pkg / sftp

SFTP support for the go.crypto/ssh package
BSD 2-Clause "Simplified" License
1.5k stars 379 forks source link

add support for discarded longname field #496

Closed kexirong closed 2 years ago

kexirong commented 2 years ago

495

kexirong commented 2 years ago

The longname field is equally important to the client @puellanivis #452

kexirong commented 2 years ago

Thank you for correcting my code irregularities. I didn't parse the longname, add it to the Extended field, just no longer discard and implement the get function. There is no other way to get the linkcount field, it is required for interaction, such as cli or gui. My native language is not English, but Chinese.

puellanivis commented 2 years ago

While this is indeed the only way to get the linkcount field, but you are incorrect. It is not required for interactive interfaces like CLI or GUI. Linkcount is rarely useful, and as noted, the very next version of the standard removes access to it entirely. And the standard specifically says that no client should be parsing information from this field anyways.

And as noted, the longname format is not actually standardized. Implementations are technically free to format their longname text differently, and remove the linkcount field entirely. The bug you pointed to was a compatibility bug, caused by a client that was incorrectly parsing the longname. That client never should have been doing that in the first place, but there we were.

As a participant in the wider world of SFTP program compatibility, I do not see any value in exposing this opaque text, and giving any amount of encouragement to people to use it in any way.

kexirong commented 2 years ago

openssh is the most widely used ssh. It doesn't appear to be planning to upgrade the protocol to the next version. And ssh is mainly used by Unix-like systems. Then in my eyes openssh is orthodox, and other protocol versions for compatibility with windows are not business, and other file transfer protocols should be used instead of sftp, so I think as long as the text is in a fixed format, it can be parsed. These are my thoughts on the value of the longname field. And you don't need to use dard link count, so it makes no sense to think it I have no intention of changing your mind. I plan to copy this library in my spare time and implement a client library for sftp compatible with openssh. The above is my translation from Chinese to English using Google.

puellanivis commented 2 years ago

This library is already compatible with openssh. So, I am confused by why you’re making this argument. If openssh is showing this field, it is because it is dumping the raw opaque text for ls, openssh is definitely not parsing this field, and is definitively not extracting the linkcount out of the text.

Right now, this library only targets the -02 version, but it would be poor form to shoot ourselves in the foot and cut off all ability to be interoperable. Note, that when we were not generating this link text properly, we only had one report of an SFTP client that was misbehaving. This demonstrates a fairly widespread consensus that this field is wholly unreliable.

And ignoring all other clients and OSes is unrealistic, and improper. We have to be interoperable, because people expect us to be interoperable. This is not openssh-sftp it is sftp.

drakkan commented 2 years ago

as already noted the specs say:

However, clients SHOULD NOT attempt to parse the longname field for file attributes; they SHOULD use the attrs field instead

so you are doing something not recommended in specs and we don't want to do it here.

Of course you are free to maintain a custom fork with your patch, good luck with your project!