robherley / snips.sh

✂️ passwordless, anonymous SSH-powered pastebin with a human-friendly TUI and web UI
https://snips.sh
MIT License
1.05k stars 45 forks source link

Extra `CR` when getting file over SSH #201

Open andyDoucette opened 2 months ago

andyDoucette commented 2 months ago

What happened? What is expected?

I put a file into snips.sh that had a single line with a \n character at the end (no \r).

On another system, I retrieved the file using ssh.

I compared the sha256 of the source and the retrieved file and they were not the same.

snips.sh seems to have added a \r before the \n. :(

I expect snips.sh not to change my files in any way whatsoever.

How can we reproduce it?

root@staging:/# echo 'hi' | od -a 0000000 h i nl 0000003

root@staging:/# echo 'hi' | ssh snips.sh Pseudo-terminal will not be allocated because stdin is not a terminal. ┃ File Uploaded 📤
┃ id: qbH7FNuWG8
┃ size: 3 B • type: plaintext • visibility: public ┃ SSH 📠
┃ ssh f:qbH7FNuWG8@snips.sh ┃ URL 🔗
https://snips.sh/f/qbH7FNuWG8

root@staging:/# ssh f:qbH7FNuWG8@snips.sh | od -a Connection to snips.sh closed. 0000000 h i cr nl 0000004

Anything else we need to know?

no

robherley commented 1 month ago

👋 I looked into this a bit.

If you download your file over curl, you'll notice that it's fine:

$ curl -s https://snips.sh/f/qbH7FNuWG8 | od -a
0000000    h   i  nl                                                    
0000003

However, the problem does come in when fetching over ssh:

$ ssh f:qbH7FNuWG8@snips.sh | od -a
0000000    h   i  cr  nl                                                
0000004

The SSH library that snips uses, (wish) is built on top of gliderlabs/ssh.

Within that library, I found the following: https://github.com/gliderlabs/ssh/blob/adec695b0aa80b0a03f251e1f8c302f0ea192ef5/session.go#L133-L137

// normalize \n to \r\n when pty is accepted.
// this is a hardcoded shortcut since we don't support terminal modes.
p = bytes.Replace(p, []byte{'\n'}, []byte{'\r', '\n'}, -1)
p = bytes.Replace(p, []byte{'\r', '\r', '\n'}, []byte{'\r', '\n'}, -1)

I'll look into this a bit more.

robherley commented 1 month ago

Looks like wish's ssh fork is doing something slightly different with a PtyWriter: https://github.com/charmbracelet/ssh/blob/eb71b85b27aa2fbecab39bbd5ba2cd6e7f4cab24/session.go#L144-L146

For now, you can disable pty allocation:

$ ssh -T f:qbH7FNuWG8@snips.sh | od -a
0000000    h   i  nl                                                    
0000003
robherley commented 1 month ago

Also updated the issue title to clarify the behavior we're seeing 👍