[PATCH] CLI/restore: handle missing keys and values in config data.

classic Classic list List threaded Threaded
3 messages Options
David Bremner-2 David Bremner-2
Reply | Threaded
Open this post in threaded view
|

[PATCH] CLI/restore: handle missing keys and values in config data.

jrollins discovered that

   % echo '@#foo' | notmuch restore

currently segfaults.

Although such lines can't occur in notmuch dump output, they might be
useful for clearing config, and anyway segfaulting is not the best
error message.
---
 notmuch-restore.c | 8 ++++++++
 1 file changed, 8 insertions(+)

This should probably add a couple of tests. And maybe the commit
message should mention handling null keys.

diff --git a/notmuch-restore.c b/notmuch-restore.c
index dee19c20..3d36fc55 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -36,7 +36,15 @@ process_config_line (notmuch_database_t *notmuch, const char* line)
     void *local = talloc_new(NULL);
 
     key_p = strtok_len_c (line, delim, &key_len);
+    if (!key_p) {
+ fprintf (stderr, "missing config key on line %s\n", line);
+ goto DONE;
+    }
+
     val_p = strtok_len_c (key_p+key_len, delim, &val_len);
+    if (!val_p) {
+ val_p = "";
+    }
 
     key = talloc_strndup (local, key_p, key_len);
     val = talloc_strndup (local, val_p, val_len);
--
2.15.1

_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch
Daniel Kahn Gillmor Daniel Kahn Gillmor
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] CLI/restore: handle missing keys and values in config data.

On Sun 2018-01-07 17:30:25 -0400, David Bremner wrote:
> Although such lines can't occur in notmuch dump output, they might be
> useful for clearing config, and anyway segfaulting is not the best
> error message.

I don't think we want "notmuch restore" to actually clear any
configurations (it's always been additive thus far and changing that
seems dicey to me), but the patch itself here looks good to me.

thanks for fixing the segfault, bremner.

       --dkg

_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch

signature.asc (847 bytes) Download Attachment
David Bremner-2 David Bremner-2
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] CLI/restore: handle missing keys and values in config data.

Daniel Kahn Gillmor <[hidden email]> writes:

> On Sun 2018-01-07 17:30:25 -0400, David Bremner wrote:
>> Although such lines can't occur in notmuch dump output, they might be
>> useful for clearing config, and anyway segfaulting is not the best
>> error message.
>
> I don't think we want "notmuch restore" to actually clear any
> configurations (it's always been additive thus far and changing that
> seems dicey to me), but the patch itself here looks good to me.

I'm not sure how to reconcile those two statements: the patch does clear
configuration if given a key without a value.

_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch