Don't strip HT when pasting in non-bracketed mode #1084

Closed
opened 6 months ago by dnkl · 8 comments
dnkl commented 6 months ago
Owner

Foot is stripping a number of control characters when pasting text. Some of them are skipped only when bracketed paste hasn't been enabled (HT being one of them): 377997be9d/selection.c (L1848-L1855)

This behavior was based on XTerm's documented default behavior. However, it seems XTerm, as well as every other terminal I've tested, does allow HT in non-bracketed mode.

Foot is stripping a number of control characters when pasting text. Some of them are skipped only when bracketed paste hasn't been enabled (HT being one of them): https://codeberg.org/dnkl/foot/src/commit/377997be9d9e805ea9a02fc3e82a0b8f2939d7c4/selection.c#L1848-L1855 This behavior was based on XTerm's **documented** default behavior. However, it seems XTerm, as well as every other terminal I've tested, does allow HT in non-bracketed mode.
dnkl added the
bug
label 6 months ago
Poster
Owner

#312 mentions that XTerm does in fact filter HT, but instead of stripping it, it's replaced with spaces.

Still, should be safe to simply allow HT?

https://codeberg.org/dnkl/foot/pulls/312 mentions that XTerm does in fact filter HT, but instead of stripping it, it's replaced with spaces. Still, should be safe to simply allow HT?
Poster
Owner

I'm starting to question the facts in #312. If we do e.g printf '\v\f' | wl-copy -p and then paste into cat - in XTerm, then we see ^K^L. I.e. not spaces.

Perhaps we should just allow all the formatting characters we're currently stripping out in non-bracketed mode?

I'm starting to question the facts in #312. If we do e.g `printf '\v\f' | wl-copy -p` and then paste into `cat -` in XTerm, then we see `^K^L`. I.e. **not** spaces. Perhaps we should just allow **all** the formatting characters we're currently stripping out in non-bracketed mode?
Poster
Owner

Foot's behavior was probably based on XTerm's disallowedPasteControls resource:

     disallowedPasteControls (class DisallowedPasteControls)
               The allowPasteControls resource is normally used to prevent pasting C1 controls, as well
               as non-formatting C0 controls such as the ASCII escape character.  Those characters are
               simply ignored.  This resource further extends the set of control characters which
               cannot be pasted, converting each into a space.

               The resource value is a comma-separated list of names.  Xterm ignores capitalization.
               The default value is

                   BS,DEL,ENQ,EOT,ESC,NUL

               The names are listed below:

               C0   all ASCII control characters.

               Individual C0 characters
                    NUL, SOH, STX, ETX, EOT, ENQ, ACK, BEL, BS, HT, LF, VT, FF, CR, SO, SI, DLE, DC1,
                    DC2, DC3, DC4, NAK, SYN, ETB, CAN, EM, SUB, ESC, FS, GS, RS, US

               DEL  ASCII delete

               NL   ASCII line-feed, i.e., “newline” is the same as LF.

However, as can be seen, HT is not in the default list (as of XTerm-372). Unsure if XTerm's default has changed since foot's implementation was done, or if I misread the documentation.

Foot's behavior was probably based on XTerm's `disallowedPasteControls` resource: ``` disallowedPasteControls (class DisallowedPasteControls) The allowPasteControls resource is normally used to prevent pasting C1 controls, as well as non-formatting C0 controls such as the ASCII escape character. Those characters are simply ignored. This resource further extends the set of control characters which cannot be pasted, converting each into a space. The resource value is a comma-separated list of names. Xterm ignores capitalization. The default value is BS,DEL,ENQ,EOT,ESC,NUL The names are listed below: C0 all ASCII control characters. Individual C0 characters NUL, SOH, STX, ETX, EOT, ENQ, ACK, BEL, BS, HT, LF, VT, FF, CR, SO, SI, DLE, DC1, DC2, DC3, DC4, NAK, SYN, ETB, CAN, EM, SUB, ESC, FS, GS, RS, US DEL ASCII delete NL ASCII line-feed, i.e., “newline” is the same as LF. ``` However, as can be seen, `HT` is **not** in the default list (as of XTerm-372). Unsure if XTerm's default has changed since foot's implementation was done, or if I misread the documentation.
Yeah, XTerm's default changed in 362d, see https://github.com/ThomasDickey/xterm-snapshots/commit/1283a4ce9a1c76a8525419b4a4dffc8ed3e339a2#diff-af7f818c3ce8154b67394975acd5cbcef2a44a898f71b4fedb647950a360f097L31291
Poster
Owner

So, let's see, what should we change in foot:

  • Allow HT, regardless of mode.
  • We're not filtering out NUL, at all (i.e. it's allowed in both bracketed and non-bracketed mode). Should we start stripping it too?
  • ENQ/EOT/ESC is already ignored in foot, regardless of mode
  • DEL is already ignored in non-bracketed mode

(there's an intentional difference between XTerm and foot; where XTerm replaces characters with spaces, foot strips them in non-bracketed mode, but allows them as-is in bracketed mode).

So, let's see, what should we change in foot: * Allow HT, regardless of mode. * We're not filtering out NUL, at all (i.e. it's allowed in both bracketed and non-bracketed mode). Should we start stripping it too? * ENQ/EOT/ESC is already ignored in foot, regardless of mode * DEL is already ignored in non-bracketed mode (there's an intentional difference between XTerm and foot; where XTerm replaces characters with spaces, foot strips them in non-bracketed mode, but allows them as-is in bracketed mode).
Poster
Owner

Then there's VT and FF. Foot is currently ignoring these in non-bracketed mode, since they are formatting characters that potentionally could be used to trick the user. Should we keep doing that, or allow them, like XTerm does?

Then there's VT and FF. Foot is currently ignoring these in non-bracketed mode, since they are formatting characters that potentionally could be used to trick the user. Should we keep doing that, or allow them, like XTerm does?

If my wayland clipboard has any NUL character, then terminals already refuse to paste anything, so I don't have a case where it would make a difference. I guess it's different on X.

No strong opinion about VT and FF. I guess they can trick users into pasting a command they didn't intend to run? Not sure.

If my wayland clipboard has any NUL character, then terminals already refuse to paste anything, so I don't have a case where it would make a difference. I guess it's different on X. No strong opinion about VT and FF. I guess they can trick users into pasting a command they didn't intend to run? Not sure.
Poster
Owner

Digging through the old issues and PRs related to this, I found this, which I remember was also a source we used when deciding on which control sequences to filter out: https://security.stackexchange.com/questions/39118/how-can-i-protect-myself-from-this-kind-of-clipboard-abuse/52655#52655

Here, \t is mentioned being very dangerous as it can trigger completions. But, afaik, all shells now enables bracketed paste, meaning \t wont trigger completions. So I still think it should be safe to follow suite, and allow \t in non-bracketed mode. As far as I can see, it can't really be used to make pasted text look like something it's not.

It doesn't mention \v and \f though. Still not sure where we got those from. But I'm thinking we should allow them as well, for the same reason; it'll move the cursor, but can't really be used to "hide" text.

Digging through the old issues and PRs related to this, I found this, which I remember was also a source we used when deciding on which control sequences to filter out: https://security.stackexchange.com/questions/39118/how-can-i-protect-myself-from-this-kind-of-clipboard-abuse/52655#52655 Here, `\t` is mentioned being very dangerous as it can trigger completions. But, afaik, all shells now enables bracketed paste, meaning `\t` wont trigger completions. So I still think it should be safe to follow suite, and allow `\t` in non-bracketed mode. As far as I can see, it can't really be used to make pasted text look like something it's not. It doesn't mention `\v` and `\f` though. Still not sure where we got those from. But I'm thinking we should allow them as well, for the same reason; it'll move the cursor, but can't really be used to "hide" text.
dnkl closed this issue 5 months ago
Sign in to join this conversation.
No Milestone
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: dnkl/foot#1084
Loading…
There is no content yet.