Main opts with space broken apart

Question

{code:title=src/foo/core.clj}
(ns foo.core)

(defn -main [& args]
  (prn args))



$ clj -Srepro -Sdeps '{:aliases {:space {:main-opts ["-m" "foo.core" "-co" "{:aot-cache true}"]}}}' -Aspace
("-co" "{:aot-cache" "true}")


Expectation is that there be two options passed to {{foo.core/-main}}, not three.

Answer 1

Comment made by: mfikes

See https://dev.clojure.org/jira/browse/TDEPS-72?focusedCommentId=48946&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-48946

Answer 2

Comment made by: pesterhazy

Attaching two patches, one for the "brew install" repository and one for the "tools deps" repository.

The approach taken is to quote arguments written to the .main file using backslashes, e.g.:

\-\-n\a\m\e \J\o\h\n\ \S\n\o\w

The bash portion of the change unquotes the string using eval, splitting it into multiple arguments.

Both patches need to be applied simultaneously.

Answer 3

_Comment made by: pesterhazy_

With the patch applied, the example properly prints


("-co" "{:aot-cache true}")


FWIW I found the two helpers scripts "try" and "demo" useful for testing changes involving brew-install and tools.deps.alpha: https://github.com/pesterhazy/brew-install/pull/1/files#diff-d67b65744525f3bb15715e95ba3117eb

Answer 4

Comment made by: alexmiller

Can you discuss any downsides to the quoting approach or use of eval?

Are there other options you ran into for handling this?

Answer 5

Comment made by: alexmiller

Also, if users have existing (non-backslashed) cached main files, what is their experience? That is, is it backwards-compatible with existing cached files?

Answer 6

Comment made by: alexmiller

It seems like there are significant security downsides to using eval to read arbitrary user files (that could be manipulated between the two points) in this way: http://mywiki.wooledge.org/BashFAQ/048 so I would prefer to find an alternative here.

Answer 7

Comment made by: pesterhazy

Re: alterantives, the Bash code needs to be able to read the .main file and split the contents into individual arguments. Ideally this should be done with Bash builtins only, i.e. without spwaning another process.

Parsing JSON, or EDN, is not possible with pure Bash. Another possibility would be to separate arguments by newline. But the Bash code to read that back into an array would not be trivial, or at least less trivial than using eval to parse the contents.

Using the eval builtin has the potential downside that, like the Clojure reader, the Bash reader can have side effects. For example, when the contents contain $(rm /important/file) or >/important/file.

However, we control the code that writes the file and escape all characters. So the dollar and larger-than characters would be prefixed with a backslash. I think that's safe.

I'll have to think about backward/forward compatibility - good point.

Answer 8

Comment made by: pesterhazy

A simple solution to the backward compatibility problem would be to use a different suffix like .qmain, or to start the file with a magic string like 1. quoted.

Answer 9

Comment made by: pesterhazy

Another option is to check the input before eval using

[[ "\\a\\b\\c \\d" =~ ^(\\.| )*$ ]]

Answer 10

Comment made by: arichiardi

Very useful scripts indeed (I mean try and demo)

Answer 11

Comment made by: pesterhazy

As an alternative to eval, the Bash code can read the string into an array manually, character by character. Arguments need to be separated by a delimiter.

Three possible delimiters come to mind:

• NUL: won't work because readarray is not available in Bash3 (mandated by macOS) and NUL can't be stored in Bash variables
• space
• newline

As newlines are less likely to appear in main opts, that would be my choice. However, newlines might still occur so they should be escaped by the Clojure code using a backslash. In addition, literal backslashes need to be escaped as well.

I've done a proof of concept that this works with Bash3: https://github.com/clojure/brew-install/pull/3/files#diff-b3212e45e19f61de4754a755466b793f

If that sounds right, I'll update the patch.

Answer 12

Comment made by: dottedmag

I have attached {{brew-install-nul-terminated.patch}} and {{tda-nul-terminated.patch}} that change format of {{.main}} file to have args NUL-terminated rather than space-separated.

Backward compatibility:
- the patches changes cache ids, so caches with old {{.main}} format won't be reused
- NUL characters in {{:main-opts}} previously were treated inconsistently: under Bash < 4.3 (e.g. MacOS) the remainder of the argument after NUL character was skipped, under Bash >= 4.3 (e.g. fresh Linux) NUL character was skipped, so we're not breaking anything here by reusing NUL as an argument delimiter.

OS compatibility:
- Bash >= 2.04 (released ca. 2000, all relevant OSes have newer Bash)

Alternative approaches:

• Escaping spaces instead of NUL-terminating lines
  -- Upsides: none?
  -- Downsides: Bash parsing code becomes much more convoluted (unless one resorts to {{eval}} with its associated issues)
• Using {{.main2}} files instead of changing hashes for existing cache entries
  -- Upsides: existing cache entries that don't need a {{.main}} file can be reused after upgrade
  -- Downsides: additional convoluted logic in {{clojure}} is needed to detect cache entries with {{.main}} file, treat them as stale, and remove old {{.main}} files after refreshing the cache.

Answer 13

Reference: https://clojure.atlassian.net/browse/TDEPS-56 (reported by mfikes)