Re: Risky local variable mechanism

[Chong Yidong]

Here's another proposed patch for the risky variables mechanism.  This
is the system I propose:

Instead of setting the local variables as soon as they are read in,
`hack-local-variables' and `hack-local-variables-prop-line' constructs
a list of local variables, and decides what to do with it at the very
end of the `hack-local-variables' function.

Each local variable can be safe, risky, or neither.  A variable-value
pair is safe if it is found in the new customizable alist
`safe-local-variables', or the variable's `safe-local-variables'
property is t, or the variable's `safe-local-variables' property is a
function that evaluates to t with that value.

IF a variable is not safe, it is risky IF its `risky-local-variable'
property is set, or its name ends in "-hooks", "-functions", etc.
(One small change: `safe-local-variables' property evaluating to nil
does not automatically mean the variable is risky.)

If the local variables are all safe, they are set automatically.
Otherwise, we raise a prompt asking whether to set them as a unit.
The user can answer y, n, or !, where ! means to also allow these
variable-value pairs for future sessions, via customize-save-variable.
However, if a variable is risky, it won't be saved in this way, though
it can still be set.

(This last part is a little dubious.  If we decide to save all
variables, that removes the justification for "risky" variables, so
the `risky-local-variable' property becomes obsolete.  I don't know if
this will break anything.)


*** emacs/lisp/files.el.~1.804.~        2006-02-09 23:32:56.000000000 -0500
--- emacs/lisp/files.el 2006-02-10 00:05:20.000000000 -0500
***************
*** 2213,2254 ****
         (goto-char beg)
         end))))
  
! (defun hack-local-variables-confirm (string flag-to-check)
!   (or (eq flag-to-check t)
!       (and flag-to-check
!          (save-window-excursion
!            (condition-case nil
!                (switch-to-buffer (current-buffer))
!              (error
!               ;; If we fail to switch in the selected window,
!               ;; it is probably a minibuffer or dedicated window.
!               ;; So try another window.
!               (let ((pop-up-frames nil))
!                 ;; Refrain from popping up frames since it can't
!                 ;; be undone by save-window-excursion.
!                 (pop-to-buffer (current-buffer)))))
!            (save-excursion
!              (beginning-of-line)
!              (set-window-start (selected-window) (point)))
!            (y-or-n-p (format string
!                              (if buffer-file-name
!                                  (file-name-nondirectory buffer-file-name)
!                                (concat "buffer " (buffer-name)))))))))
  
  (defun hack-local-variables-prop-line (&optional mode-only)
!   "Set local variables specified in the -*- line.
  Ignore any specification for `mode:' and `coding:';
  `set-auto-mode' should already have handled `mode:',
  `set-auto-coding' should already have handled `coding:'.
  If MODE-ONLY is non-nil, all we do is check whether the major mode
! is specified, returning t if it is specified."
    (save-excursion
      (goto-char (point-min))
!     (let ((result nil)
!         (end (set-auto-mode-1))
!         mode-specified
!         (enable-local-variables
!          (and local-enable-local-variables enable-local-variables)))
        ;; Parse the -*- line into the RESULT alist.
        ;; Also set MODE-SPECIFIED if we see a spec or `mode'.
        (cond ((not end)
--- 2213,2270 ----
         (goto-char beg)
         end))))
  
! (defun hack-local-variables-confirm (vars maybe-safe)
!   (if noninteractive
!       t
!     (let (char)
!       (save-window-excursion
!       (with-output-to-temp-buffer "*Local Variables*"
!         (princ "Some local variables are specified
! Do you want to set them?\n
! You can type
! y  -- to set these variables.
! n  -- to ignore these variables.
! !  -- to set these variables, and mark these values as safe
!       (in the future, they can be set without asking you.)\n\n")
!         (dolist (elt vars)
!           (princ (car elt))
!           (princ " : ")
!           (princ (cdr elt))
!           (princ "\n")))
!       (message "Please type y, n, or !: ")
!       (let ((inhibit-quit t)
!             (cursor-in-echo-area t))
!         (while (or (not (numberp (setq char (read-event))))
!                    (not (memq (downcase char)
!                               '(?! ?y ?n ?  ?\C-g))))
!           (message "Please type y, n, or !: "))
!         (if (= char ?\C-g)
!             (setq quit-flag nil)))
!       (setq char (downcase char))
!       (when (= char ?!)
!         (dolist (elt maybe-safe)
!           (push elt safe-local-variables))
!         (customize-save-variable
!          'safe-local-variables 
!          safe-local-variables))
!       (or (= char ?!)
!           (= char ? )
!           (= char ?y))))))
  
  (defun hack-local-variables-prop-line (&optional mode-only)
!   "Return local variables specified in the -*- line.
  Ignore any specification for `mode:' and `coding:';
  `set-auto-mode' should already have handled `mode:',
  `set-auto-coding' should already have handled `coding:'.
+ 
  If MODE-ONLY is non-nil, all we do is check whether the major mode
! is specified, returning t if it is specified.  Otherwise, return
! an alist of elements (VAR . VAL), where VAR is the variable and VAL
! is the specified value."
    (save-excursion
      (goto-char (point-min))
!     (let ((end (set-auto-mode-1))
!         result mode-specified)
        ;; Parse the -*- line into the RESULT alist.
        ;; Also set MODE-SPECIFIED if we see a spec or `mode'.
        (cond ((not end)
***************
*** 2283,2317 ****
                     (setq result (cons (cons key val) result)))
                 (if (equal (downcase (symbol-name key)) "mode")
                     (setq mode-specified t))
!                (skip-chars-forward " \t;")))
!            (setq result (nreverse result))))
  
!       (if mode-only mode-specified
!       (if (and result
!                (or mode-only
!                    (hack-local-variables-confirm
!                     "Set local variables as specified in -*- line of %s? "
!                     enable-local-variables)))
!           (let ((enable-local-eval enable-local-eval))
!             (while result
!               (hack-one-local-variable (car (car result)) (cdr (car result)))
!               (setq result (cdr result)))))
!       nil))))
  
  (defvar hack-local-variables-hook nil
    "Normal hook run after processing a file's local variables specs.
  Major modes can use this to examine user-specified local variables
  in order to initialize other data structure based on them.")
  
  (defun hack-local-variables (&optional mode-only)
    "Parse and put into effect this buffer's local variables spec.
  If MODE-ONLY is non-nil, all we do is check whether the major mode
  is specified, returning t if it is specified."
!   (let ((mode-specified
!        ;; If MODE-ONLY is t, we check here for specifying the mode
!        ;; in the -*- line.  If MODE-ONLY is nil, we process
!        ;; the -*- line here.
!        (hack-local-variables-prop-line mode-only))
        (enable-local-variables
         (and local-enable-local-variables enable-local-variables)))
      ;; Look for "Local variables:" line in last page.
--- 2299,2327 ----
                     (setq result (cons (cons key val) result)))
                 (if (equal (downcase (symbol-name key)) "mode")
                     (setq mode-specified t))
!                (skip-chars-forward " \t;")))))
  
!       (if mode-only
!         mode-specified
!       result))))
  
  (defvar hack-local-variables-hook nil
    "Normal hook run after processing a file's local variables specs.
  Major modes can use this to examine user-specified local variables
  in order to initialize other data structure based on them.")
  
+ (defcustom safe-local-variables nil
+   "Alist of safe local variables.
+ Each element is a cons cell (VAR . VAL), where VAR is the
+ variable symbol and VAL is a value considered safe."
+   :group 'find-file
+   :type  'alist)
+ 
  (defun hack-local-variables (&optional mode-only)
    "Parse and put into effect this buffer's local variables spec.
  If MODE-ONLY is non-nil, all we do is check whether the major mode
  is specified, returning t if it is specified."
!   (let ((result (hack-local-variables-prop-line mode-only))
        (enable-local-variables
         (and local-enable-local-variables enable-local-variables)))
      ;; Look for "Local variables:" line in last page.
***************
*** 2321,2329 ****
        (when (let ((case-fold-search t))
              (and (search-forward "Local Variables:" nil t)
                   (or mode-only
!                      (hack-local-variables-confirm
!                       "Set local variables as specified at end of %s? "
!                       enable-local-variables))))
        (skip-chars-forward " \t")
        (let ((enable-local-eval enable-local-eval)
              ;; suffix is what comes after "local variables:" in its line.
--- 2331,2337 ----
        (when (let ((case-fold-search t))
              (and (search-forward "Local Variables:" nil t)
                   (or mode-only
!                      enable-local-variables)))
        (skip-chars-forward " \t")
        (let ((enable-local-eval enable-local-eval)
              ;; suffix is what comes after "local variables:" in its line.
***************
*** 2384,2397 ****
                  (setq val (read (current-buffer)))
                  (if mode-only
                      (if (eq var 'mode)
!                         (setq mode-specified t))
!                   ;; Set the variable.  "Variables" mode and eval are funny.
!                   (with-current-buffer thisbuf
!                     (hack-one-local-variable var val))))
                (forward-line 1)))))))
!     (unless mode-only
!       (run-hooks 'hack-local-variables-hook))
!     mode-specified))
  
  (defvar ignored-local-variables ()
    "Variables to be ignored in a file's local variable spec.")
--- 2392,2425 ----
                  (setq val (read (current-buffer)))
                  (if mode-only
                      (if (eq var 'mode)
!                         (setq result t))
!                   (unless (eq var 'coding)
!                     (push (cons var val) result))))
                (forward-line 1)))))))
!     ;; We've read all the local variables.  Now, return whether the
!     ;; mode is specified (if MODE-ONLY is non-nil), or set the
!     ;; variables (if MODE-ONLY is nil.)
!     (if mode-only
!       result
!       (when enable-local-variables
!       (setq result (nreverse result))
!       (dolist (ignored ignored-local-variables)
!         (setq result (assq-delete-all ignored result)))
!       ;; Find those variables that we may want to save to
!       ;; `safe-local-variables'.
!       (let (maybe-safe risky)
!         (dolist (elt result)
!           (or (safe-local-variable-p (car elt) (cdr elt))
!               (and (risky-local-variable-p (car elt) (cdr elt))
!                    (setq risky t))
!               (push elt maybe-safe)))
!         (if (or (and (eq enable-local-variables t)
!                      (null maybe-safe)
!                      (not risky))
!                 (hack-local-variables-confirm result maybe-safe))
!             (dolist (elt result)
!               (hack-one-local-variable (car elt) (cdr elt)))))
!       (run-hooks 'hack-local-variables-hook)))))
  
  (defvar ignored-local-variables ()
    "Variables to be ignored in a file's local variable spec.")
***************
*** 2451,2458 ****
  (put 'display-time-string 'risky-local-variable t)
  (put 'parse-time-rules 'risky-local-variable t)
  
! ;; This case is safe because the user gets to check it before it is used.
! (put 'compile-command 'safe-local-variable 'stringp)
  
  (defun risky-local-variable-p (sym &optional val)
    "Non-nil if SYM could be dangerous as a file-local variable with value VAL.
--- 2479,2510 ----
  (put 'display-time-string 'risky-local-variable t)
  (put 'parse-time-rules 'risky-local-variable t)
  
! ;; Commonly-encountered local variables that are safe:
! (mapc (lambda (pair)
!       (put (car pair) 'safe-local-variable (cdr pair)))
!       '((compile-command  . stringp)
!       (fill-column      . integerp)
!       (fill-prefix      . t)
!       (indent-tabs-mode . t)
!       (page-delimiter   . t)
!       (paragraph-separate . t)
!       (sentence-end     . t)
!       (sentence-end-double-space . t)
!       (tab-width        . integerp)
!       (version-control  . t)))
! 
! (defun safe-local-variable-p (sym val)
!   "Non-nil if SYM is safe as a file-local variable with value VAL."
!   (or (member (cons sym val) safe-local-variables)
!       (eq sym 'mode)
!       (progn
!       (condition-case nil
!           (setq sym (indirect-variable sym))
!         (error nil))
!       (let ((safep (get sym 'safe-local-variable)))
!         (and safep
!              (or (eq safep t)
!                  (funcall safep val)))))))
  
  (defun risky-local-variable-p (sym &optional val)
    "Non-nil if SYM could be dangerous as a file-local variable with value VAL.
***************
*** 2462,2477 ****
    (condition-case nil
        (setq sym (indirect-variable sym))
      (error nil))
!   (let ((safep (get sym 'safe-local-variable)))
!     (or (get sym 'risky-local-variable)
!       (and (string-match 
"-hooks?$\\|-functions?$\\|-forms?$\\|-program$\\|-commands?$\\|-predicates?$\\|font-lock-keywords$\\|font-lock-keywords-[0-9]+$\\|font-lock-syntactic-keywords$\\|-frame-alist$\\|-mode-alist$\\|-map$\\|-map-alist$"
!                          (symbol-name sym))
!            (not safep))
!       ;; If the safe-local-variable property isn't t or nil,
!       ;; then it must return non-nil on the proposed value to be safe.
!       (and (not (memq safep '(t nil)))
!            (or (null val)
!                (not (funcall safep val)))))))
  
  (defcustom safe-local-eval-forms nil
    "*Expressions that are considered \"safe\" in an `eval:' local variable.
--- 2514,2522 ----
    (condition-case nil
        (setq sym (indirect-variable sym))
      (error nil))
!   (or (get sym 'risky-local-variable)
!       (string-match 
"-hooks?$\\|-functions?$\\|-forms?$\\|-program$\\|-commands?$\\|-predicates?$\\|font-lock-keywords$\\|font-lock-keywords-[0-9]+$\\|font-lock-syntactic-keywords$\\|-frame-alist$\\|-mode-alist$\\|-map$\\|-map-alist$"
!                   (symbol-name sym))))
  
  (defcustom safe-local-eval-forms nil
    "*Expressions that are considered \"safe\" in an `eval:' local variable.
***************
*** 2534,2561 ****
    (cond ((eq var 'mode)
         (funcall (intern (concat (downcase (symbol-name val))
                                  "-mode"))))
!       ((eq var 'coding)
!        ;; We have already handled coding: tag in set-auto-coding.
!        nil)
!       ((memq var ignored-local-variables)
!        nil)
!       ;; "Setting" eval means either eval it or do nothing.
!       ;; Likewise for setting hook variables.
!       ((risky-local-variable-p var val)
!        ;; Permit evalling a put of a harmless property.
!        ;; if the args do nothing tricky.
!        (if (or (and (eq var 'eval)
!                     (hack-one-local-variable-eval-safep val))
!                ;; Permit eval if not root and user says ok.
!                (and (not (zerop (user-uid)))
!                     (hack-local-variables-confirm
!                      "Process `eval' or hook local variables in %s? "
!                      enable-local-eval)))
!            (if (eq var 'eval)
!                (save-excursion (eval val))
!              (make-local-variable var)
!              (set var val))
!          (message "Ignoring risky spec in the local variables list")))
        ;; Ordinary variable, really set it.
        (t (make-local-variable var)
           ;; Make sure the string has no text properties.
--- 2579,2587 ----
    (cond ((eq var 'mode)
         (funcall (intern (concat (downcase (symbol-name val))
                                  "-mode"))))
!       ((eq var 'eval)
!        (if (hack-one-local-variable-eval-safep val)
!            (save-excursion (eval val))))
        ;; Ordinary variable, really set it.
        (t (make-local-variable var)
           ;; Make sure the string has no text properties.