GNU bug report logs - #39211
[PATCH 1/1] 00-repl-server.test: don't use fixed path for socket

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 39211 in the body.
You can then email your comments to 39211 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Rob Browning <rlb <at> defaultvalue.org>
To: bug-guile <at> gnu.org
Subject: [PATCH 1/1] 00-repl-server.test: don't use fixed path for socket
Date: Mon, 20 Jan 2020 21:22:14 -0600

Even setting aside any security concerns, this caused tests to fail if
you ran them as a given user and then ran them again as another user.

---

 It didn't look like we have anything like mkdtemp or I'd have used it
 instead.  And it looks like this might apply to, and it would be nice
 to have something like it on stable-2.2 as well.

 test-suite/tests/00-repl-server.test | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/test-suite/tests/00-repl-server.test b/test-suite/tests/00-repl-server.test
index 54f518a66..4324f7371 100644
--- a/test-suite/tests/00-repl-server.test
+++ b/test-suite/tests/00-repl-server.test
@@ -24,13 +24,25 @@
   #:use-module (web request)
   #:use-module (test-suite lib))
 
+(define (make-tempdir)
+  (let loop ((try 0)
+             (n (random:uniform)))
+    (let* ((path (string-append "/tmp/repl-server-test-" (number->string n)))
+           (dir (false-if-exception (mkdir path #o700))))
+      (cond
+       (dir path)
+       ((> try 10)
+        (error "Unable to create directory in /tmp for 00-repl-server.test"))
+       (else (loop (1+ try) (random:uniform)))))))
+
 (define (call-with-repl-server proc)
   "Set up a REPL server in a separate process and call PROC with a
 socket connected to that server."
-  (let ((sockaddr      (make-socket-address AF_UNIX "/tmp/repl-server"))
-        (client-socket (socket AF_UNIX SOCK_STREAM 0)))
-    (false-if-exception
-     (delete-file (sockaddr:path sockaddr)))
+  (let* ((tmpdir (make-tempdir))
+         (sockaddr (make-socket-address AF_UNIX (string-append tmpdir "/repl-server")))
+         (client-socket (socket AF_UNIX SOCK_STREAM 0)))
+    (false-if-exception (delete-file (sockaddr:path sockaddr)))
+    (false-if-exception (rmdir tmpdir))
 
     ;; The REPL server requires thread. The test requires fork.
     (unless (and (provided? 'threads) (provided? 'fork))
-- 
2.24.0

Message #8 received at 39211 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Rob Browning <rlb <at> defaultvalue.org>
Cc: 39211 <at> debbugs.gnu.org
Subject: Re: bug#39211: [PATCH 1/1] 00-repl-server.test: don't use fixed path
 for socket
Date: Tue, 21 Jan 2020 17:50:54 +0100

Hi Rob,

Rob Browning <rlb <at> defaultvalue.org> skribis:

> Even setting aside any security concerns, this caused tests to fail if
> you ran them as a given user and then ran them again as another user.

Oops, indeed.

>  It didn't look like we have anything like mkdtemp or I'd have used it
>  instead.  And it looks like this might apply to, and it would be nice
>  to have something like it on stable-2.2 as well.

I thought we had ‘mkdtemp!’ but it’s just in Guix.  :-/

Well we can go that way for now, perhaps with a FIXME comment suggesting
the use of ‘mkdtemp!’ once it’s available.

Also please make sure to include a ChangeLog-style commit log.

Apart from that it LGTM!

I think we should add ‘mkdtemp!’ to libguile in a separate commit; it’s
OK to _add_ procedures during a stable release.

Thanks,
Ludo’.

Message #13 received at 39211-done <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Rob Browning <rlb <at> defaultvalue.org>
Cc: 39211-done <at> debbugs.gnu.org
Subject: Re: bug#39211: [PATCH 1/1] 00-repl-server.test: don't use fixed path
 for socket
Date: Fri, 06 Mar 2020 15:44:20 +0100

Rob Browning <rlb <at> defaultvalue.org> skribis:

> Even setting aside any security concerns, this caused tests to fail if
> you ran them as a given user and then ran them again as another user.
>
> ---
>
>  It didn't look like we have anything like mkdtemp or I'd have used it
>  instead.  And it looks like this might apply to, and it would be nice
>  to have something like it on stable-2.2 as well.
>
>  test-suite/tests/00-repl-server.test | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)

I believe what you pushed as ddcab06f20525d975503d8d9611e02021fb0dff1
fixes this issue, so I’m closing it now.

Thanks,
Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.