fix!(pgfkeys): more careful treatment of \relax in pgfkeys

There are a lot of instances in pgfkeys where \csname is used, e.g. in
\pgfkeysgetvalueof. This will \let the mentioned csname become \relax so we have
to be prepared for this in across the board in pgfkeys. Indeed, this was the
original behavior before eTeX but it somehow got lost when transitioning to
\ifcsname. The behavior is broken with eTeX since 641a883.

BREAKING CHANGE: \pgfkeysvalueof will expand to an alias for the primitve \relax
in case the key was undefined rather than to a \relaxed csname.

Co-authored-by: Jonathan Spratte <jspratte@yahoo.de>
Co-authored-by: muzimuzhi <muzimuzhi@gmail.com>
Signed-off-by: Henri Menke <henri@henrimenke.de>

doc/generic/pgf/CHANGELOG.md

@@ -16,6 +16,18 @@ lot of contributed changes. Thanks to everyone who volunteered their time!
   `\pgfrevision{,date}` and `\pgfversion{,date}` are synonyms for now, but the
   revision should eventually gain back its original meaning.  However, as of now
   this is not supported by l3build.
+- Many operations in `pgfkeys` used to use `\csname` directly which lets the
+  given csname become `\relax` in case it wasn't defined. This resulted in some
+  leakage of accidentally `\relax`ed keys into the global scope. This has been
+  cleaned up a little. To preserve compatibility macros that used to expand to a
+  `\relax`ed csname now expand to a primitive `\relax`. This affects the
+  user-level commands `\pgfkeysgetvalue` and `\pgfkeysgetvalueof`. For the
+  former the change should not be visible except for the number of expansions
+  required. For `\pgfkeysgetvalueof`, however, the behavior is manifestly
+  different in that it will now expand to an alias for the primitive `\relax` in
+  case the value is undefined instead of a `\relax`ed csname. It has always been
+  semantically wrong to assign to the result of `\pgfkeysgetvalueof`, but now it
+  will have undesired side-effects. Therefore this change is breaking.
 
 ### Added
 
@@ -60,6 +72,7 @@ lot of contributed changes. Thanks to everyone who volunteered their time!
   #1112
 - Form-only patterns have no specified color #1122 
 - Make `graphdrawing` work with `name prefix` and `name suffix` options #1087
+- pgfkeys was a bit too relaxed around `\relax` #1132
 
 ### Changed
 

doc/generic/pgf/pgfmanual-en-pgfkeys.tex

@@ -254,25 +254,24 @@ \subsection{The Key Tree}
 \end{command}
 
 \begin{command}{\pgfkeysvalueof\marg{full key}}
-    Inserts the value stored in \meta{full key} at the current position into
-    the text.
+    Inserts the value stored in \meta{full key} at the current position into the
+    text. It expands to an alias of the primitive |\relax| if the key is undefined.
     %
 \begin{codeexample}[]
 \pgfkeyssetvalue{/my family/my key}{Hello, world!}
 \pgfkeysvalueof{/my family/my key}
 \end{codeexample}
     %
+    \emph{Note:} It is an error to assign to the result of the expansion of
+    |\pgfkeysvalueof|, not only semantically but in recent versions of \pgfname\
+    also logically. To set the value of a key \emph{always} use the appropriate
+    interfaces, e.g.\ |\pgfkeyssetvalue|.
 \end{command}
 
 \begin{command}{\pgfkeysifdefined\marg{full key}\marg{if}\marg{else}}
     Checks whether this key was previously set using either |\pgfkeyssetvalue|
     or |\pgfkeyslet|. If so, the code in \meta{if} is executed, otherwise the
     code in \meta{else}.
-
-    This command will use e\TeX's |\ifcsname| command, if available, for
-    efficiency. This means, however, that it may behave differently for \TeX\
-    and for e\TeX\ when you set keys to |\relax|. For this reason you should
-    not do so.
     %
 \begin{codeexample}[]
 \pgfkeyssetvalue{/my family/my key}{Hello, world!}
@@ -284,7 +283,7 @@ \subsection{The Key Tree}
 
 \subsection{Setting Keys}
 
-Settings keys is done using a powerful command called |\pgfkeys|. This command
+Setting keys is done using a powerful command called |\pgfkeys|. This command
 takes a list of so-called \emph{key--value pairs}. These are pairs of the form
 \meta{key}|=|\meta{value}. The principal idea is the following: For each pair
 in the list, some \emph{action} is taken. This action can be one of the

testfiles/gh-issue-1131.lvt

@@ -0,0 +1,36 @@
+\documentclass{minimal}
+\input{pgf-regression-test}
+
+\RequirePackage{pgfkeys}
+
+\begin{document}
+
+\START
+
+\BEGINTEST{pgfkeys: prevent csname from implicitly defining /foo}
+\pgfkeysgetvalue{/foo}{\mycmd}
+\pgfkeysifdefined{/foo}{\TYPE{true}}{\TYPE{false}}
+\ENDTEST
+
+\BEGINTEST{pgfkeys: keys must be able to hold \relax}
+
+% essentially \let\csname pgfk@/bar\endcsname\relax
+\pgfkeyslet{/bar}{\relax}
+\pgfkeys{/bar}
+\pgfkeysifdefined{/bar}{\TYPE{true}}{\TYPE{false}}
+
+% essentially \def\csname pgfk@/bar\endcsname{\relax}
+\pgfkeys{/bar/.initial=\relax}
+\pgfkeys{/bar}
+\pgfkeysifdefined{/bar}{\TYPE{true}}{\TYPE{false}}
+
+\pgfkeyslet{/bar}{\undefined}
+\pgfkeysifdefined{/bar}{\TYPE{true}}{\TYPE{false}}
+\ENDTEST
+
+\BEGINTEST{pgfkeys: nice error handling for accidental \relax in /.@cmd}
+\csname pgfk@/baz/.@cmd\endcsname
+\pgfkeys{/baz}
+\ENDTEST
+
+\END

testfiles/gh-issue-1131.tlg

@@ -0,0 +1,27 @@
+This is a generated file for the l3build validation system.
+Don't change this file in any respect.
+============================================================
+TEST 1: pgfkeys: prevent csname from implicitly defining /foo
+============================================================
+false
+============================================================
+============================================================
+TEST 2: pgfkeys: keys must be able to hold \relax 
+============================================================
+true
+true
+false
+============================================================
+============================================================
+TEST 3: pgfkeys: nice error handling for accidental \relax in /.@cmd
+============================================================
+! Package pgfkeys Error: I do not know the key '/baz' and I am going to ignore it. Perhaps you misspelled it.
+See the pgfkeys package documentation for explanation.
+Type  H <return>  for immediate help.
+ ...                                              
+l. ...\pgfkeys{/baz}
+This error message was generated by an \errmessage
+command, so I can't give any explicit help.
+Pretend that you're Hercule Poirot: Examine all clues,
+and deduce the truth by order and method.
+============================================================

tex/generic/pgf/utilities/pgfkeys.code.tex

@@ -26,6 +26,13 @@
 \def\pgfkeys@empty{}
 \long\def\pgfkeys@firstoftwo#1#2{#1}
 \long\def\pgfkeys@secondoftwo#1#2{#2}
+\long\def\pgfkeys@ifcsname#1{%
+  \ifcsname#1\endcsname
+    \expandafter\pgfkeys@firstoftwo
+  \else
+    \expandafter\pgfkeys@secondoftwo
+  \fi
+}
 
 % This is useful:
 
@@ -145,7 +152,7 @@
 %
 % \pgfkeyslet{/algo/swap}{\myswap}
 
-\def\pgfkeyslet#1#2{%
+\long\def\pgfkeyslet#1#2{%
   \expandafter\let\csname pgfk@#1\endcsname#2%
 }
 
@@ -163,7 +170,10 @@
 %
 % \pgfkeysgetvalue{/tikz/swap}{\myswap}
 
-\def\pgfkeysgetvalue#1#2{\expandafter\let\expandafter#2\csname pgfk@#1\endcsname}
+\long\def\pgfkeysgetvalue#1#2{%
+  \pgfkeys@ifcsname{pgfk@#1}%
+    {\expandafter\let\expandafter#2\csname pgfk@#1\endcsname}%
+    {\let#2\relax}}
 
 
 
@@ -180,7 +190,8 @@
 %
 % The length is \pgfkeysvalue{/tikz/length}.
 
-\def\pgfkeysvalueof#1{\csname pgfk@#1\endcsname}
+\let\pgfkeys@relax\relax
+\long\def\pgfkeysvalueof#1{\csname\pgfkeys@ifcsname{pgfk@#1}{pgfk@#1}{pgfkeys@relax}\endcsname}
 
 
 
@@ -379,7 +390,9 @@
 \def\pgfkeys@case@one{%
   \pgfkeysifdefined{\pgfkeyscurrentkey/.@cmd}%
   {\pgfkeysgetvalue{\pgfkeyscurrentkey/.@cmd}{\pgfkeys@code}%
-   \expandafter\pgfkeys@code\pgfkeyscurrentvalue\pgfeov}
+    \ifx\pgfkeys@code\relax\expandafter\pgfkeys@firstoftwo\else\expandafter\pgfkeys@secondoftwo\fi
+    {\pgfkeys@unknown}%
+    {\expandafter\pgfkeys@code\pgfkeyscurrentvalue\pgfeov}}
   {\pgfkeys@case@two}%
 }
 
@@ -447,20 +460,20 @@
 \def\pgfkeys@ifexecutehandler#1#2{#1}%
 \let\pgfkeys@ifexecutehandler@handleall=\pgfkeys@ifexecutehandler
 \def\pgfkeys@ifexecutehandler@handleonlyexisting#1#2{%
-  \ifcsname pgfk@excpt@\pgfkeyscurrentname\endcsname%
+  \pgfkeys@ifcsname{pgfk@excpt@\pgfkeyscurrentname}{%
      #1% ok, this particular key handler is known and should be processed in any case (for example .try)
-  \else
+  }{%
      % implement the 'only existing' feature here:
      \pgfkeysifdefined{\pgfkeyscurrentpath}{#1}{%
     \pgfkeysifdefined{\pgfkeyscurrentpath/.@cmd}{#1}{#2}%
      }{}%
-  \fi%
+  }%
 }%
 \def\pgfkeys@ifexecutehandler@handlefullorexisting#1#2{%
   \ifpgfkeysaddeddefaultpath
-    \ifcsname pgfk@excpt@\pgfkeyscurrentname\endcsname%
+    \pgfkeys@ifcsname{pgfk@excpt@\pgfkeyscurrentname}{%
        #1% ok, this particular key handler is known and be processed in any case (for example .try)
-    \else
+    }{%
        % implement the 'only existing' feature here:
        \pgfkeysifdefined{\pgfkeyscurrentpath}{%
          #1%
@@ -471,7 +484,7 @@
            #2%
          }%
        }%
-    \fi%
+    }%
   \else
     #1% ok, always true if the USER explicitly provided the full key path.
   \fi
@@ -859,12 +872,12 @@
   }%
 }
 \def\pgfkeys@handle@boolean#1#2{%
-  \ifcsname#1#2\endcsname%
+  \pgfkeys@ifcsname{#1#2}{%
     \csname#1#2\endcsname%
-  \else%
+  }{%
     \def\pgf@marshal{\pgfkeysvalueof{/errors/boolean expected/.@cmd}}%
     \expandafter\pgf@marshal\expandafter{\pgfkeyscurrentkey}{#2}\pgfeov%
-  \fi
+  }%
 }
 
 \pgfkeys{/handlers/.is choice/.code=%
@@ -996,12 +1009,12 @@
 \long\def\pgfkeys@exp@call#1{\pgfkeysalso{\pgfkeyscurrentpath={#1}}}
 
 \def\pgfkeys@mathparse{%
-  \ifcsname pgfmathparse\endcsname
-    \expandafter\pgfmathparse
-  \else
+  \pgfkeys@ifcsname{pgfmathparse}{%
+    \pgfmathparse
+  }{%
     \pgfkeys@error{You have to load `pgfmath' to use \string\pgfmathparse}%
-    \expandafter\def\expandafter\pgfmathresult
-  \fi
+    \def\pgfmathresult
+  }%
 }
 \pgfkeys{/handlers/.evaluated/.code=\pgfkeys@mathparse{#1}\expandafter\pgfkeys@exp@call\expandafter{\pgfmathresult}}