From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Eric Abrahamsen Newsgroups: gmane.emacs.bugs Subject: bug#28489: Acknowledgement (27.0.50; eieio-persistent slot type validation should be a bit smarter) Date: Sat, 30 Sep 2017 11:05:44 -0700 Message-ID: <87efqodptz.fsf@ericabrahamsen.net> References: <87lglcn8dt.fsf@ericabrahamsen.net> <878th1i50l.fsf@ericabrahamsen.net> <87wp4lf1kq.fsf@users.sourceforge.net> <87ing4cd04.fsf@ericabrahamsen.net> <87h8vnftnx.fsf@users.sourceforge.net> <87zi9fxvnh.fsf@ericabrahamsen.net> <8760c2fike.fsf@users.sourceforge.net> <87vak16ybk.fsf@ericabrahamsen.net> <87r2updmvj.fsf@users.sourceforge.net> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Trace: blaine.gmane.org 1506794832 21221 195.159.176.226 (30 Sep 2017 18:07:12 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Sat, 30 Sep 2017 18:07:12 +0000 (UTC) User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.0.60 (gnu/linux) To: 28489@debbugs.gnu.org Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Sat Sep 30 20:07:08 2017 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dyMAM-0004pZ-Ra for geb-bug-gnu-emacs@m.gmane.org; Sat, 30 Sep 2017 20:07:07 +0200 Original-Received: from localhost ([::1]:39994 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dyMAQ-000651-Tr for geb-bug-gnu-emacs@m.gmane.org; Sat, 30 Sep 2017 14:07:10 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:41694) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dyMAK-00064u-SW for bug-gnu-emacs@gnu.org; Sat, 30 Sep 2017 14:07:06 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dyMAI-0002jK-51 for bug-gnu-emacs@gnu.org; Sat, 30 Sep 2017 14:07:04 -0400 Original-Received: from debbugs.gnu.org ([208.118.235.43]:60958) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1dyMAI-0002j8-23 for bug-gnu-emacs@gnu.org; Sat, 30 Sep 2017 14:07:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1dyMAH-00024N-S8 for bug-gnu-emacs@gnu.org; Sat, 30 Sep 2017 14:07:01 -0400 X-Loop: help-debbugs@gnu.org In-Reply-To: <87lglcn8dt.fsf@ericabrahamsen.net> Resent-From: Eric Abrahamsen Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 30 Sep 2017 18:07:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 28489 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: X-Debbugs-Original-To: bug-gnu-emacs@gnu.org Original-Received: via spool by submit@debbugs.gnu.org id=B.15067947707892 (code B ref -1); Sat, 30 Sep 2017 18:07:01 +0000 Original-Received: (at submit) by debbugs.gnu.org; 30 Sep 2017 18:06:10 +0000 Original-Received: from localhost ([127.0.0.1]:41406 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dyM9R-00023E-JP for submit@debbugs.gnu.org; Sat, 30 Sep 2017 14:06:09 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:49631) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dyM9P-00022x-OO for submit@debbugs.gnu.org; Sat, 30 Sep 2017 14:06:08 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dyM9J-0001BO-FC for submit@debbugs.gnu.org; Sat, 30 Sep 2017 14:06:02 -0400 Original-Received: from lists.gnu.org ([2001:4830:134:3::11]:47915) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dyM9J-0001BB-CH for submit@debbugs.gnu.org; Sat, 30 Sep 2017 14:06:01 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:40535) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dyM9H-00060j-QV for bug-gnu-emacs@gnu.org; Sat, 30 Sep 2017 14:06:01 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dyM9F-000150-1w for bug-gnu-emacs@gnu.org; Sat, 30 Sep 2017 14:05:59 -0400 Original-Received: from [195.159.176.226] (port=49855 helo=blaine.gmane.org) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1dyM9E-0000wB-Oc for bug-gnu-emacs@gnu.org; Sat, 30 Sep 2017 14:05:56 -0400 Original-Received: from list by blaine.gmane.org with local (Exim 4.84_2) (envelope-from ) id 1dyM92-00072g-4a for bug-gnu-emacs@gnu.org; Sat, 30 Sep 2017 20:05:44 +0200 X-Injected-Via-Gmane: http://gmane.org/ Original-Lines: 155 Original-X-Complaints-To: usenet@blaine.gmane.org Cancel-Lock: sha1:l6WVt/tR3WS9yCL44fd58T6bz40= X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6.x X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 208.118.235.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:137699 Archived-At: --=-=-= Content-Type: text/plain Noam Postavsky writes: > Eric Abrahamsen writes: > >> That sounds like the right solution. I've never looked at >> unsafep.el, and don't know exactly how it works, > > Basically, there is a whitelist: symbols which have the property `safe', > are ok, stuff like progn is okay if all the things inside are also > `safe'. So if we can be sure an object constructor does nothing but > create an object then it could be marked safe. > >> 3. Object creation could run malicious code *if* someone had overridden >> `initialize-instance' or `shared-initialize', > > Hmm, it might be a difficult to be confident that calling some generic > function is safe. It would indeed be complicated -- you'd have to locate all currently-defined methods for those two generics, then run through them and make sure everything in them was safe. I suppose it would be possible, but pretty annoying. I still think it would be worth bringing `eieio-skip-typecheck' into play. I'm probably the only person in the world who cares about the performance of eieio-persistent-read, but it wouldn't hurt! >> I might as well write tests that exercise the whole eieio-persistent >> round-trip: create a few test objects, write them to a tmp file, and >> read them back as objects. > > Sounds good. Here's the commit as it stands, seems to work fine. I'll let it mellow for a while, and then commit to... emacs-26? Since it's technically a bug fix? --=-=-= Content-Type: text/x-diff Content-Disposition: attachment; filename=0001-Fix-slot-typecheck-in-eieio-persistent.patch >From 45c2e7ffbbcbe564b816af85274ac84a170fca3e Mon Sep 17 00:00:00 2001 From: Eric Abrahamsen Date: Sat, 30 Sep 2017 10:57:52 -0700 Subject: [PATCH] Fix slot typecheck in eieio-persistent * lisp/emacs-lisp/eieio-base.el (eieio-persistent-slot-type-is-class-p): An `or' form can specify multiple potential classes (or null) as valid types for a slot, but previously only the final element of the `or' was actually checked. Now returns all valid classes in the `or' form. * lisp/emacs-lisp/eieio-base.el (eieio-persistent-validate/fix-slot-value): Check if proposed value matches any of the valid classes. * test/lisp/emacs-lisp/eieio-tests/eieio-test-persist.el (eieio-test-multiple-class-slot): Test this behavior. --- lisp/emacs-lisp/eieio-base.el | 28 ++++++++++------------ .../emacs-lisp/eieio-tests/eieio-test-persist.el | 21 ++++++++++++++++ 2 files changed, 34 insertions(+), 15 deletions(-) diff --git a/lisp/emacs-lisp/eieio-base.el b/lisp/emacs-lisp/eieio-base.el index 6b39b4f262..1ddf9c4460 100644 --- a/lisp/emacs-lisp/eieio-base.el +++ b/lisp/emacs-lisp/eieio-base.el @@ -308,14 +308,6 @@ eieio-persistent-validate/fix-slot-value (= (length proposed-value) 1)) nil) - ;; We have a slot with a single object that can be - ;; saved here. Recurse and evaluate that - ;; sub-object. - ((and classtype (class-p classtype) - (child-of-class-p (car proposed-value) classtype)) - (eieio-persistent-convert-list-to-object - proposed-value)) - ;; List of object constructors. ((and (eq (car proposed-value) 'list) ;; 2nd item is a list. @@ -346,6 +338,16 @@ eieio-persistent-validate/fix-slot-value objlist)) ;; return the list of objects ... reversed. (nreverse objlist))) + ;; We have a slot with a single object that can be + ;; saved here. Recurse and evaluate that + ;; sub-object. + ((and classtype + (seq-some + (lambda (elt) + (child-of-class-p (car proposed-value) elt)) + classtype)) + (eieio-persistent-convert-list-to-object + proposed-value)) (t proposed-value)))) @@ -402,13 +404,9 @@ eieio-persistent-slot-type-is-class-p type)) ((eq (car-safe type) 'or) - ;; If type is a list, and is an or, it is possibly something - ;; like (or null myclass), so check for that. - (let ((ans nil)) - (dolist (subtype (cdr type)) - (setq ans (eieio-persistent-slot-type-is-class-p - subtype))) - ans)) + ;; If type is a list, and is an `or', return all valid class + ;; types within the `or' statement. + (seq-filter #'eieio-persistent-slot-type-is-class-p (cdr type))) (t ;; No match, not a class. diff --git a/test/lisp/emacs-lisp/eieio-tests/eieio-test-persist.el b/test/lisp/emacs-lisp/eieio-tests/eieio-test-persist.el index e2cff3fbca..59eb287bc9 100644 --- a/test/lisp/emacs-lisp/eieio-tests/eieio-test-persist.el +++ b/test/lisp/emacs-lisp/eieio-tests/eieio-test-persist.el @@ -195,6 +195,27 @@ persistent-with-objs-slot-subs (persist-test-save-and-compare persist-woss) (delete-file (oref persist-woss file)))) +;; A slot that can contain one of two different classes, to exercise +;; the `or' slot type. + +(defclass persistent-random-class () + ()) + +(defclass persistent-multiclass-slot (eieio-persistent) + ((slot1 :initarg :slot1 + :type (or persistent-random-class null persist-not-persistent)) + (slot2 :initarg :slot2 + :type (or persist-not-persistent persist-random-class null)))) + +(ert-deftest eieio-test-multiple-class-slot () + (let ((persist + (persistent-multiclass-slot "random string" + :slot1 (persistent-random-class) + :slot2 (persist-not-persistent) + :file (concat default-directory "test-ps5.pt")))) + (persist-test-save-and-compare persist) + (delete-file (oref persist file)))) + ;;; Slot with a list of Objects ;; ;; A slot that contains another object that isn't persistent -- 2.14.2 --=-=-=--