Nikita Karetnikov <nik...@karetnikov.org> skribis:
> I think the current docstring of ‘assert-valid-signature’ is not correct
> since ‘signature’ must be a string (as produced by
> ‘canonical-sexp->string’), not an sexp.
In guix/nar.scm, the comment is:
(define (assert-valid-signature signature hash file)
;; Bail out if SIGNATURE, an sexp, doesn't match HASH, a bytevector
;; containing the expected hash for FILE.
and indeed, SIGNATURE must be a string here.
> Similarly, the “signature is not a valid s-expression” and “corrupt
> signature data” messages are a bit confusing due to the way
> ‘string->canonical-sexp’ works (try ‘string->canonical-sexp "foo"’).
> But I may be wrong about the latter.
Ah right, you could get “corrupt signature data” when
(string->canonical-sexp signature) returns the null canonical sexp,
whereas you’d want “not a valid s-expression”.
Well, we can fix that in a separate patch if you want.
> +(define* (assert-valid-signature signature hash port
> + #:optional (acl (current-acl)))
> + ;; Bail out if SIGNATURE, a string, doesn't match HASH, a bytevector
> + ;; containing the expected hash for PORT.
Make it a docstring.
Also, please make this change a separate patch.
> + (let* ((file (port-filename port))
I don’t think this will work, because most of the time PORT is a pipe
(an input port), whereas FILE is supposed to be the name of the file
being restored.
> + (raise (condition (&message (message "invalid hash"))
> + (&nar-invalid-hash-error
> + (port port) (file file)
> + (signature signature)
> + (expected (hash-data->bytevector data))
> + (actual hash)))))
> + (raise (condition (&message (message "unauthorized public key"))
> + (&nar-signature-error
> + (signature signature) (file file) (port
> port)))))
> + (raise (condition
> + (&message (message "corrupt signature data"))
> + (&nar-signature-error
> + (signature signature) (file file) (port port)))))))
Actually, the problem with making ‘assert-valid-signature’ public is
that it raises &nar error conditions.
It could be changed to raise a more generic &signature-error, but then
‘restore-file-set’ would have to guard against it to re-throw it along
with a &nar-error (making a compound condition.) And then ui.scm would
figure it out. Blech.
It’s worth factorizing, but I don’t see how to do it nicely. Thoughts?
> +(define (parse-signature str)
> + "Return the value of a narinfo's 'Signature' field as a canonical sexp."
I don’t remember if I said it before, but I’d prefer a name like
‘narinfo-signature->canonical-sexp’.
> +(define* (read-narinfo port #:optional url (acl (current-acl)))
> + "Read a narinfo from PORT. If URL is true, it must be a string used to
> +build full URIs from relative URIs found while reading PORT."
> + (let* ((str (begin (set-port-encoding! port "UTF-8")
> + (get-string-all port)))
Rather set the encoding when PORT is created, or use
(utf8->string (get-bytevector-all port))
That’s it.
Did I miss something?
Thanks,
Ludo’.
