Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

scan.go: scan() shall support adjacent quoting #267

Closed
dumblob opened this issue Dec 11, 2019 · 11 comments
Closed

scan.go: scan() shall support adjacent quoting #267

dumblob opened this issue Dec 11, 2019 · 11 comments
Labels

Comments

@dumblob
Copy link

dumblob commented Dec 11, 2019

Currently it's impossible to write e.g. :select 'my'""'file' (not even :select 'my''file') to select the file myfile. This is needed for safe handling of files containing either control characters (see scan.go: scan(): case s.chr == '"': ... for the full list of them) or ' or both in their names.

I stumbled upon this when writing a safe lf command like this:

# won't work for files with ' in their name
cmd c1 &{{ lf -remote "send $id select '$f'" }}
@gokcehan
Copy link
Owner

@dumblob Quotes in lf are implemented differently than shell. You can use ' by quoting it in between " and control characters by escaping them with octal escapes between double quotes.

@dumblob
Copy link
Author

dumblob commented Dec 18, 2019

Yeah, I saw the implementation, but I'm unable to find a way for the described issue. It also doesn't sound right to me to list all special cases (e.g. escape all control characters lf understands) by doing something like

cmd c1 &{{ lf -remote 'send '"$id"' select "'"$(sed -Ez -e 's|('"'"')|"\1"|g' -e 's|(\\)|"134"|g' -e '...for_each_ctrl_char' <<END
$f
END
)"'"' }}

(not even sure the code above will work)

as that's really not nice and in addition it's fragile (e.g. if lf will change it's behavior in this regards even slightly, there'll be a bug in the sed regex).

@gokcehan
Copy link
Owner

@dumblob How does concatenating strings solve the example you provided?

@dumblob
Copy link
Author

dumblob commented Dec 20, 2019

One can easily guarantee (in a forward compatible manner) no control character nor any quote will get interpreted by something like:

cmd c1 &{{ lf -remote "send $id select '$( sed -Ez "s|'|'\"'\"'|g" <<END
$f
END
printf "'"  # this is to overcome subshell removing all trailing newlines (in case filename has trailing newline(s)
)" }}

I.e. filename abc' def will be transformed into'abc'"'"' def'.

@gokcehan
Copy link
Owner

@dumblob I still don't understand. You only escaped ' and did not do anything about control characters. If you want to escape ', you can already escape it within " with the current implementation.

@dumblob
Copy link
Author

dumblob commented Dec 27, 2019

You only escaped ' and did not do anything about control characters.

The point is, that if one is inside of '' block, there are no control characters at all. So one needs to escape only the ' character itself. Or am I missing something?

@gokcehan
Copy link
Owner

gokcehan commented Jan 9, 2020

@dumblob What about the control characters then?

@dumblob
Copy link
Author

dumblob commented Jan 9, 2020

Inside of anything enclosed in single apostrophes ('), there are no control characters. That's how I read:

lf/scan.go

Lines 260 to 268 in 3c5abbb

case s.chr == '\'':
s.next()
beg := s.off
for !s.eof && s.chr != '\'' {
s.next()
}
s.typ = tokenIdent
s.tok = string(s.buf[beg:s.off])
s.next()

@gokcehan
Copy link
Owner

@dumblob Weren't you trying to handle control characters in filenames? That is what you told in the beginning of the issue. If that's the case, why are you trying this with single quotes rather than double quotes?

@dumblob
Copy link
Author

dumblob commented Jan 10, 2020

I meant control characters lf recognizes, but not control characters the terminal (emulator) or whatever else recognizes.

@gokcehan gokcehan closed this as completed Jul 8, 2020
@dumblob
Copy link
Author

dumblob commented Jul 21, 2020

Is there any better way than to support all currently implemented lf control characters as outlined above?

Let me demonstrate the issue with pure Go code.

s := "/path/with'so me/sep\" arat e's ingle'and double\" qu otes \"and arbitrary spaces"
//cmd := exec.Command( "lf", "-remote", "send 1234 select " + s )  // wrong
//cmd := exec.Command( "lf", "-remote", "send 1234 select '" + s + "'" )  // also wrong (actually impossible to achieve with single quotes at all)
//cmd := exec.Command( "lf", "-remote", "send 1234 select \"" + s + "\"" )  // also wrong (escaping of " and all control sequences is missing)

s = strings.ReplaceAll( s, "\\", "\\\\" )
s = strings.ReplaceAll( s, "\"", "\\\"" )
cmd := exec.Command( "lf", "-remote", "send 1234 select \"" + s + "\"" )  // finally right

With my proposal above this would be simpler and more forward compatible (I can imagine lf escape sequences changing or being extended, but I can't imagine the adjacent functionality being removed or changed):

s = strings.ReplaceAll( s, "'", "'\"'\"'" )
cmd := exec.Command( "lf", "-remote", "send 1234 select '" + s + "'" )

Shouldn't this issue stay open?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants