* Warn about shell-expansion in the docstring of org-latex-to-html-convert-command
@ 2024-02-16 23:10 Martin Edström
2024-02-18 16:06 ` Ihor Radchenko
0 siblings, 1 reply; 33+ messages in thread
From: Martin Edström @ 2024-02-16 23:10 UTC (permalink / raw)
To: emacs-orgmode
I've just been struggling with my custom setting for
`org-latex-to-html-convert-command` outputting many math snippets
wrong. The fault was mine: I didn't correctly shell-quote the input.
I propose to add a warning in the docstring, because many people will
trip the same problem.
The thing is that double-quotes don't work in shell commands. I had
\"%i\", but it should've been '%i':
(setopt org-latex-to-html-convert-command "node
/home/kept/pub/texToMathML.js \"%i\"")
Math snippets that start with $, like $y = 200$ of course get
butchered into just " = 200$" because of course the first $y gets
interpreted as a shell environment variable.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Warn about shell-expansion in the docstring of org-latex-to-html-convert-command
2024-02-16 23:10 Warn about shell-expansion in the docstring of org-latex-to-html-convert-command Martin Edström
@ 2024-02-18 16:06 ` Ihor Radchenko
2024-02-18 18:56 ` Martin Edström
0 siblings, 1 reply; 33+ messages in thread
From: Ihor Radchenko @ 2024-02-18 16:06 UTC (permalink / raw)
To: Martin Edström; +Cc: emacs-orgmode
Martin Edström <meedstrom91@gmail.com> writes:
> I've just been struggling with my custom setting for
> `org-latex-to-html-convert-command` outputting many math snippets
> wrong. The fault was mine: I didn't correctly shell-quote the input.
> I propose to add a warning in the docstring, because many people will
> trip the same problem.
> The thing is that double-quotes don't work in shell commands. I had
> \"%i\", but it should've been '%i':
Would you be interested to submit a patch?
See https://orgmode.org/worg/org-contribute.html#first-patch
--
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] 33+ messages in thread
* Re: Warn about shell-expansion in the docstring of org-latex-to-html-convert-command
2024-02-18 16:06 ` Ihor Radchenko
@ 2024-02-18 18:56 ` Martin Edström
2024-02-18 19:36 ` Martin Edström
0 siblings, 1 reply; 33+ messages in thread
From: Martin Edström @ 2024-02-18 18:56 UTC (permalink / raw)
To: Ihor Radchenko; +Cc: emacs-orgmode
[-- Attachment #1: Type: text/plain, Size: 976 bytes --]
I will try to do a patch, thanks for the link. Stay tuned.
On Sun, Feb 18, 2024 at 15:06 Ihor Radchenko <yantar92@posteo.net> wrote:
> Martin Edström <meedstrom91@gmail.com> writes:
>
> > I've just been struggling with my custom setting for
> > `org-latex-to-html-convert-command` outputting many math snippets
> > wrong. The fault was mine: I didn't correctly shell-quote the input.
> > I propose to add a warning in the docstring, because many people will
> > trip the same problem.
>
> > The thing is that double-quotes don't work in shell commands. I had
> > \"%i\", but it should've been '%i':
>
> Would you be interested to submit a patch?
> See https://orgmode.org/worg/org-contribute.html#first-patch
>
> --
> 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>
>
[-- Attachment #2: Type: text/html, Size: 1816 bytes --]
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Warn about shell-expansion in the docstring of org-latex-to-html-convert-command
2024-02-18 18:56 ` Martin Edström
@ 2024-02-18 19:36 ` Martin Edström
2024-02-19 8:30 ` Ihor Radchenko
2024-02-21 14:38 ` Max Nikulin
0 siblings, 2 replies; 33+ messages in thread
From: Martin Edström @ 2024-02-18 19:36 UTC (permalink / raw)
To: Ihor Radchenko; +Cc: emacs-orgmode
[-- Attachment #1: Type: text/plain, Size: 1260 bytes --]
Here you go!
Tests passed (14 SKIPPED), compiled fine. I've made no prior
contributions and this changes 5 lines. I'm ok if you want to rephrase
it in any way.
Martin
On Sun, 18 Feb 2024 at 19:56, Martin Edström <meedstrom91@gmail.com> wrote:
>
> I will try to do a patch, thanks for the link. Stay tuned.
>
> On Sun, Feb 18, 2024 at 15:06 Ihor Radchenko <yantar92@posteo.net> wrote:
>>
>> Martin Edström <meedstrom91@gmail.com> writes:
>>
>> > I've just been struggling with my custom setting for
>> > `org-latex-to-html-convert-command` outputting many math snippets
>> > wrong. The fault was mine: I didn't correctly shell-quote the input.
>> > I propose to add a warning in the docstring, because many people will
>> > trip the same problem.
>>
>> > The thing is that double-quotes don't work in shell commands. I had
>> > \"%i\", but it should've been '%i':
>>
>> Would you be interested to submit a patch?
>> See https://orgmode.org/worg/org-contribute.html#first-patch
>>
>> --
>> 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>
[-- Attachment #2: 0001-lisp-org.el-Enhance-a-docstring.patch --]
[-- Type: text/x-patch, Size: 1332 bytes --]
From d3b1b0a3cc4deac7ac47f446fb0bf27f61169ac4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Edstr=C3=B6m?= <meedstrom91@gmail.com>
Date: Sun, 18 Feb 2024 20:29:48 +0100
Subject: [PATCH] lisp/org.el: Enhance a docstring
* org.el (org-latex-to-html-convert-command): Add a note in the
docstring about proper shell-quoting.
It can trip you up because wrongly quoted input still works with some
math snippets, so the command may work during testing but not later
when you have different math snippets in play.
TINYCHANGE
---
lisp/org.el | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/lisp/org.el b/lisp/org.el
index 947037559..6b2ebf9ac 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -3275,7 +3275,11 @@ Replace format-specifiers in the command as noted below and use
%i: The LaTeX fragment to be converted.
For example, this could be used with LaTeXML as
-\"latexmlc \\='literal:%i\\=' --profile=math --preload=siunitx.sty 2>/dev/null\"."
+\"latexmlc \\='literal:%i\\=' --profile=math --preload=siunitx.sty 2>/dev/null\".
+
+Since this is a shell-command, remember to use single-quotes
+around \\='%i\\=', not double-quotes! Else a math fragment such
+as \"$y = 200$\" gets butchered into only \" = 200\"."
:group 'org-latex
:package-version '(Org . "9.4")
:type '(choice
--
2.40.1
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: Warn about shell-expansion in the docstring of org-latex-to-html-convert-command
2024-02-18 19:36 ` Martin Edström
@ 2024-02-19 8:30 ` Ihor Radchenko
2024-02-21 14:38 ` Max Nikulin
1 sibling, 0 replies; 33+ messages in thread
From: Ihor Radchenko @ 2024-02-19 8:30 UTC (permalink / raw)
To: Martin Edström; +Cc: emacs-orgmode
Martin Edström <meedstrom91@gmail.com> writes:
> Tests passed (14 SKIPPED), compiled fine. I've made no prior
> contributions and this changes 5 lines. I'm ok if you want to rephrase
> it in any way.
Applied onto main, with amendments.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=a8443f2c7
Thanks for your contribution!
You are now listed as one of the Org mode contributors:
https://git.sr.ht/~bzg/worg/commit/53d1e6e3
--
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] 33+ messages in thread
* Re: Warn about shell-expansion in the docstring of org-latex-to-html-convert-command
2024-02-18 19:36 ` Martin Edström
2024-02-19 8:30 ` Ihor Radchenko
@ 2024-02-21 14:38 ` Max Nikulin
2024-02-21 14:57 ` Martin Edström
` (2 more replies)
1 sibling, 3 replies; 33+ messages in thread
From: Max Nikulin @ 2024-02-21 14:38 UTC (permalink / raw)
To: Martin Edström; +Cc: emacs-orgmode
On 19/02/2024 02:36, Martin Edström wrote:
> +Since this is a shell-command, remember to use single-quotes
> +around \\='%i\\=', not double-quotes! Else a math fragment such
> +as \"$y = 200$\" gets butchered into only \" = 200\"."
I am afraid, the code, not the docstring must be fixed. I have not tried
it, but I expect an issue with
Test \(f' = df/dx\)
So `shell-quote-argument' is necessary and quotes around %i must be
stripped similar to %s in mailcap entries in `org-open-file'.
Notice that dollar-math $x = y$ is not recommended and considered as
obsolete syntax. Use \(x = y\) or \[x = y\] (the latter for display math).
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Warn about shell-expansion in the docstring of org-latex-to-html-convert-command
2024-02-21 14:38 ` Max Nikulin
@ 2024-02-21 14:57 ` Martin Edström
2024-02-21 15:04 ` Martin Edström
2024-02-23 12:46 ` Ihor Radchenko
2 siblings, 0 replies; 33+ messages in thread
From: Martin Edström @ 2024-02-21 14:57 UTC (permalink / raw)
To: Martin Edström, emacs-orgmode
[-- Attachment #1: Type: text/plain, Size: 1619 bytes --]
Ah yes. My custom Node script (which I hope it's OK to paste below) strips
the beginnings and ends of the input. I suppose that it could be done
already on the Elisp side, which would take care of interpreting the
$-signs as envvars too.
const katex = require('katex')
let input = process.argv[2].trim()
let disp = true
if (input.slice(0, 2) === '\\[' || input.slice(0, 2) === '$$') {
input = input.slice(2, -2)
}
else if (input.slice(0, 2) === '\\(') {
input = input.slice(2, -2)
disp = false
}
else if (input.slice(0, 1) === '$') {
input = input.slice(1, -1)
disp = false
}
else {
console.error("Did you quote the input correctly?")
process.exit(1)
}
console.log(katex.renderToString(
input, {
displayMode: disp,
output: 'mathml',
trust: true,
strict: false,
throwOnError: false,
}
))
On Wed, Feb 21, 2024 at 15:38 Max Nikulin <manikulin@gmail.com> wrote:
> On 19/02/2024 02:36, Martin Edström wrote:
> > +Since this is a shell-command, remember to use single-quotes
> > +around \\='%i\\=', not double-quotes! Else a math fragment such
> > +as \"$y = 200$\" gets butchered into only \" = 200\"."
>
> I am afraid, the code, not the docstring must be fixed. I have not tried
> it, but I expect an issue with
>
> Test \(f' = df/dx\)
>
> So `shell-quote-argument' is necessary and quotes around %i must be
> stripped similar to %s in mailcap entries in `org-open-file'.
>
> Notice that dollar-math $x = y$ is not recommended and considered as
> obsolete syntax. Use \(x = y\) or \[x = y\] (the latter for display math).
>
[-- Attachment #2: Type: text/html, Size: 2213 bytes --]
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Warn about shell-expansion in the docstring of org-latex-to-html-convert-command
2024-02-21 14:38 ` Max Nikulin
2024-02-21 14:57 ` Martin Edström
@ 2024-02-21 15:04 ` Martin Edström
2024-02-21 15:08 ` Martin Edström
2024-02-23 12:46 ` Ihor Radchenko
2 siblings, 1 reply; 33+ messages in thread
From: Martin Edström @ 2024-02-21 15:04 UTC (permalink / raw)
To: Martin Edström, emacs-orgmode
Actually, I agree about your test case, that looks like it'd cause a problem.
So we patch the function to use `shell-quote-argument'?
On Wed, 21 Feb 2024 at 15:38, Max Nikulin <manikulin@gmail.com> wrote:
>
> On 19/02/2024 02:36, Martin Edström wrote:
> > +Since this is a shell-command, remember to use single-quotes
> > +around \\='%i\\=', not double-quotes! Else a math fragment such
> > +as \"$y = 200$\" gets butchered into only \" = 200\"."
>
> I am afraid, the code, not the docstring must be fixed. I have not tried
> it, but I expect an issue with
>
> Test \(f' = df/dx\)
>
> So `shell-quote-argument' is necessary and quotes around %i must be
> stripped similar to %s in mailcap entries in `org-open-file'.
>
> Notice that dollar-math $x = y$ is not recommended and considered as
> obsolete syntax. Use \(x = y\) or \[x = y\] (the latter for display math).
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Warn about shell-expansion in the docstring of org-latex-to-html-convert-command
2024-02-21 15:04 ` Martin Edström
@ 2024-02-21 15:08 ` Martin Edström
0 siblings, 0 replies; 33+ messages in thread
From: Martin Edström @ 2024-02-21 15:08 UTC (permalink / raw)
To: Martin Edström, emacs-orgmode
And ignore my suggestion about stripping input on the Elisp side.
Didn't think that through.
On Wed, 21 Feb 2024 at 16:04, Martin Edström <meedstrom91@gmail.com> wrote:
>
> Actually, I agree about your test case, that looks like it'd cause a problem.
>
> So we patch the function to use `shell-quote-argument'?
>
> On Wed, 21 Feb 2024 at 15:38, Max Nikulin <manikulin@gmail.com> wrote:
> >
> > On 19/02/2024 02:36, Martin Edström wrote:
> > > +Since this is a shell-command, remember to use single-quotes
> > > +around \\='%i\\=', not double-quotes! Else a math fragment such
> > > +as \"$y = 200$\" gets butchered into only \" = 200\"."
> >
> > I am afraid, the code, not the docstring must be fixed. I have not tried
> > it, but I expect an issue with
> >
> > Test \(f' = df/dx\)
> >
> > So `shell-quote-argument' is necessary and quotes around %i must be
> > stripped similar to %s in mailcap entries in `org-open-file'.
> >
> > Notice that dollar-math $x = y$ is not recommended and considered as
> > obsolete syntax. Use \(x = y\) or \[x = y\] (the latter for display math).
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Warn about shell-expansion in the docstring of org-latex-to-html-convert-command
2024-02-21 14:38 ` Max Nikulin
2024-02-21 14:57 ` Martin Edström
2024-02-21 15:04 ` Martin Edström
@ 2024-02-23 12:46 ` Ihor Radchenko
2024-02-25 10:41 ` Max Nikulin
2 siblings, 1 reply; 33+ messages in thread
From: Ihor Radchenko @ 2024-02-23 12:46 UTC (permalink / raw)
To: Max Nikulin; +Cc: Martin Edström, emacs-orgmode
Max Nikulin <manikulin@gmail.com> writes:
> On 19/02/2024 02:36, Martin Edström wrote:
>> +Since this is a shell-command, remember to use single-quotes
>> +around \\='%i\\=', not double-quotes! Else a math fragment such
>> +as \"$y = 200$\" gets butchered into only \" = 200\"."
>
> I am afraid, the code, not the docstring must be fixed. I have not tried
> it, but I expect an issue with
>
> Test \(f' = df/dx\)
>
> So `shell-quote-argument' is necessary and quotes around %i must be
> stripped similar to %s in mailcap entries in `org-open-file'.
That would be backwards-incompatible.
What about introducing %e replacement that will be shell-escaped?
--
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] 33+ messages in thread
* Re: Warn about shell-expansion in the docstring of org-latex-to-html-convert-command
2024-02-23 12:46 ` Ihor Radchenko
@ 2024-02-25 10:41 ` Max Nikulin
2024-02-26 10:48 ` Ihor Radchenko
2024-03-05 12:01 ` Max Nikulin
0 siblings, 2 replies; 33+ messages in thread
From: Max Nikulin @ 2024-02-25 10:41 UTC (permalink / raw)
To: emacs-orgmode; +Cc: Martin Edström
On 23/02/2024 19:46, Ihor Radchenko wrote:
> Max Nikulin <manikulin@gmail.com> writes:
>
>> On 19/02/2024 02:36, Martin Edström wrote:
>>> +Since this is a shell-command, remember to use single-quotes
>>> +around \\='%i\\=', not double-quotes! Else a math fragment such
>>> +as \"$y = 200$\" gets butchered into only \" = 200\"."
>>
>> I am afraid, the code, not the docstring must be fixed. I have not tried
>> it, but I expect an issue with
>>
>> Test \(f' = df/dx\)
>>
>> So `shell-quote-argument' is necessary and quotes around %i must be
>> stripped similar to %s in mailcap entries in `org-open-file'.
>
> That would be backwards-incompatible.
> What about introducing %e replacement that will be shell-escaped?
Ihor, it is just a bug and its manifestation depends on content of .org
files more than on user configuration. Do you really want to allow part
of equations be executed as shell commands for the sake of miracle
backward compatibility?
To minimize user annoyance my suggestion is to strip quotes in words like
- '%i'
- "%i"
- 'something%i'
- "something%i"
- something='%i'
- something="%i"
before calling `format-spec' with `shell-quote-argument' result.
Please, revert the commit that added a misleading recommendation.
By the way, single quotes have no special meaning in cmd.exe on windows.
Example of silent error resulting in incorrect equation:
(let ((org-latex-to-html-convert-command
"printf '%%s' '%i'"))
(org-format-latex-as-html "$f'' = df/dx$"))
"$f = df/dx$"
Random parts of math becomes part of shell command:
(let ((org-latex-to-html-convert-command
"printf '%%s' '%i'"))
(org-format-latex-as-html "$f' = df/dx$"))
"/bin/bash: -c: line 1: unexpected EOF while looking for matching `''
"
Something weird may be executed in the case of sufficiently complex
equations.
It should be more reliable to pass fragment to command stdin. It can be
done if %i is missed in `org-latex-to-html-convert-command'.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Warn about shell-expansion in the docstring of org-latex-to-html-convert-command
2024-02-25 10:41 ` Max Nikulin
@ 2024-02-26 10:48 ` Ihor Radchenko
2024-02-26 16:37 ` Max Nikulin
2024-03-05 12:01 ` Max Nikulin
1 sibling, 1 reply; 33+ messages in thread
From: Ihor Radchenko @ 2024-02-26 10:48 UTC (permalink / raw)
To: Max Nikulin; +Cc: emacs-orgmode, Martin Edström
[-- Attachment #1: Type: text/plain, Size: 1824 bytes --]
Max Nikulin <manikulin@gmail.com> writes:
> (let ((org-latex-to-html-convert-command
> "printf '%%s' '%i'"))
> (org-format-latex-as-html "$f' = df/dx$"))
> "/bin/bash: -c: line 1: unexpected EOF while looking for matching `''
> "
>
> Something weird may be executed in the case of sufficiently complex
> equations.
> It should be more reliable to pass fragment to command stdin. It can be
> done if %i is missed in `org-latex-to-html-convert-command'.
I agree that it will be more reliable to shell-escape argument.
However, I am concerned that escaping may break certain uses like
somecommand << EOF
%i
EOF
In the above scenario, escaping will break things.
That's why I prefer to add a new replacement, not change the meaning of
%i. We might even remove %i from the docstring, keeping support in the
code for backwards-compatibility.
Also, I just looked closer into the example with latexml we provide in
the docstring and played around with it.
I noticed that
with
(defun org-format-latex-as-html (latex-fragment)
"Convert LATEX-FRAGMENT to HTML.
This uses `org-latex-to-html-convert-command', which see."
(let ((cmd (format-spec org-latex-to-html-convert-command
`((?i . ,latex-fragment)
(?I . ,(shell-quote-argument latex-fragment))))))
(message "Running %s" cmd)
(shell-command-to-string cmd)))
(with-temp-file "/tmp/test2.html"
(let ((org-latex-to-html-convert-command
"latexmlc literal:%I --profile=math --preload=siunitx.sty 2>/dev/null"))
(insert (org-format-latex-as-html "$f' = df/dx$"))))
test2.html is rendered *incorrectly* as in the attached screenshot.
In contrast, manually providing output file as
latexmlc literal:\$f\'\ =\ df/dx\$ --profile=math --preload=siunitx.sty --output /tmp/test3.html
yields correct rendering.
[-- Attachment #2: incorrect-rendering.png --]
[-- Type: image/png, Size: 11391 bytes --]
[-- Attachment #3: correct-rendering.png --]
[-- Type: image/png, Size: 6596 bytes --]
[-- Attachment #4: Type: text/plain, Size: 224 bytes --]
--
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] 33+ messages in thread
* Re: Warn about shell-expansion in the docstring of org-latex-to-html-convert-command
2024-02-26 10:48 ` Ihor Radchenko
@ 2024-02-26 16:37 ` Max Nikulin
2024-03-08 11:16 ` Ihor Radchenko
0 siblings, 1 reply; 33+ messages in thread
From: Max Nikulin @ 2024-02-26 16:37 UTC (permalink / raw)
To: emacs-orgmode; +Cc: Martin Edström
On 26/02/2024 17:48, Ihor Radchenko wrote:
> Max Nikulin writes:
>> Something weird may be executed in the case of sufficiently complex
>> equations.
>
>> It should be more reliable to pass fragment to command stdin. It can be
>> done if %i is missed in `org-latex-to-html-convert-command'.
>
> I agree that it will be more reliable to shell-escape argument.
> However, I am concerned that escaping may break certain uses like
>
> somecommand << EOF
> %i
> EOF
>
> In the above scenario, escaping will break things.
It is unsafe to use such command. Variable expansion, etc. is performed
inside here document blocks. Try
cat << EOF
\[f(i), \text{where $i \ne 10$}\]
EOF
That is why I proposed to use stdin in the case of missed %i.
`org-latex-to-html-convert-command' should be set to something like
"latexmlc --profile=math --preload=siunitx.sty - 2>/dev/null"
this case.
> That's why I prefer to add a new replacement, not change the meaning of
> %i. We might even remove %i from the docstring, keeping support in the
> code for backwards-compatibility.
What you calls backward compatibility is actually a means to get strange
results in the case of complex math. It is better to force users to
update configuration (I hope, it actually will not be necessary) and to
ensure safe command without pitfalls related to missed parts of equations.
> (with-temp-file "/tmp/test2.html"
> (let ((org-latex-to-html-convert-command
> "latexmlc literal:%I --profile=math --preload=siunitx.sty 2>/dev/null"))
> (insert (org-format-latex-as-html "$f' = df/dx$"))))
>
> test2.html is rendered *incorrectly* as in the attached screenshot.
Looks like missed <meta charset="UTF-8"> inside <head>
> In contrast, manually providing output file as
>
> latexmlc literal:\$f\'\ =\ df/dx\$ --profile=math --preload=siunitx.sty --output /tmp/test3.html
>
> yields correct rendering.
Perhaps this time the browser just guessed file encoding. Anyway
rendering is incorrect. Gecko puts derivative into the correct place. I
have no idea if it is a fault of latexml generating incorrect MathML or
a browser which is likely a KHTML descendant.
It seems, latexml is terribly broken in Debian. With
--preload=siunitx.sty it hangs during processing of expl3-code.tex,
without this option it removes all files in /tmp.
I am still strongly against code that may cause execution of equations
as shell commands and may silently lose parts of equations.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Warn about shell-expansion in the docstring of org-latex-to-html-convert-command
2024-02-25 10:41 ` Max Nikulin
2024-02-26 10:48 ` Ihor Radchenko
@ 2024-03-05 12:01 ` Max Nikulin
1 sibling, 0 replies; 33+ messages in thread
From: Max Nikulin @ 2024-03-05 12:01 UTC (permalink / raw)
To: emacs-orgmode; +Cc: Martin Edström
On 25/02/2024 17:41, Max Nikulin wrote:
>> Max Nikulin writes:
>>> So `shell-quote-argument' is necessary and quotes around %i must be
>>> stripped similar to %s in mailcap entries in `org-open-file'.
...
> Please, revert the commit that added a misleading recommendation.
...
> It should be more reliable to pass fragment to command stdin. It can be
> done if %i is missed in `org-latex-to-html-convert-command'.
I have realized that there is `org-latex-to-mathml-convert-command'
introduced a decade earlier and affected by the same issue with possible
leak of formula to shell command. Even if there are reasons against
obsoleting `org-latex-to-html-convert-command' in favor of
`org-latex-to-mathml-convert-command', both user options should be
handled by the same function.
I am unsure if it is an intended feature that when an org file is opened
from a remote location like /ssh:... then
`org-latex-to-html-convert-command' is executed on the remote host. It
makes implementation of stdin more tricky. Ideally, it should be
configurable where the command is executed: where emacs is running,
where the document resides, or even with specific `default-directory'.
Double quotes are recommended around %i for ODT export
(info "(org) LaTeX math snippets")
https://orgmode.org/manual/LaTeX-math-snippets.html
and it should be fixed as well.
It seems --preload=siunitx.sty should be recommended any more for latexml:
https://github.com/brucemiller/LaTeXML/issues/2268
Problem width loading expl3-code.tex
Perhaps at least some cases may be handled by pandoc
https://list.orgmode.org/CAEPTPEzvx5ZhY5qrCJnFtAC_NpPC9d1a-Q=yE+XNTrPXiMpTag@mail.gmail.com/
David Lukeš. Using pandoc to convert LaTeX math to MathML
Tue, 1 Mar 2022 15:59:36 +0100
Unfortunately I am not familiar with MathML enough to evaluate that
there are no caveats with pandoc.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Warn about shell-expansion in the docstring of org-latex-to-html-convert-command
2024-02-26 16:37 ` Max Nikulin
@ 2024-03-08 11:16 ` Ihor Radchenko
2024-03-09 15:23 ` Max Nikulin
0 siblings, 1 reply; 33+ messages in thread
From: Ihor Radchenko @ 2024-03-08 11:16 UTC (permalink / raw)
To: Max Nikulin; +Cc: emacs-orgmode, Martin Edström
[-- Attachment #1: Type: text/plain, Size: 2051 bytes --]
Max Nikulin <manikulin@gmail.com> writes:
>>> It should be more reliable to pass fragment to command stdin. It can be
>>> done if %i is missed in `org-latex-to-html-convert-command'.
>>
>> I agree that it will be more reliable to shell-escape argument.
>> However, I am concerned that escaping may break certain uses like
>>
>> somecommand << EOF
>> %i
>> EOF
>>
>> In the above scenario, escaping will break things.
>
> It is unsafe to use such command. Variable expansion, etc. is performed
> inside here document blocks. Try
>
> cat << EOF
> \[f(i), \text{where $i \ne 10$}\]
> EOF
I did non know this. Thanks for the info.
> That is why I proposed to use stdin in the case of missed %i.
>
> `org-latex-to-html-convert-command' should be set to something like
> "latexmlc --profile=math --preload=siunitx.sty - 2>/dev/null"
> this case.
I decided not to introduce stdin. User can always use echo %i | ... instead.
>> That's why I prefer to add a new replacement, not change the meaning of
>> %i. We might even remove %i from the docstring, keeping support in the
>> code for backwards-compatibility.
>
> What you calls backward compatibility is actually a means to get strange
> results in the case of complex math. It is better to force users to
> update configuration (I hope, it actually will not be necessary) and to
> ensure safe command without pitfalls related to missed parts of equations.
Agree.
This breaking change cannot be avoided, unfortunately.
Even stripping quotes is unreliable when we use the example from
docstring: 'literal:%i'. So, we have to bite the bullet.
>> test2.html is rendered *incorrectly* as in the attached screenshot.
>
> Looks like missed <meta charset="UTF-8"> inside <head>
> ...
Exporting Org document using
(setq org-html-with-latex 'html)
(setq org-latex-to-html-convert-command "latexmlc 'literal:%i' --profile=math --preload=siunitx.sty 2>/dev/null")
renders just fine, so these caveats appear to be terminal-specific. Not
our problem.
Attaching tentative patch that fixes the problem.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-latex-to-mathml-html-convert-command-Prevent-she.patch --]
[-- Type: text/x-patch, Size: 4506 bytes --]
From 34e5e14260cf895b32f13ed8f4c2e50684f91baf Mon Sep 17 00:00:00 2001
Message-ID: <34e5e14260cf895b32f13ed8f4c2e50684f91baf.1709896570.git.yantar92@posteo.net>
From: Ihor Radchenko <yantar92@posteo.net>
Date: Fri, 8 Mar 2024 14:05:12 +0300
Subject: [PATCH] org-latex-to-mathml/html-convert-command: Prevent shell
expansion
* lisp/org.el (org-create-math-formula):
(org-format-latex-as-html): Shell-quote LaTeX fragment text when
replacing %i placeholder. This prevents shell expansion of
$... and similar constructs inside the code.
(org-latex-to-mathml-convert-command):
(org-latex-to-html-convert-command): Update the docstring.
* etc/ORG-NEWS (~org-latex-to-mathml-convert-command~ and
~org-latex-to-html-convert-command~ shell-escape LaTeX code): Announce
the breaking change.
Reported-by: Max Nikulin <manikulin@gmail.com>
Link: https://orgmode.org/list/735645dd-1ddf-4579-a6dd-2700f3e83c94@gmail.com
---
etc/ORG-NEWS | 10 ++++++++++
lisp/org.el | 17 ++++++-----------
2 files changed, 16 insertions(+), 11 deletions(-)
diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index abe62daaf..9f628bc10 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -13,6 +13,16 @@ Please send Org bug reports to mailto:emacs-orgmode@gnu.org.
* Version 9.7 (not released yet)
** Important announcements and breaking changes
+*** ~org-latex-to-mathml-convert-command~ and ~org-latex-to-html-convert-command~ shell-escape LaTeX code
+
+Previously, ~org-latex-to-mathml-convert-command~ and
+~org-latex-to-html-convert-command~ replaced %i placeholders with raw
+LaTeX fragment text, potentially triggered shell-expansion.
+
+Now, the %i placeholders are shell-escaped to prevent shell expansion - this will prevent.
+
+The existing customizations that assume no shell-escaping must be updated.
+
*** When ~org-link-file-path-type~ is a function, its argument is now a filename as it is read by ~org-insert-link~; not an absolute path
Previously, when ~org-link-file-path-type~ is set to a function, the
diff --git a/lisp/org.el b/lisp/org.el
index 33d90506b..a00d50c51 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -3246,7 +3246,7 @@ (defcustom org-latex-to-mathml-convert-command nil
%j: Executable file in fully expanded form as specified by
`org-latex-to-mathml-jar-file'.
%I: Input LaTeX file in fully expanded form.
-%i: The latex fragment to be converted.
+%i: Shell-escaped LaTeX fragment to be converted.
%o: Output MathML file.
This command is used by `org-create-math-formula'.
@@ -3255,7 +3255,7 @@ (defcustom org-latex-to-mathml-convert-command nil
\"java -jar %j -unicode -force -df %o %I\".
When using LaTeXML set this option to
-\"latexmlmath \"%i\" --presentationmathml=%o\"."
+\"latexmlmath %i --presentationmathml=%o\"."
:group 'org-latex
:version "24.1"
:type '(choice
@@ -3268,15 +3268,10 @@ (defcustom org-latex-to-html-convert-command nil
directly replace the LaTeX fragment in the resulting HTML.
Replace format-specifiers in the command as noted below and use
`shell-command' to convert LaTeX to HTML.
-%i: The LaTeX fragment to be converted.
+%i: The LaTeX fragment to be converted (shell-escaped).
For example, this could be used with LaTeXML as
-\"latexmlc \\='literal:%i\\=' --profile=math --preload=siunitx.sty 2>/dev/null\".
-
-The LaTeX fragment is replaced as is, without escaping special shell
-syntax. It may be necessary to use single-quotes around \\='%i\\=', not
-double-quotes. Else a math fragment such as \"$y = 200$\" may be
-expanded to \" = 200\"."
+\"latexmlc literal:%i --profile=math --preload=siunitx.sty 2>/dev/null\"."
:group 'org-latex
:package-version '(Org . "9.4")
:type '(choice
@@ -16210,7 +16205,7 @@ (defun org-create-math-formula (latex-frag &optional mathml-file)
(expand-file-name
org-latex-to-mathml-jar-file))))
(?I . ,(shell-quote-argument tmp-in-file))
- (?i . ,latex-frag)
+ (?i . ,(shell-quote-argument latex-frag))
(?o . ,(shell-quote-argument tmp-out-file)))))
mathml shell-command-output)
(when (called-interactively-p 'any)
@@ -16277,7 +16272,7 @@ (defun org-format-latex-as-html (latex-fragment)
"Convert LATEX-FRAGMENT to HTML.
This uses `org-latex-to-html-convert-command', which see."
(let ((cmd (format-spec org-latex-to-html-convert-command
- `((?i . ,latex-fragment)))))
+ `((?i . ,(shell-quote-argument latex-fragment))))))
(message "Running %s" cmd)
(shell-command-to-string cmd)))
--
2.43.0
[-- Attachment #3: Type: text/plain, Size: 224 bytes --]
--
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 related [flat|nested] 33+ messages in thread
* Re: Warn about shell-expansion in the docstring of org-latex-to-html-convert-command
2024-03-08 11:16 ` Ihor Radchenko
@ 2024-03-09 15:23 ` Max Nikulin
2024-03-10 5:02 ` [PATCH] Unit tests for function calling MathML converters (Re: Warn about shell-expansion in the docstring of org-latex-to-html-convert-command) Max Nikulin
` (2 more replies)
0 siblings, 3 replies; 33+ messages in thread
From: Max Nikulin @ 2024-03-09 15:23 UTC (permalink / raw)
To: emacs-orgmode
On 08/03/2024 18:16, Ihor Radchenko wrote:
> Max Nikulin writes:
>
> I decided not to introduce stdin. User can always use echo %i | ... instead.
printf "%%s" %i
should be safer. However in this particular case, input that may be
recognized like echo options ("-n") should be wrapped with LaTeX delimiters.
> Even stripping quotes is unreliable when we use the example from
> docstring: 'literal:%i'.
My idea is to recognize this case. If stripping is not performed then it
is necessary to detect if user command is safe. Otherwise apostrophe in
a formula (even after escaping) may cause leaking math to shell. I have
not figured out if it is possible to bypass double quotes, but extra
slashes may distort math expression.
It is trivial to cause shell failure when single quotes are used around
%i. I am in doubts concerning double quotes. Perhaps stripping them is
more reliable.
> Attaching tentative patch that fixes the problem.
I think it is in the right direction.
- Manual needs update as well.
- I would explicitly stress that quotes causes undefined or even
dangerous behavior. See e.g. the last paragraph
https://specifications.freedesktop.org/desktop-entry-spec/latest/ar01s07.html
- I expected it as bugfix.
I have tried to add some unit tests, but I faced an issue with
`org-create-math-formula'. It creates temporary files in
`default-directory' and does not remove them on failure. Moreover, it
does not work in a container where git is not installed:
Debugger entered--Lisp error: (file-missing "Searching for program" "No
such file or directory" "git")
that is called from `find-file-hook'.
(ert-deftest test-org/create-math-formula ()
"Test shell special characters escaping in `org-create-math-formula'."
(let ((org-latex-to-mathml-convert-command
"printf \"<math
xmlns=\\\"http://www.w3.org/1998/Math/MathML\\\"><I%%sI></math>\" %i >%o"))
;; No backslashes added by `shell-quote-argumet'
;; are leaked to command arguments. dash(1) "Double Quotes":
;;
;; The backslash inside double quotes is historically weird,
;; and serves to quote only the following characters:
;; $ ` " \ <newline>.
;; Otherwise it remains literal.
(should
(equal "<I(|)`[[\\]]{}#$'!I>"
(org-create-math-formula "(|)`[[\\]]{}#$'!")))
;; Multiple words
(should
(equal "<Iwords ; |I>"
(org-create-math-formula "words ; |")))
;; Bypass single quote
(should
(equal "<Iapostrophe' ; |I>"
(org-create-math-formula "apostrophe' ; |")))
;; Bypass double quote
(should
(equal "<Iquote\" ; |I>"
(org-create-math-formula "quote\" ; |")))))
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH] Unit tests for function calling MathML converters (Re: Warn about shell-expansion in the docstring of org-latex-to-html-convert-command)
2024-03-09 15:23 ` Max Nikulin
@ 2024-03-10 5:02 ` Max Nikulin
2024-03-31 8:27 ` Ihor Radchenko
2024-03-12 13:03 ` Warn about shell-expansion in the docstring of org-latex-to-html-convert-command Ihor Radchenko
2024-03-31 8:25 ` Warn about shell-expansion in the docstring of org-latex-to-html-convert-command Ihor Radchenko
2 siblings, 1 reply; 33+ messages in thread
From: Max Nikulin @ 2024-03-10 5:02 UTC (permalink / raw)
To: emacs-orgmode
[-- Attachment #1: Type: text/plain, Size: 692 bytes --]
On 09/03/2024 22:23, Max Nikulin wrote:
> On 08/03/2024 18:16, Ihor Radchenko wrote:
>
>> Attaching tentative patch that fixes the problem.
I propose to add unit tests, see first attachment.
> I have tried to add some unit tests, but I faced an issue with
> `org-create-math-formula'. It creates temporary files in
> `default-directory' and does not remove them on failure. Moreover, it
> does not work in a container where git is not installed:
>
> Debugger entered--Lisp error: (file-missing "Searching for program" "No
> such file or directory" "git")
>
> that is called from `find-file-hook'.
Is second attachment appropriate to fix the issue or it has some
undesired effects.
[-- Attachment #2: 0002-test-org.el-LaTeX-to-MathML-tests-of-shell-escaping.patch --]
[-- Type: text/x-patch, Size: 4128 bytes --]
From 73541dbeafff47f03d4aa2f47da70ba71d5b8253 Mon Sep 17 00:00:00 2001
From: Max Nikulin <manikulin@gmail.com>
Date: Sun, 10 Mar 2024 11:16:46 +0700
Subject: [PATCH 2/3] test-org.el: LaTeX to MathML tests of shell escaping
* testing/lisp/test-org.el (test-org/format-latex-as-html)
(test-org/create-math-formula): New tests for escaping of shell specials
in commands executed by `org-format-latex-as-html'
and `org-create-math-formula'.
These tests do not require applications for conversion of LaTeX
snippets and use simple shell commands instead.
---
testing/lisp/test-org.el | 75 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 75 insertions(+)
diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el
index cecf9b412..652fc0676 100644
--- a/testing/lisp/test-org.el
+++ b/testing/lisp/test-org.el
@@ -9590,6 +9590,81 @@ (ert-deftest test-org/org--open-file-format-command ()
(string-match-p "\\`Invalid format.*%2" err-text))
err)))))
+\f
+;;; LaTeX-related functions.
+
+(ert-deftest test-org/format-latex-as-html ()
+ "Test shell special characters escaping in `org-format-latex-as-html'."
+ (let ((org-latex-to-html-convert-command
+ "printf \"<I%%sI>\" %i"))
+ ;; No backslashes added by `shell-quote-argumet'
+ ;; are leaked to command arguments. See e.g. dash(1) "Double Quotes":
+ ;;
+ ;; The backslash inside double quotes is historically weird,
+ ;; and serves to quote only the following characters:
+ ;; $ ` " \ <newline>.
+ ;; Otherwise it remains literal.
+ ;;
+ ;; Actually extra backslashes may appear if a user adds
+ ;; double quotes around "%i", however it is not subject
+ ;; of this test.
+ (should
+ (equal "<I(|)`[[\\]]{}#$'!I>"
+ (org-format-latex-as-html "(|)`[[\\]]{}#$'!")))
+ ;; The following tests generate shell errors if %i
+ ;; substitution is not passed throuhg `shell-quote-argument'.
+ ;; Multiple words.
+ (should
+ (equal "<Iwords ; |I>"
+ (org-format-latex-as-html "words ; |")))
+ ;; Bypass single quote.
+ ;; The same snippet causes shell error if '%i' is wrapped
+ ;; in single quotes in user configuration.
+ (should
+ (equal "<Iapostrophe' ; |I>"
+ (org-format-latex-as-html "apostrophe' ; |")))
+ ;; Bypass double quote.
+ ;; Double quotes around "%i" in user configuration leads
+ ;; to extra backslashes (see above), however likely
+ ;; such user error can not cause execution of the argument
+ ;; as raw shell commands.
+ (should
+ (equal "<Iquote\" ; |I>"
+ (org-format-latex-as-html "quote\" ; |")))))
+
+(defun test-org/extract-mathml-math (xml)
+ "Extract body from result of `org-create-math-formula'."
+ (and (string-match "<math[^>]*>\\(\\(?:.\\|\n\\)*\\)</math>" xml)
+ (match-string 1 xml)))
+
+(ert-deftest test-org/create-math-formula ()
+ "Test shell special characters escaping in `org-create-math-formula'."
+ ;; The function requires <math>...</math> elements.
+ (let ((org-latex-to-mathml-convert-command
+ "printf \"<math xmlns=\\\"http://www.w3.org/1998/Math/MathML\\\"><I%%sI></math>\" %i >%o"))
+ ;; See comments in `test-org/format-latex-as-html'.
+ ;;
+ ;; No backslashes added by `shell-quote-argumet'
+ ;; are leaked to command arguments.
+ (should (equal "<I(|)`[[\\]]{}#$'!I>"
+ (test-org/extract-mathml-math
+ (org-create-math-formula "(|)`[[\\]]{}#$'!"))))
+ ;; Multiple words.
+ (should
+ (equal "<Iwords ; |I>"
+ (test-org/extract-mathml-math
+ (org-create-math-formula "words ; |"))))
+ ;; Bypass single quote.
+ (should
+ (equal "<Iapostrophe' ; |I>"
+ (test-org/extract-mathml-math
+ (org-create-math-formula "apostrophe' ; |"))))
+ ;; Bypass double quote.
+ (should
+ (equal "<Iquote\" ; |I>"
+ (test-org/extract-mathml-math
+ (org-create-math-formula "quote\" ; |"))))))
+
(provide 'test-org)
;;; test-org.el ends here
--
2.39.2
[-- Attachment #3: 0003-org.el-Avoid-find-file-no-select-during-MathML-gener.patch --]
[-- Type: text/x-patch, Size: 1668 bytes --]
From cb5d20b54349dabea25241072feca4822ae0e77d Mon Sep 17 00:00:00 2001
From: Max Nikulin <manikulin@gmail.com>
Date: Sun, 10 Mar 2024 11:22:15 +0700
Subject: [PATCH 3/3] org.el: Avoid `find-file-no-select' during MathML
generation
* lisp/org.el (org-create-math-formula): Bypass `find-file-hook' during
reading MathML files.
At least in Emacs-28 calling `find-file-noselect' for a file in a
directory under git control when git is not installed causes failures:
Lisp error:
(file-missing "Searching for program" "No such file or directory" "git")
`org-create-math-formula' uses document `default-directory' for
temporary files for conversion process input and output.
I have faced an issue with Org mode unit tests running in a minimal
container with no git installed.
---
lisp/org.el | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/lisp/org.el b/lisp/org.el
index a00d50c51..caf1bfa67 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -16215,14 +16215,15 @@ (defun org-create-math-formula (latex-frag &optional mathml-file)
(setq shell-command-output (shell-command-to-string cmd))
(setq mathml
(when (file-readable-p tmp-out-file)
- (with-current-buffer (find-file-noselect tmp-out-file t)
+ (with-temp-buffer
+ (insert-file-contents tmp-out-file)
(goto-char (point-min))
(when (re-search-forward
(format "<math[^>]*?%s[^>]*?>\\(.\\|\n\\)*</math>"
(regexp-quote
"xmlns=\"http://www.w3.org/1998/Math/MathML\""))
nil t)
- (prog1 (match-string 0) (kill-buffer))))))
+ (match-string 0)))))
(cond
(mathml
(setq mathml
--
2.39.2
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: Warn about shell-expansion in the docstring of org-latex-to-html-convert-command
2024-03-09 15:23 ` Max Nikulin
2024-03-10 5:02 ` [PATCH] Unit tests for function calling MathML converters (Re: Warn about shell-expansion in the docstring of org-latex-to-html-convert-command) Max Nikulin
@ 2024-03-12 13:03 ` Ihor Radchenko
2024-03-13 14:27 ` Max Nikulin
2024-03-31 8:25 ` Warn about shell-expansion in the docstring of org-latex-to-html-convert-command Ihor Radchenko
2 siblings, 1 reply; 33+ messages in thread
From: Ihor Radchenko @ 2024-03-12 13:03 UTC (permalink / raw)
To: Max Nikulin; +Cc: emacs-orgmode
Max Nikulin <manikulin@gmail.com> writes:
>> Even stripping quotes is unreliable when we use the example from
>> docstring: 'literal:%i'.
>
> My idea is to recognize this case. If stripping is not performed then it
> is necessary to detect if user command is safe. Otherwise apostrophe in
> a formula (even after escaping) may cause leaking math to shell. I have
> not figured out if it is possible to bypass double quotes, but extra
> slashes may distort math expression.
>
> It is trivial to cause shell failure when single quotes are used around
> %i. I am in doubts concerning double quotes. Perhaps stripping them is
> more reliable.
May you list the cases to you propose to recognize?
>> Attaching tentative patch that fixes the problem.
>
> I think it is in the right direction.
> - Manual needs update as well.
Yes,
#+begin_src emacs-lisp
(setq org-latex-to-mathml-convert-command
"latexmlmath \"%i\" --presentationmathml=%o")
#+end_src
example in "LaTeX math snippets" section should be updated. (note to self)
> - I would explicitly stress that quotes causes undefined or even
> dangerous behavior. See e.g. the last paragraph
> https://specifications.freedesktop.org/desktop-entry-spec/latest/ar01s07.html
In ORG-NEWS?
> - I expected it as bugfix.
It is a breaking change.
Also, only users who customized the variable may be prone to unexpected
shell expansion. So, I do not see it as a critical bug.
Hence, not for bugfix.
> I have tried to add some unit tests, but I faced an issue with
> `org-create-math-formula'. It creates temporary files in
> `default-directory' and does not remove them on failure. Moreover, it
> does not work in a container where git is not installed:
> ...
> Debugger entered--Lisp error: (file-missing "Searching for program" "No
> such file or directory" "git")
>
> that is called from `find-file-hook'.
with emacs -Q?
--
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] 33+ messages in thread
* Re: Warn about shell-expansion in the docstring of org-latex-to-html-convert-command
2024-03-12 13:03 ` Warn about shell-expansion in the docstring of org-latex-to-html-convert-command Ihor Radchenko
@ 2024-03-13 14:27 ` Max Nikulin
2024-03-15 13:49 ` Ihor Radchenko
0 siblings, 1 reply; 33+ messages in thread
From: Max Nikulin @ 2024-03-13 14:27 UTC (permalink / raw)
To: emacs-orgmode
On 12/03/2024 20:03, Ihor Radchenko wrote:
> Max Nikulin writes:
>> It is trivial to cause shell failure when single quotes are used around
>> %i. I am in doubts concerning double quotes. Perhaps stripping them is
>> more reliable.
>
> May you list the cases to you propose to recognize?
Sun, 25 Feb 2024 17:41:43 +0700
https://list.orgmode.org/6e49c590-ad27-4fb0-b1f2-6a89c60a0b58@gmail.com
- '%i' and "%i" in any position including e.g. --option='%i' and
protocol:"%i"
- 'something%i' and "something%i" surrounded by spaces or at the end of
command but with no spaces in "something". It should be applied to %%%i,
but not to %%i.
>> - I would explicitly stress that quotes causes undefined or even
>> dangerous behavior. See e.g. the last paragraph
>> https://specifications.freedesktop.org/desktop-entry-spec/latest/ar01s07.html
>
> In ORG-NEWS?
Since quotes were recommended in the manual and the docscring, it is
better to say there that they lead to undefined behavior. ORG-NEWS is to
make the change visible for those who have read docs earlier.
>> - I expected it as bugfix.
>
> It is a breaking change.
> Also, only users who customized the variable may be prone to unexpected
> shell expansion. So, I do not see it as a critical bug.
> Hence, not for bugfix.
I am still in doubts. I have no idea how much users need ODT export with
math and rely on the backend shipped with Org. All of them have to
customize the user option and those who added %i with quotes have risk
to get incorrect output. If quotes around %i are stripped then the
change is not breaking one for most of them.
>> Moreover, it
>> does not work in a container where git is not installed:
>> ...
>> Debugger entered--Lisp error: (file-missing "Searching for program" "No
>> such file or directory" "git")
>
> with emacs -Q?
emacs -Q --batch --eval '(find-file-noselect "not-found.txt" t)'
Starting new Ispell process ispell with default dictionary... \
Error enabling Flyspell mode:
(Searching for program No such file or directory ispell)
Error: (file-missing "Searching for program" "No such file or directory"
"git")
Emacs-28.2
ispell error is due to my .dir-locals-2.el
(text-mode . ((mode . flyspell)))
bug to get the git error it is enough to create .git subdirectory.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Warn about shell-expansion in the docstring of org-latex-to-html-convert-command
2024-03-13 14:27 ` Max Nikulin
@ 2024-03-15 13:49 ` Ihor Radchenko
2024-03-18 10:50 ` Max Nikulin
0 siblings, 1 reply; 33+ messages in thread
From: Ihor Radchenko @ 2024-03-15 13:49 UTC (permalink / raw)
To: Max Nikulin; +Cc: emacs-orgmode
Max Nikulin <manikulin@gmail.com> writes:
> On 12/03/2024 20:03, Ihor Radchenko wrote:
>> Max Nikulin writes:
>>> It is trivial to cause shell failure when single quotes are used around
>>> %i. I am in doubts concerning double quotes. Perhaps stripping them is
>>> more reliable.
>>
>> May you list the cases to you propose to recognize?
>
> Sun, 25 Feb 2024 17:41:43 +0700
> https://list.orgmode.org/6e49c590-ad27-4fb0-b1f2-6a89c60a0b58@gmail.com
>
> - '%i' and "%i" in any position including e.g. --option='%i' and
> protocol:"%i"
> - 'something%i' and "something%i" surrounded by spaces or at the end of
> command but with no spaces in "something".
I am not confident that it will be safe. For example, consider something
awkward like foo\"%ibar\". I imagine that other edge cases are possible,
especially in exotic shells.
> ... It should be applied to %%%i,
> but not to %%i.
I am not sure what you mean here.
>>> - I expected it as bugfix.
>>
>> It is a breaking change.
>> Also, only users who customized the variable may be prone to unexpected
>> shell expansion. So, I do not see it as a critical bug.
>> Hence, not for bugfix.
>
> I am still in doubts. I have no idea how much users need ODT export with
> math and rely on the backend shipped with Org. All of them have to
> customize the user option and those who added %i with quotes have risk
> to get incorrect output. If quotes around %i are stripped then the
> change is not breaking one for most of them.
It does not matter that most users will not be affected. Some users
being affected is enough to not commit this to bugfix. Our policy is not
to commit unsafe changes that may break existing configurations to
bugfix branch. Except critical fixes. See
https://orgmode.org/worg/org-maintenance.html#release-types
>>> Moreover, it
>>> does not work in a container where git is not installed:
>>> ...
>>> Debugger entered--Lisp error: (file-missing "Searching for program" "No
>>> such file or directory" "git")
>>
>> with emacs -Q?
>
> emacs -Q --batch --eval '(find-file-noselect "not-found.txt" t)'
> Starting new Ispell process ispell with default dictionary... \
> Error enabling Flyspell mode:
> (Searching for program No such file or directory ispell)
> Error: (file-missing "Searching for program" "No such file or directory"
> "git")
>
> Emacs-28.2
>
> ispell error is due to my .dir-locals-2.el
> (text-mode . ((mode . flyspell)))
> bug to get the git error it is enough to create .git subdirectory.
This looks like Emacs bug. Likely in `vc-refresh-state'.
--
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] 33+ messages in thread
* Re: Warn about shell-expansion in the docstring of org-latex-to-html-convert-command
2024-03-15 13:49 ` Ihor Radchenko
@ 2024-03-18 10:50 ` Max Nikulin
2024-03-19 14:48 ` Ihor Radchenko
0 siblings, 1 reply; 33+ messages in thread
From: Max Nikulin @ 2024-03-18 10:50 UTC (permalink / raw)
To: emacs-orgmode
On 15/03/2024 20:49, Ihor Radchenko wrote:
> Max Nikulin writes:
>> On 12/03/2024 20:03, Ihor Radchenko wrote:
>> - '%i' and "%i" in any position including e.g. --option='%i' and
>> protocol:"%i"
>> - 'something%i' and "something%i" surrounded by spaces or at the end of
>> command but with no spaces in "something".
>
> I am not confident that it will be safe. For example, consider something
> awkward like foo\"%ibar\". I imagine that other edge cases are possible,
> especially in exotic shells.
I think quotes should not be stripped in such peculiar cases. The
variants I suggested do not match it. Is it realistic?
>> ... It should be applied to %%%i,
>> but not to %%i.
>
> I am not sure what you mean here.
"%%" is a way to specify literal "%" in `format-spec'. So '%%i' means in
%i shell command and unquoting should not be applied to it.
>>>> - I expected it as bugfix.
>
> It does not matter that most users will not be affected. Some users
> being affected is enough to not commit this to bugfix. Our policy is not
> to commit unsafe changes that may break existing configurations to
> bugfix branch. Except critical fixes.
Reasons why I consider this issue a severe enough:
- Something weird may be executed as shell commands
- Incorrect formulas in exported documents are more than just
disappointment. An example of complain related to another bug:
Re: Inequalities in math blocks. Wed, 06 Oct 2021 09:39:23 +0200.
https://list.orgmode.org/m2bl42bo0k.fsf@me.com
From my point of view, it is better to explain users that they are
disturbed to be on the safe side. It is not choice between good and bad
variants. Any decision is bad.
>> emacs -Q --batch --eval '(find-file-noselect "not-found.txt" t)'
>> Error: (file-missing "Searching for program" "No such file or directory"
>> "git")
>
> This looks like Emacs bug. Likely in `vc-refresh-state'.
It as an Emacs bug that missing git executable leads to a fatal error.
It is a bug in Org that some hooks are called when just file content is
necessary.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Warn about shell-expansion in the docstring of org-latex-to-html-convert-command
2024-03-18 10:50 ` Max Nikulin
@ 2024-03-19 14:48 ` Ihor Radchenko
2024-03-19 14:49 ` Ihor Radchenko
0 siblings, 1 reply; 33+ messages in thread
From: Ihor Radchenko @ 2024-03-19 14:48 UTC (permalink / raw)
To: Max Nikulin; +Cc: emacs-orgmode
Max Nikulin <manikulin@gmail.com> writes:
>>> On 12/03/2024 20:03, Ihor Radchenko wrote:
>>> - '%i' and "%i" in any position including e.g. --option='%i' and
>>> protocol:"%i"
>>> - 'something%i' and "something%i" surrounded by spaces or at the end of
>>> command but with no spaces in "something".
>>
>> I am not confident that it will be safe. For example, consider something
>> awkward like foo\"%ibar\". I imagine that other edge cases are possible,
>> especially in exotic shells.
>
> I think quotes should not be stripped in such peculiar cases. The
> variants I suggested do not match it. Is it realistic?
I am afraid that there are other peculiar cases. I do not know how to
determine which case is peculiar and when it is safe to strip the quotes
in the code. I feel that if we do try to strip only "safe" cases, we
will introduce subtle bugs and then introduce even more breaking changes
by fixing those bugs.
It is more robust to not strip the quotes at all and go ahead with
breaking change.
>>>>> - I expected it as bugfix.
>>
>> It does not matter that most users will not be affected. Some users
>> being affected is enough to not commit this to bugfix. Our policy is not
>> to commit unsafe changes that may break existing configurations to
>> bugfix branch. Except critical fixes.
>
> Reasons why I consider this issue a severe enough:
> - Something weird may be executed as shell commands
> - Incorrect formulas in exported documents are more than just
> disappointment. An example of complain related to another bug:
> Re: Inequalities in math blocks. Wed, 06 Oct 2021 09:39:23 +0200.
> https://list.orgmode.org/m2bl42bo0k.fsf@me.com
I do not see these reasons as _critical_. In my mind, critical reasons
would be (1) Org mode completely broken for many users (it is not); (2)
Security vulnerability.
This particular case seems to be subjective, so it is a judgment call.
If you insist that the fix should land on bugfix, we can add Bastien to
the discussion to get a third opinion.
>>> emacs -Q --batch --eval '(find-file-noselect "not-found.txt" t)'
>>> Error: (file-missing "Searching for program" "No such file or directory"
>>> "git")
>>
>> This looks like Emacs bug. Likely in `vc-refresh-state'.
>
> It as an Emacs bug that missing git executable leads to a fatal error.
>
> It is a bug in Org that some hooks are called when just file content is
> necessary.
I would not necessarily call it a bug, but I do not see downsides of
using `insert-file-contents' instead of `find-file-noselect' and not
running `find-file-hook' in this particular case (other cases in Org
tree appears to be fine from a quick glance).
--
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] 33+ messages in thread
* Re: Warn about shell-expansion in the docstring of org-latex-to-html-convert-command
2024-03-19 14:48 ` Ihor Radchenko
@ 2024-03-19 14:49 ` Ihor Radchenko
2024-03-19 16:22 ` Max Nikulin
0 siblings, 1 reply; 33+ messages in thread
From: Ihor Radchenko @ 2024-03-19 14:49 UTC (permalink / raw)
To: Max Nikulin; +Cc: emacs-orgmode
Ihor Radchenko <yantar92@posteo.net> writes:
>> It is a bug in Org that some hooks are called when just file content is
>> necessary.
>
> I would not necessarily call it a bug, but I do not see downsides of
> using `insert-file-contents' instead of `find-file-noselect' and not
> running `find-file-hook' in this particular case (other cases in Org
> tree appears to be fine from a quick glance).
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=b338a9069
--
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] 33+ messages in thread
* Re: Warn about shell-expansion in the docstring of org-latex-to-html-convert-command
2024-03-19 14:49 ` Ihor Radchenko
@ 2024-03-19 16:22 ` Max Nikulin
2024-03-19 16:27 ` Ihor Radchenko
0 siblings, 1 reply; 33+ messages in thread
From: Max Nikulin @ 2024-03-19 16:22 UTC (permalink / raw)
To: emacs-orgmode
On 19/03/2024 21:49, Ihor Radchenko wrote:
>>
>> but I do not see downsides of
>> using `insert-file-contents' instead of `find-file-noselect' and not
>> running `find-file-hook' in this particular case (other cases in Org
>> tree appears to be fine from a quick glance).
>
> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=b338a9069
`with-temp-buffer' makes `kill-buffer' unnecessary.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Warn about shell-expansion in the docstring of org-latex-to-html-convert-command
2024-03-19 16:22 ` Max Nikulin
@ 2024-03-19 16:27 ` Ihor Radchenko
2024-03-19 16:45 ` fixup! and git Max Nikulin
0 siblings, 1 reply; 33+ messages in thread
From: Ihor Radchenko @ 2024-03-19 16:27 UTC (permalink / raw)
To: Max Nikulin; +Cc: emacs-orgmode
Max Nikulin <manikulin@gmail.com> writes:
>> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=b338a9069
>
> `with-temp-buffer' makes `kill-buffer' unnecessary.
Oops.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=4f548f948
--
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] 33+ messages in thread
* fixup! and git
2024-03-19 16:27 ` Ihor Radchenko
@ 2024-03-19 16:45 ` Max Nikulin
2024-03-19 16:50 ` Ihor Radchenko
0 siblings, 1 reply; 33+ messages in thread
From: Max Nikulin @ 2024-03-19 16:45 UTC (permalink / raw)
To: emacs-orgmode
On 19/03/2024 23:27, Ihor Radchenko wrote:
> Oops.
> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=4f548f948
>
> fixup! org-create-math-formula: Do not run file-related hooks when checking output
"fixup!" is a keyword for "git rebase --interactive --autosquash" and
usually added for minor correction of commits that have not been pushed
yet to a shared repository. It is a bit confusing to see it in a public
repository in a branch that is not a draft.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: fixup! and git
2024-03-19 16:45 ` fixup! and git Max Nikulin
@ 2024-03-19 16:50 ` Ihor Radchenko
0 siblings, 0 replies; 33+ messages in thread
From: Ihor Radchenko @ 2024-03-19 16:50 UTC (permalink / raw)
To: Max Nikulin; +Cc: emacs-orgmode
Max Nikulin <manikulin@gmail.com> writes:
>> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=4f548f948
>>
>> fixup! org-create-math-formula: Do not run file-related hooks when checking output
>
> "fixup!" is a keyword for "git rebase --interactive --autosquash" and
> usually added for minor correction of commits that have not been pushed
> yet to a shared repository. It is a bit confusing to see it in a public
> repository in a branch that is not a draft.
I personally find it more clear compared for something like "Fix <commit
hash>" or "Fix previous commit".
--
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] 33+ messages in thread
* Re: Warn about shell-expansion in the docstring of org-latex-to-html-convert-command
2024-03-09 15:23 ` Max Nikulin
2024-03-10 5:02 ` [PATCH] Unit tests for function calling MathML converters (Re: Warn about shell-expansion in the docstring of org-latex-to-html-convert-command) Max Nikulin
2024-03-12 13:03 ` Warn about shell-expansion in the docstring of org-latex-to-html-convert-command Ihor Radchenko
@ 2024-03-31 8:25 ` Ihor Radchenko
2024-04-01 10:29 ` Max Nikulin
2 siblings, 1 reply; 33+ messages in thread
From: Ihor Radchenko @ 2024-03-31 8:25 UTC (permalink / raw)
To: Max Nikulin; +Cc: emacs-orgmode
[-- Attachment #1: Type: text/plain, Size: 453 bytes --]
Max Nikulin <manikulin@gmail.com> writes:
>> Attaching tentative patch that fixes the problem.
>
> I think it is in the right direction.
> - Manual needs update as well.
> - I would explicitly stress that quotes causes undefined or even
> dangerous behavior. See e.g. the last paragraph
> https://specifications.freedesktop.org/desktop-entry-spec/latest/ar01s07.html
I have incorporated the above suggestions into the attached version of
the patch.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-latex-to-mathml-html-convert-command-Prevent-she.patch --]
[-- Type: text/x-patch, Size: 5262 bytes --]
From 5dbe4457d0d938e8830888bc3ac58d6a43136558 Mon Sep 17 00:00:00 2001
Message-ID: <5dbe4457d0d938e8830888bc3ac58d6a43136558.1711873441.git.yantar92@posteo.net>
From: Ihor Radchenko <yantar92@posteo.net>
Date: Fri, 8 Mar 2024 14:05:12 +0300
Subject: [PATCH] org-latex-to-mathml/html-convert-command: Prevent shell
expansion
* lisp/org.el (org-create-math-formula):
(org-format-latex-as-html): Shell-quote LaTeX fragment text when
replacing %i placeholder. This prevents shell expansion of
$... and similar constructs inside the code.
(org-latex-to-mathml-convert-command):
(org-latex-to-html-convert-command): Update the docstring.
* etc/ORG-NEWS (~org-latex-to-mathml-convert-command~ and
~org-latex-to-html-convert-command~ shell-escape LaTeX code): Announce
the breaking change.
* doc/org-manual.org (LaTeX math snippets): Update example.
Reported-by: Max Nikulin <manikulin@gmail.com>
Link: https://orgmode.org/list/735645dd-1ddf-4579-a6dd-2700f3e83c94@gmail.com
---
doc/org-manual.org | 2 +-
etc/ORG-NEWS | 10 ++++++++++
lisp/org.el | 21 ++++++++++-----------
3 files changed, 21 insertions(+), 12 deletions(-)
diff --git a/doc/org-manual.org b/doc/org-manual.org
index c4f62644f..acc4512a5 100644
--- a/doc/org-manual.org
+++ b/doc/org-manual.org
@@ -15176,7 +15176,7 @@ **** LaTeX math snippets
#+begin_src emacs-lisp
(setq org-latex-to-mathml-convert-command
- "latexmlmath \"%i\" --presentationmathml=%o")
+ "latexmlmath %i --presentationmathml=%o")
#+end_src
To quickly verify the reliability of the LaTeX-to-MathML
diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index ee2cdfd16..739c3a43b 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -13,6 +13,16 @@ Please send Org bug reports to mailto:emacs-orgmode@gnu.org.
* Version 9.7 (not released yet)
** Important announcements and breaking changes
+*** ~org-latex-to-mathml-convert-command~ and ~org-latex-to-html-convert-command~ shell-escape LaTeX code
+
+Previously, ~org-latex-to-mathml-convert-command~ and
+~org-latex-to-html-convert-command~ replaced %i placeholders with raw
+LaTeX fragment text, potentially triggering shell-expansion.
+
+Now, the %i placeholders are shell-escaped to prevent shell expansion.
+
+The existing customizations that assume no shell-escaping must be updated.
+
*** Built-in HTML, LaTeX, Man, Markdown, ODT, and Texinfo exporters preserve the link protocol during export
Previously, some link types where not exported as =protocol:uri= but
diff --git a/lisp/org.el b/lisp/org.el
index f3fae134d..f56767a1a 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -3266,7 +3266,9 @@ (defcustom org-latex-to-mathml-convert-command nil
%j: Executable file in fully expanded form as specified by
`org-latex-to-mathml-jar-file'.
%I: Input LaTeX file in fully expanded form.
-%i: The latex fragment to be converted.
+%i: Shell-escaped LaTeX fragment to be converted.
+ It must not be used inside a quoted argument, the result of %i
+ expansion inside a quoted argument is undefined.
%o: Output MathML file.
This command is used by `org-create-math-formula'.
@@ -3275,7 +3277,7 @@ (defcustom org-latex-to-mathml-convert-command nil
\"java -jar %j -unicode -force -df %o %I\".
When using LaTeXML set this option to
-\"latexmlmath \"%i\" --presentationmathml=%o\"."
+\"latexmlmath %i --presentationmathml=%o\"."
:group 'org-latex
:version "24.1"
:type '(choice
@@ -3288,15 +3290,12 @@ (defcustom org-latex-to-html-convert-command nil
directly replace the LaTeX fragment in the resulting HTML.
Replace format-specifiers in the command as noted below and use
`shell-command' to convert LaTeX to HTML.
-%i: The LaTeX fragment to be converted.
+%i: The LaTeX fragment to be converted (shell-escaped).
+ It must not be used inside a quoted argument, the result of %i
+ expansion inside a quoted argument is undefined.
For example, this could be used with LaTeXML as
-\"latexmlc \\='literal:%i\\=' --profile=math --preload=siunitx.sty 2>/dev/null\".
-
-The LaTeX fragment is replaced as is, without escaping special shell
-syntax. It may be necessary to use single-quotes around \\='%i\\=', not
-double-quotes. Else a math fragment such as \"$y = 200$\" may be
-expanded to \" = 200\"."
+\"latexmlc literal:%i --profile=math --preload=siunitx.sty 2>/dev/null\"."
:group 'org-latex
:package-version '(Org . "9.4")
:type '(choice
@@ -16332,7 +16331,7 @@ (defun org-create-math-formula (latex-frag &optional mathml-file)
(expand-file-name
org-latex-to-mathml-jar-file))))
(?I . ,(shell-quote-argument tmp-in-file))
- (?i . ,latex-frag)
+ (?i . ,(shell-quote-argument latex-frag))
(?o . ,(shell-quote-argument tmp-out-file)))))
mathml shell-command-output)
(when (called-interactively-p 'any)
@@ -16400,7 +16399,7 @@ (defun org-format-latex-as-html (latex-fragment)
"Convert LATEX-FRAGMENT to HTML.
This uses `org-latex-to-html-convert-command', which see."
(let ((cmd (format-spec org-latex-to-html-convert-command
- `((?i . ,latex-fragment)))))
+ `((?i . ,(shell-quote-argument latex-fragment))))))
(message "Running %s" cmd)
(shell-command-to-string cmd)))
--
2.44.0
[-- Attachment #3: Type: text/plain, Size: 224 bytes --]
--
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 related [flat|nested] 33+ messages in thread
* Re: [PATCH] Unit tests for function calling MathML converters (Re: Warn about shell-expansion in the docstring of org-latex-to-html-convert-command)
2024-03-10 5:02 ` [PATCH] Unit tests for function calling MathML converters (Re: Warn about shell-expansion in the docstring of org-latex-to-html-convert-command) Max Nikulin
@ 2024-03-31 8:27 ` Ihor Radchenko
2024-04-01 10:39 ` Max Nikulin
0 siblings, 1 reply; 33+ messages in thread
From: Ihor Radchenko @ 2024-03-31 8:27 UTC (permalink / raw)
To: Max Nikulin; +Cc: emacs-orgmode
Max Nikulin <manikulin@gmail.com> writes:
> Is second attachment appropriate to fix the issue or it has some
> undesired effects.
Thanks!
> +(ert-deftest test-org/format-latex-as-html ()
> + "Test shell special characters escaping in `org-format-latex-as-html'."
> + (let ((org-latex-to-html-convert-command
> + "printf \"<I%%sI>\" %i"))
I have a concern about this test - it will not work on windows or in
non-standard system shells. We should probably disable the test unless
"printf" can be evaluated in the current system shell.
--
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] 33+ messages in thread
* Re: Warn about shell-expansion in the docstring of org-latex-to-html-convert-command
2024-03-31 8:25 ` Warn about shell-expansion in the docstring of org-latex-to-html-convert-command Ihor Radchenko
@ 2024-04-01 10:29 ` Max Nikulin
2024-04-01 11:15 ` Ihor Radchenko
0 siblings, 1 reply; 33+ messages in thread
From: Max Nikulin @ 2024-04-01 10:29 UTC (permalink / raw)
To: emacs-orgmode
On 31/03/2024 15:25, Ihor Radchenko wrote:
> Max Nikulin writes:
>>
>> I think it is in the right direction.
>> - Manual needs update as well.
>> - I would explicitly stress that quotes causes undefined or even
>> dangerous behavior. See e.g. the last paragraph
>> https://specifications.freedesktop.org/desktop-entry-spec/latest/ar01s07.html
>
> I have incorporated the above suggestions into the attached version of
> the patch.
Thanks, I have not tried the updated patch in action, but it looks like
what I expect.
> +++ b/etc/ORG-NEWS
> @@ -13,6 +13,16 @@ Please send Org bug reports to mailto:emacs-orgmode@gnu.org.
>
> * Version 9.7 (not released yet)
> ** Important announcements and breaking changes
> +*** ~org-latex-to-mathml-convert-command~ and ~org-latex-to-html-convert-command~ shell-escape LaTeX code
> +
> +Previously, ~org-latex-to-mathml-convert-command~ and
> +~org-latex-to-html-convert-command~ replaced %i placeholders with raw
> +LaTeX fragment text, potentially triggering shell-expansion.
> +
> +Now, the %i placeholders are shell-escaped to prevent shell expansion.
> +
> +The existing customizations that assume no shell-escaping must be updated.
> +
I would consider explicit mention of stripping quotes
+Previously, =%i= placeholders in the ~org-latex-to-mathml-convert-command~
and ~org-latex-to-html-convert-command~ user options were replaced
with raw LaTeX fragment text, potentially triggering shell-expansion
and incorrect result.
Now, the =%i= placeholders are shell-escaped to prevent shell expansion.
If you have single or double quotes around =%i= then update
customizations and remove quotes.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] Unit tests for function calling MathML converters (Re: Warn about shell-expansion in the docstring of org-latex-to-html-convert-command)
2024-03-31 8:27 ` Ihor Radchenko
@ 2024-04-01 10:39 ` Max Nikulin
2024-04-01 11:23 ` Ihor Radchenko
0 siblings, 1 reply; 33+ messages in thread
From: Max Nikulin @ 2024-04-01 10:39 UTC (permalink / raw)
To: emacs-orgmode
On 31/03/2024 15:27, Ihor Radchenko wrote:
> Max Nikulin writes:
>
>> +(ert-deftest test-org/format-latex-as-html ()
>> + "Test shell special characters escaping in `org-format-latex-as-html'."
>> + (let ((org-latex-to-html-convert-command
>> + "printf \"<I%%sI>\" %i"))
>
> I have a concern about this test - it will not work on windows or in
I do not mind to add
(skip-unless (not (memq system-type '(ms-dos windows-nt))))
> non-standard system shells. We should probably disable the test unless
> "printf" can be evaluated in the current system shell.
POSIX printf is more portable than "echo". Anyway Makefile expects a
POSIX compatible shell. So I believe, it is developer responsibility to
run build and test with a sane shell even if they prefer something
unusual as the interactive shell.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Warn about shell-expansion in the docstring of org-latex-to-html-convert-command
2024-04-01 10:29 ` Max Nikulin
@ 2024-04-01 11:15 ` Ihor Radchenko
0 siblings, 0 replies; 33+ messages in thread
From: Ihor Radchenko @ 2024-04-01 11:15 UTC (permalink / raw)
To: Max Nikulin; +Cc: emacs-orgmode
Max Nikulin <manikulin@gmail.com> writes:
>> I have incorporated the above suggestions into the attached version of
>> the patch.
>
> Thanks, I have not tried the updated patch in action, but it looks like
> what I expect.
> I would consider explicit mention of stripping quotes
> ...
I incorporated your suggestions and applied the updated version of the
patch.
Fixed, on main.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=a698d073a
--
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] 33+ messages in thread
* Re: [PATCH] Unit tests for function calling MathML converters (Re: Warn about shell-expansion in the docstring of org-latex-to-html-convert-command)
2024-04-01 10:39 ` Max Nikulin
@ 2024-04-01 11:23 ` Ihor Radchenko
0 siblings, 0 replies; 33+ messages in thread
From: Ihor Radchenko @ 2024-04-01 11:23 UTC (permalink / raw)
To: Max Nikulin; +Cc: emacs-orgmode
Max Nikulin <manikulin@gmail.com> writes:
>> I have a concern about this test - it will not work on windows or in
>
> I do not mind to add
>
> (skip-unless (not (memq system-type '(ms-dos windows-nt))))
>
>> non-standard system shells. We should probably disable the test unless
>> "printf" can be evaluated in the current system shell.
>
> POSIX printf is more portable than "echo". Anyway Makefile expects a
> POSIX compatible shell. So I believe, it is developer responsibility to
> run build and test with a sane shell even if they prefer something
> unusual as the interactive shell.
Agree.
Applied, onto main, after adding skip-unless.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=a3bcb5536
--
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] 33+ messages in thread
end of thread, other threads:[~2024-04-01 11:24 UTC | newest]
Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-16 23:10 Warn about shell-expansion in the docstring of org-latex-to-html-convert-command Martin Edström
2024-02-18 16:06 ` Ihor Radchenko
2024-02-18 18:56 ` Martin Edström
2024-02-18 19:36 ` Martin Edström
2024-02-19 8:30 ` Ihor Radchenko
2024-02-21 14:38 ` Max Nikulin
2024-02-21 14:57 ` Martin Edström
2024-02-21 15:04 ` Martin Edström
2024-02-21 15:08 ` Martin Edström
2024-02-23 12:46 ` Ihor Radchenko
2024-02-25 10:41 ` Max Nikulin
2024-02-26 10:48 ` Ihor Radchenko
2024-02-26 16:37 ` Max Nikulin
2024-03-08 11:16 ` Ihor Radchenko
2024-03-09 15:23 ` Max Nikulin
2024-03-10 5:02 ` [PATCH] Unit tests for function calling MathML converters (Re: Warn about shell-expansion in the docstring of org-latex-to-html-convert-command) Max Nikulin
2024-03-31 8:27 ` Ihor Radchenko
2024-04-01 10:39 ` Max Nikulin
2024-04-01 11:23 ` Ihor Radchenko
2024-03-12 13:03 ` Warn about shell-expansion in the docstring of org-latex-to-html-convert-command Ihor Radchenko
2024-03-13 14:27 ` Max Nikulin
2024-03-15 13:49 ` Ihor Radchenko
2024-03-18 10:50 ` Max Nikulin
2024-03-19 14:48 ` Ihor Radchenko
2024-03-19 14:49 ` Ihor Radchenko
2024-03-19 16:22 ` Max Nikulin
2024-03-19 16:27 ` Ihor Radchenko
2024-03-19 16:45 ` fixup! and git Max Nikulin
2024-03-19 16:50 ` Ihor Radchenko
2024-03-31 8:25 ` Warn about shell-expansion in the docstring of org-latex-to-html-convert-command Ihor Radchenko
2024-04-01 10:29 ` Max Nikulin
2024-04-01 11:15 ` Ihor Radchenko
2024-03-05 12:01 ` Max Nikulin
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).