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 Thread Ihor Radchenko 
Max Nikulin  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 .
Support Org development at ,
or support my work at

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 Thread Max Nikulin 

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"))


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.

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 Thread Ihor Radchenko 
Max Nikulin  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"))

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 .
Support Org development at ,
or support my work at

[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 Thread Max Nikulin 

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.
From 73541dbeafff47f03d4aa2f47da70ba71d5b8253 Mon Sep 17 00:00:00 2001
From: Max Nikulin 
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)
 
+
+;;; 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"))
+;; 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:
+;; $ ` " \ .
+;; 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 ""
+(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 ""
+   (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 ""
+(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 ""
+(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 "]*>\\(\\(?:.\\|\n\\)*\\)" 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 ... elements.
+  (let ((org-latex-to-mathml-convert-command
+ "printf \"http://www.w3.org/1998/Math/MathML\\\;>\" %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 ""
+(test-org/extract-mathml-math
+ (org-create-math-formula "(|)`[[\\]]{}#$'!"
+;; Multiple words.
+(should
+ (equal ""
+(test-org/extract-mathml-math
+ (org-create-math-formula "words ; |"
+;; Bypass single quote.
+(should
+ (equal ""
+(test-org/extract-mathml-math
+ (org-create-math-formula "apostrophe' ; |"
+;; Bypass double quote.
+(should
+ (equal ""
+(test-org/extract-mathml-math
+ (org-create-math-formula "quote\" ; |"))
+
 (provide 'test-org)
 
 ;;; test-org.el ends here
-- 
2.39.2

From cb5d20b54349dabea25241072feca4822ae0e77d Mon Sep 17 00:00:00 2001
From: Max Nikulin 
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