diff mbox series

[bug#55673,v2] cache: Catch valid integer for 'last-expiry-cleanup'.

Message ID 20220527154652.700549-1-zimon.toutoune@gmail.com
State New
Headers
Series [bug#55673,v2] cache: Catch valid integer for 'last-expiry-cleanup'. |

Commit Message

Simon Tournier May 27, 2022, 3:46 p.m. UTC 
Fixes <http://issues.guix.gnu.org/55638>.

* guix/cache.scm (maybe-remove-expired-cache-entries)[last-expiry-date]: Check
if the date is a valid integer.
* tests/cache.scm: Test it.
---
 guix/cache.scm  | 16 ++++++++++++----
 tests/cache.scm | 22 ++++++++++++++++++++++
 2 files changed, 34 insertions(+), 4 deletions(-)


base-commit: 38bf6c7d0cb588e8d4546db7d2e0bae2ec25183d

Comments

Maxime Devos May 27, 2022, 5:29 p.m. UTC | #1 
zimoun schreef op vr 27-05-2022 om 17:46 [+0200]:
> +      (catch #t   ; Handle value out of range (e.g., 1234567890 -> 12E4567890)
> +        (lambda ()
> +          ;; Handle empty or corrupted 'expiry-file' when 'write' below is
> +          ;; interrupted before being complete (e.g., SIGINT with C-c) or when
> +          ;; the filesystem crashes.
> +          (or (string->number value) 0))

Why are 'stack-error' and 'out-of-memory' being caught?
I recommend using one of the regexes from string->generations
in (guix ui) instead to avoid catching too much.

Greetings,
Maxime.
Simon Tournier May 30, 2022, 1:09 p.m. UTC | #2 
On Fri, 27 May 2022 at 19:33, Maxime Devos <maximedevos@telenet.be> wrote:

> Why are 'stack-error' and 'out-of-memory' being caught?
> I recommend using one of the regexes from string->generations
> in (guix ui) instead to avoid catching too much.

Well, I have sent a v3.  Note that that 'string->generations' does not
catch the value out of range we are talking.

--8<---------------cut here---------------start------------->8---
$ guix package --list-generations=12E4567890
Backtrace:
           9 (primitive-load "/home/simon/.config/guix/current/bin/guix")
In guix/ui.scm:
   2230:7  8 (run-guix . _)
  2193:10  7 (run-guix-command _ . _)
In ice-9/boot-9.scm:
  1752:10  6 (with-exception-handler _ _ #:unwind? _ #:unwind-for-type _)
In guix/scripts/package.scm:
    799:7  5 (_)
In ice-9/boot-9.scm:
  1747:15  4 (with-exception-handler #<procedure 7f97f7728150 at
ice-9/boot-9.scm:1831:7 (exn)> _ …)
In guix/scripts/package.scm:
   810:16  3 (_)
In guix/ui.scm:
   1887:9  2 (matching-generations "12E4567890"
"/var/guix/profiles/per-user/simon/guix-profile" …)
  1760:13  1 (string->generations _)
In ice-9/boot-9.scm:
  1685:16  0 (raise-exception _ #:continuable? _)

ice-9/boot-9.scm:1685:16: In procedure raise-exception:
In procedure string->number: Value out of range: 4567890
--8<---------------cut here---------------end--------------->8---


Cheers,
simon
diff mbox series

Patch

diff --git a/guix/cache.scm b/guix/cache.scm
index 51009809bd..13f9e4a439 100644
--- a/guix/cache.scm
+++ b/guix/cache.scm
@@ -20,6 +20,7 @@  (define-module (guix cache)
   #:use-module (srfi srfi-19)
   #:use-module (srfi srfi-26)
   #:use-module (ice-9 match)
+  #:use-module ((ice-9 textual-ports) #:select (get-string-all))
   #:export (obsolete?
             delete-file*
             file-expiration-time
@@ -91,10 +92,17 @@  (define expiry-file
     (string-append cache "/last-expiry-cleanup"))
 
   (define last-expiry-date
-    (catch 'system-error
-      (lambda ()
-        (call-with-input-file expiry-file read))
-      (const 0)))
+    (let ((value (catch 'system-error
+                   (lambda ()
+                     (call-with-input-file expiry-file get-string-all))
+                   (const "0"))))
+      (catch #t   ; Handle value out of range (e.g., 1234567890 -> 12E4567890)
+        (lambda ()
+          ;; Handle empty or corrupted 'expiry-file' when 'write' below is
+          ;; interrupted before being complete (e.g., SIGINT with C-c) or when
+          ;; the filesystem crashes.
+          (or (string->number value) 0))
+        (const 0))))
 
   (when (obsolete? last-expiry-date now cleanup-period)
     (remove-expired-cache-entries (cache-entries cache)
diff --git a/tests/cache.scm b/tests/cache.scm
index 80b44d69aa..bd6fd64a22 100644
--- a/tests/cache.scm
+++ b/tests/cache.scm
@@ -1,5 +1,6 @@ 
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2017, 2020 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2022 Simon Tournier <zimon.toutoune@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -74,6 +75,27 @@  (define-syntax-rule (test-cache-cleanup cache exp ...)
       (lambda (port)
         (display 0 port)))))
 
+(test-equal "maybe-remove-expired-cache-entries, empty cache"
+  '("a" "b" "c")
+  (test-cache-cleanup cache
+    (call-with-output-file (string-append cache "/last-expiry-cleanup")
+      (lambda (port)
+        (display "" port)))))
+
+(test-equal "maybe-remove-expired-cache-entries, corrupted cache"
+  '("a" "b" "c")
+  (test-cache-cleanup cache
+    (call-with-output-file (string-append cache "/last-expiry-cleanup")
+      (lambda (port)
+        (display "1\"34657890" port)))))
+
+(test-equal "maybe-remove-expired-cache-entries, corrupted cache of out range"
+  '("a" "b" "c")
+  (test-cache-cleanup cache
+    (call-with-output-file (string-append cache "/last-expiry-cleanup")
+      (lambda (port)
+        (display "12E4567890" port)))))
+
 (test-end "cache")
 
 ;;; Local Variables: