* [PATCH] ob-maxima.el: Fix execution on MS Windows
@ 2021-12-26 20:18 Nikolay Kudryavtsev
2021-12-29 17:05 ` Max Nikulin
0 siblings, 1 reply; 8+ messages in thread
From: Nikolay Kudryavtsev @ 2021-12-26 20:18 UTC (permalink / raw)
To: emacs-orgmode
[-- Attachment #1: Type: text/plain, Size: 423 bytes --]
Hello.
I've been playing around with Maxima from time to time.
Ob-maxima currently does not work on Windows due to it using single
quotes in the Maxima invocation and those not being supported by Windows
CMD.
After some testing I've found an invocation that seems to work fine on
both Windows and Linux. I don't think this patch can cause any real
issue, since the string in those quotes is just the temp file path.
[-- Attachment #2: 0001-ob-maxima.el-Fix-execution-on-MS-Windows.patch --]
[-- Type: text/plain, Size: 1041 bytes --]
From b0206d14df8947c831e56eef21fbd38ca88c1fd5 Mon Sep 17 00:00:00 2001
From: Nikolay Kudryavtsev <nikolay.kudryavtsev@gmail.com>
Date: Sun, 26 Dec 2021 22:47:19 +0300
Subject: [PATCH] ob-maxima.el: Fix execution on MS Windows
* ob-maxima.el (org-babel-execute:maxima): Change command line
invocation to a one that should work everywhere.
---
lisp/ob-maxima.el | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lisp/ob-maxima.el b/lisp/ob-maxima.el
index 7b49bb07a..512712221 100644
--- a/lisp/ob-maxima.el
+++ b/lisp/ob-maxima.el
@@ -77,7 +77,7 @@ This function is called by `org-babel-execute-src-block'."
(result
(let* ((cmdline (or (cdr (assq :cmdline params)) ""))
(in-file (org-babel-temp-file "maxima-" ".max"))
- (cmd (format "%s --very-quiet -r 'batchload(%S)$' %s"
+ (cmd (format "%s --very-quiet -r \"batchload(\\\"%S\\\")\"$ %s"
org-babel-maxima-command in-file cmdline)))
(with-temp-file in-file (insert (org-babel-maxima-expand body params)))
(message cmd)
--
2.34.1.windows.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] ob-maxima.el: Fix execution on MS Windows
2021-12-26 20:18 [PATCH] ob-maxima.el: Fix execution on MS Windows Nikolay Kudryavtsev
@ 2021-12-29 17:05 ` Max Nikulin
2021-12-29 18:37 ` Nikolay Kudryavtsev
0 siblings, 1 reply; 8+ messages in thread
From: Max Nikulin @ 2021-12-29 17:05 UTC (permalink / raw)
To: emacs-orgmode
On 27/12/2021 03:18, Nikolay Kudryavtsev wrote:
>
> Ob-maxima currently does not work on Windows due to it using single
> quotes in the Maxima invocation and those not being supported by Windows
> CMD.
>
> After some testing I've found an invocation that seems to work fine on
> both Windows and Linux. I don't think this patch can cause any real
> issue, since the string in those quotes is just the temp file path.
> --- a/lisp/ob-maxima.el
> +++ b/lisp/ob-maxima.el
> @@ -77,7 +77,7 @@ This function is called by `org-babel-execute-src-block'."
> (result
> (let* ((cmdline (or (cdr (assq :cmdline params)) ""))
> (in-file (org-babel-temp-file "maxima-" ".max"))
> - (cmd (format "%s --very-quiet -r 'batchload(%S)$' %s"
> + (cmd (format "%s --very-quiet -r \"batchload(\\\"%S\\\")\"$ %s"
> org-babel-maxima-command in-file cmdline)))
I do not like original variant, but suggested change makes it unsafe in
more cases. `in-file' might contain apostrophe in the case of peculiar
path of the directory for temporary files. More characters may be
interpreted by BASH inside double quotes. Even docstring for
`shell-quote-argument' mentions security issues with the function.
Ideally command arguments should be passed as a list to avoid
intermediate interpretation by shell at all. Unfortunately gluing
strings to make a shell command is used too widely in org code and emacs
API encourages such unsafe way.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] ob-maxima.el: Fix execution on MS Windows
2021-12-29 17:05 ` Max Nikulin
@ 2021-12-29 18:37 ` Nikolay Kudryavtsev
2021-12-30 16:33 ` Max Nikulin
0 siblings, 1 reply; 8+ messages in thread
From: Nikolay Kudryavtsev @ 2021-12-29 18:37 UTC (permalink / raw)
To: Max Nikulin, emacs-orgmode
If your temporary-file-directory is something like "/tmp/apostrophe'",
it would not work currently either. So apostrophe is a very special case
here.
As for possible evaluation within the double quotes, while this is
theoretically possible, user sort of has to go out of his way to trigger
it, so the question is whether we should introduce any platform-specific
code to mitigate such an obscure case? Then we are also limited by
Maxima itself since it has to be able to read that path too and it's
very picky when it comes to file paths.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] ob-maxima.el: Fix execution on MS Windows
2021-12-29 18:37 ` Nikolay Kudryavtsev
@ 2021-12-30 16:33 ` Max Nikulin
2021-12-30 20:54 ` Nikolay Kudryavtsev
0 siblings, 1 reply; 8+ messages in thread
From: Max Nikulin @ 2021-12-30 16:33 UTC (permalink / raw)
To: emacs-orgmode
On 30/12/2021 01:37, Nikolay Kudryavtsev wrote:
> If your temporary-file-directory is something like "/tmp/apostrophe'",
> it would not work currently either. So apostrophe is a very special case
> here.
>
> As for possible evaluation within the double quotes, while this is
> theoretically possible, user sort of has to go out of his way to trigger
> it, so the question is whether we should introduce any platform-specific
> code to mitigate such an obscure case? Then we are also limited by
> Maxima itself since it has to be able to read that path too and it's
> very picky when it comes to file paths.
I am not a committer, so it is up to the maintainers to decide if your
patch is suitable. My intention is to draw attention to the issue,
however they may tolerate it.
I have not experimented with remote execution of babel code blocks using
tramp, so I may be unaware of some specific, e.g. execution using ssh
almost certainly assumes shell command and interface with list of
arguments may not be available.
When some external data is substituted into a Maxima command (batchload
this case) there should be an extra pass of escaping that protects
special characters like quotes (and backslashes?) accordingly to Maxima
rules.
I expect that %S formatter does a trick by adding quotes around the
string argument and adding backslashes before quote characters and
backslashes inside. I suspect that quotes your added around %S must not
be used there. Due to them file name appears outside of quotes at all.
This error is hidden unless at least a space character presents in
temporary directory path.
Unsure concerning Maxima but usually it is possible to pass arguments
avoiding quoting issues for particular language. A couple of examples
with inline code snippets
emacs -Q --batch --eval '(message "bl(%S)$" (car argv))' 'a"b\c.txt'
sh -c 'printf "bl(%s)\$\n" "$1"' 'sh' 'a"bc\d.txt'
Maybe there is a way to pass file name as a separate argument (without
combining it with command) to Maxima as well.
In my opinion, platform-specific code should be avoided when possible.
Even `shell-quote-argument' may be better. I would prefer e.g.
`call-process' from info "(elisp) Synchronous Processes"
https://www.gnu.org/software/emacs/manual/html_node/elisp/Synchronous-Processes.html
, but I am realizing that it may require more changes in babel or even
to cause problems with tramp.
Double quotes open issues with injection of commands in backticks `rm
something`, $variable expansion and other constructs. I hope,
`shell-quote-argument' is reliable enough.
P.S.
https://xkcd.com/327/ Exploits of a Mom
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] ob-maxima.el: Fix execution on MS Windows
2021-12-30 16:33 ` Max Nikulin
@ 2021-12-30 20:54 ` Nikolay Kudryavtsev
2022-01-11 13:55 ` Max Nikulin
2022-10-29 6:08 ` Ihor Radchenko
0 siblings, 2 replies; 8+ messages in thread
From: Nikolay Kudryavtsev @ 2021-12-30 20:54 UTC (permalink / raw)
To: Max Nikulin, emacs-orgmode
[-- Attachment #1: Type: text/plain, Size: 1278 bytes --]
> When some external data is substituted into a Maxima command
> (batchload this case) there should be an extra pass of escaping that
> protects special characters like quotes (and backslashes?) accordingly
> to Maxima rules.
Not necessarily, Maxima is capable of understanding unescaped paths, for
example, this works:
maxima --very-quiet -r "batchload(\"/tmp/sp
ce/babel-gxqTkM/maxima-ua3e9j.max\")"$
> I suspect that quotes your added around %S must not be used there. Due
> to them file name appears outside of quotes at all.
Yes, good catch.
> Unsure concerning Maxima but usually it is possible to pass arguments
> avoiding quoting issues for particular language.
Command line Maxima actually has a batch flag, but using it returns the
entire input file in the output too and that seems to be the reason why
the original authors of ob-maxima didn't use it. It's probably possible
to filter that on our side, but such filtering would require extra work,
which they probably deemed unnecessary, for such a rather obscure set of
use cases.
Anyway, I've tried to get it to work using shell-quote-argument, see the
attached patch. Seems to work well enough in practice on both platforms
and for cases like (setq temporary-file-directory "/tmp/`echo hi`/").
[-- Attachment #2: 0001-ob-maxima.el-Fix-execution-on-MS-Windows.patch --]
[-- Type: text/plain, Size: 1288 bytes --]
From 47690d14ac4838d8e39f08bd8224f0b4af053359 Mon Sep 17 00:00:00 2001
From: Nikolay Kudryavtsev <nikolay.kudryavtsev@gmail.com>
Date: Sun, 26 Dec 2021 22:47:19 +0300
Subject: [PATCH] ob-maxima.el: Fix execution on MS Windows
* ob-maxima.el (org-babel-execute:maxima): Change command line
invocation to a one that should work everywhere.
---
lisp/ob-maxima.el | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/lisp/ob-maxima.el b/lisp/ob-maxima.el
index 7b49bb07a..08b586414 100644
--- a/lisp/ob-maxima.el
+++ b/lisp/ob-maxima.el
@@ -77,8 +77,11 @@ This function is called by `org-babel-execute-src-block'."
(result
(let* ((cmdline (or (cdr (assq :cmdline params)) ""))
(in-file (org-babel-temp-file "maxima-" ".max"))
- (cmd (format "%s --very-quiet -r 'batchload(%S)$' %s"
- org-babel-maxima-command in-file cmdline)))
+ (cmd (format "%s --very-quiet -r %s$ %s"
+ org-babel-maxima-command
+ (shell-quote-argument
+ (format "batchload(%S)" in-file))
+ cmdline)))
(with-temp-file in-file (insert (org-babel-maxima-expand body params)))
(message cmd)
;; " | grep -v batch | grep -v 'replaced' | sed '/^$/d' "
--
2.34.1.windows.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] ob-maxima.el: Fix execution on MS Windows
2021-12-30 20:54 ` Nikolay Kudryavtsev
@ 2022-01-11 13:55 ` Max Nikulin
2022-10-29 6:08 ` Ihor Radchenko
1 sibling, 0 replies; 8+ messages in thread
From: Max Nikulin @ 2022-01-11 13:55 UTC (permalink / raw)
To: emacs-orgmode
Nikolay, I do not have any objections concerning the last version of
your patch (I have not test it though). It is tracked on
https://updates.orgmode.org/ however it may take months before it will
be committed.
Have you signed FSF papers related to copyright? See
https://orgmode.org/worg/org-contribute.html#copyright There is no
"TINYCHANGE" keyword in the commit message and I have not found you in
the list of contributors to Org mode (Bastien likely have access to more
actual data). Since several your patches have been accepted to Emacs,
you are close to the limit when formalities become necessary.
Do not consider the comments below as a request to change anything in
the patch. I am just trying to express more clear what I wrote in the
previous message.
On 31/12/2021 03:54, Nikolay Kudryavtsev wrote:
>> When some external data is substituted into a Maxima command
>> (batchload this case) there should be an extra pass of escaping that
>> protects special characters like quotes (and backslashes?) accordingly
>> to Maxima rules.
> Not necessarily, Maxima is capable of understanding unescaped paths, for
> example, this works:
>
> maxima --very-quiet -r "batchload(\"/tmp/sp
> ce/babel-gxqTkM/maxima-ua3e9j.max\")"$
Consider a really peculiar path for tmp files with quotes
/tmp/")$ something-weird$ "/maxima-ua3e9j.max
I hope, it is unrealistic and will be properly escaped by %S formatter.
I am a bit surprised that Maxima does not allow to do it in a more
reliable way since several interfaces exist (WxMaxima, texmacs). Unsure
if get_application_args is suitable
https://sourceforge.net/p/maxima/mailman/message/35180908/
> Command line Maxima actually has a batch flag, but using it returns the
> entire input file in the output too and that seems to be the reason why
> the original authors of ob-maxima didn't use it.
It would be great if it had another flag to suppress printing content of
the batch script or a flag that changes a setting that controls such
behavior.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] ob-maxima.el: Fix execution on MS Windows
2021-12-30 20:54 ` Nikolay Kudryavtsev
2022-01-11 13:55 ` Max Nikulin
@ 2022-10-29 6:08 ` Ihor Radchenko
2022-11-02 11:49 ` Nikolay Kudryavtsev
1 sibling, 1 reply; 8+ messages in thread
From: Ihor Radchenko @ 2022-10-29 6:08 UTC (permalink / raw)
To: Nikolay Kudryavtsev; +Cc: Max Nikulin, emacs-orgmode
Nikolay Kudryavtsev <nikolay.kudryavtsev@gmail.com> writes:
> Anyway, I've tried to get it to work using shell-quote-argument, see the
> attached patch. Seems to work well enough in practice on both platforms
> and for cases like (setq temporary-file-directory "/tmp/`echo hi`/").
> From 47690d14ac4838d8e39f08bd8224f0b4af053359 Mon Sep 17 00:00:00 2001
> From: Nikolay Kudryavtsev <nikolay.kudryavtsev@gmail.com>
> Date: Sun, 26 Dec 2021 22:47:19 +0300
> Subject: [PATCH] ob-maxima.el: Fix execution on MS Windows
Thanks for the patch, and sorry for the late reply.
I have applied your patch onto main adding TINYCHANGE cookie to the
commit message.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=6156b57bdf2b14ccfbf6646e8d217599738b694d
You are also listed as an Org contributor now.
https://git.sr.ht/~bzg/worg/commit/6a5bccb1aeca8e2b3ecef64238002bcfb46f68e0
--
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] ob-maxima.el: Fix execution on MS Windows
2022-10-29 6:08 ` Ihor Radchenko
@ 2022-11-02 11:49 ` Nikolay Kudryavtsev
0 siblings, 0 replies; 8+ messages in thread
From: Nikolay Kudryavtsev @ 2022-11-02 11:49 UTC (permalink / raw)
To: Ihor Radchenko; +Cc: Max Nikulin, emacs-orgmode
Oh, nice. Thank you.
I was starting to wonder what happened to this.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-11-02 11:50 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-12-26 20:18 [PATCH] ob-maxima.el: Fix execution on MS Windows Nikolay Kudryavtsev
2021-12-29 17:05 ` Max Nikulin
2021-12-29 18:37 ` Nikolay Kudryavtsev
2021-12-30 16:33 ` Max Nikulin
2021-12-30 20:54 ` Nikolay Kudryavtsev
2022-01-11 13:55 ` Max Nikulin
2022-10-29 6:08 ` Ihor Radchenko
2022-11-02 11:49 ` Nikolay Kudryavtsev
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.