latex3 / latex2e

The LaTeX2e kernel
https://www.latex-project.org/
LaTeX Project Public License v1.3c
1.93k stars 267 forks source link

\newenvironment allows to define an empty environment name #778

Open wgrundlingh opened 2 years ago

wgrundlingh commented 2 years ago

Brief outline of the bug

\newenvironment allows one to define an empty environment name. This causes problems as it redefines \end. Only the check is done on the "\begin" or opening part of the new environment definition, not whether the "\end" exists. The result, even if a small document is that \end{document} is not properly identified and compilation fails.

Minimal example showing the bug

\RequirePackage{latexbug}       % <--should be always the first line (see CONTRIBUTING)!
\documentclass{article}

  % Any preamble code goes here

\begin{document}

  % Demonstration of issue here

\newenvironment{}{X}{Y}

test

\end{document}

Log file (required) and possibly PDF file

This is pdfTeX, Version 3.141592653-2.6-1.40.23 (TeX Live 2021/W32TeX) (preloaded format=pdflatex 2022.1.29)  6 FEB 2022 21:47
entering extended mode
 \write18 enabled.
 %&-line parsing enabled.
**latex_stuff.tex
(./latex_stuff.tex
LaTeX2e <2021-11-15> patch level 1
L3 programming layer <2022-01-21>
(c:/texlive/2021/texmf-dist/tex/latex/base/article.cls
Document Class: article 2021/10/04 v1.4n Standard LaTeX document class
(c:/texlive/2021/texmf-dist/tex/latex/base/size10.clo
File: size10.clo 2021/10/04 v1.4n Standard LaTeX file (size option)
)
\c@part=\count185
\c@section=\count186
\c@subsection=\count187
\c@subsubsection=\count188
\c@paragraph=\count189
\c@subparagraph=\count190
\c@figure=\count191
\c@table=\count192
\abovecaptionskip=\skip47
\belowcaptionskip=\skip48
\bibindent=\dimen138
)
(c:/texlive/2021/texmf-dist/tex/latex/l3backend/l3backend-pdftex.def
File: l3backend-pdftex.def 2022-01-12 L3 backend support: PDF output (pdfTeX)
\l__color_backend_stack_int=\count193
\l__pdf_internal_box=\box50
)
(./latex_stuff.aux)
\openout1 = `latex_stuff.aux'.

LaTeX Font Info:    Checking defaults for OML/cmm/m/it on input line 3.
LaTeX Font Info:    ... okay on input line 3.
LaTeX Font Info:    Checking defaults for OMS/cmsy/m/n on input line 3.
LaTeX Font Info:    ... okay on input line 3.
LaTeX Font Info:    Checking defaults for OT1/cmr/m/n on input line 3.
LaTeX Font Info:    ... okay on input line 3.
LaTeX Font Info:    Checking defaults for T1/cmr/m/n on input line 3.
LaTeX Font Info:    ... okay on input line 3.
LaTeX Font Info:    Checking defaults for TS1/cmr/m/n on input line 3.
LaTeX Font Info:    ... okay on input line 3.
LaTeX Font Info:    Checking defaults for OMX/cmex/m/n on input line 3.
LaTeX Font Info:    ... okay on input line 3.
LaTeX Font Info:    Checking defaults for U/cmr/m/n on input line 3.
LaTeX Font Info:    ... okay on input line 3.
)
! Emergency stop.
<*> latex_stuff.tex

*** (job aborted, no legal \end found)

Here is how much of TeX's memory you used:
 395 strings out of 478287
 7364 string characters out of 5850743
 296513 words of memory out of 5000000
 18678 multiletter control sequences out of 15000+600000
 469259 words of font info for 28 fonts, out of 8000000 for 9000
 1141 hyphenation exceptions out of 8191
 34i,0n,38p,140b,36s stack positions out of 5000i,500n,10000p,200000b,80000s
!  ==> Fatal error occurred, no output PDF file produced!
davidcarlisle commented 2 years ago

Thanks for the report, the code for \newenvironment relies on \newcommand (or at least the internal \new@command) to check essentially this and throw an error that the empty command is defined.

\documentclass{article}

\expandafter\let\csname\endcsname\end
\expandafter\newcommand\csname\endcsname{whatever}

\begin{document}

\end{document}

But the if defined check fails here as \string on the empty csname is \csname\endcsname not empty.

This could be fixed in various ways, as always it probably comes down to deciding how much change can be done here for compatibility.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity.

PhelypeOleinik commented 2 years ago

In short, the issue happens (mostly) because (the equivalent of) \cs_to_str:N on the empty csname (\csname\endcsname) yields csname\endcsname instead of an empty string. Then, testing \@ifundefined{csname\endcsname} tests the existence of something else entirely. I think the best way to fix this issue is to change the definition of \@ifdefinable to:

\long\def\@ifdefinable #1#2{%
  \edef\reserved@a{\expandafter\@gobble\string #1}%
  \@expl@cs@if@free@@NTF #1%  <--------  replaced |\@ifundefined\reserved@a|
    {\edef\reserved@b{\expandafter\@carcube \reserved@a xxx\@nil}%
     \ifx \reserved@b\@qend \@notdefinable\else
       \ifx \reserved@a\@qrelax \@notdefinable\else
         #2%
       \fi
     \fi}%
    \@notdefinable}

that is, replace \@ifundefined{name} by \cs_if_free:NTF \name, so that the code doesn't trip over its shoelaces with the empty csname. The change should cause no harm, as the actual test done is the same after all.

This bug also affects \newcommand, in such a way that you can redefine the empty control sequence without an error:

\def\test#1{%
  \def#1{A}%
  \newcommand#1{B}%
  \show#1%
}
\expandafter\test\csname\endcsname % no error and shows B
            \test\anything % error and shows A
muzimuzhi commented 2 years ago

(the equivalent of) \cs_to_str:N on the empty csname (\csname\endcsname) yields csname\endcsname instead of an empty string

Should \cs_to_str:N take empty csname into consideration? Or, what's the expected result of \exp_args:Nc \cs_to_str:N {}? Maybe I should open an issue in latex3.

PhelypeOleinik commented 2 years ago

@muzimuzhi It would be nice, but I don't think it can do that in a reasonably efficient way, because the only ways I know to do that expandably are a string comparison or a macro delimited by "csname\endcsname". The biggest problem though is that after hit with \string, the empty csname and a the control sequence called \csname\endcsname are indistinguishable (if the character between \csname and \endcsname is the same as the current \escapechar), so expandably the test is not completely reliable either (happy to be proven wrong :). I think it's not worth it...

Then answering your question:

what's the expected result of \exp_args:Nc \cs_to_str:N {}?

csname<escapechar>endcsname: not ideal, but the lesser of two evils

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity.

josephwright commented 1 year ago

Status here? Could we schedule for Fall 2023?

PhelypeOleinik commented 1 year ago

Yes, will be done by then.

josephwright commented 5 months ago

@PhelypeOleinik Are you likely to be able to look at this for the next release?