* [PATCH 1/3] cli: export function for illegal tag checking
2014-02-23 16:55 ` [PATCH 0/3] check new.tags for invalid tags Jani Nikula
@ 2014-02-23 16:55 ` Jani Nikula
2014-02-23 16:55 ` [PATCH 2/3] cli: make sure notmuch new and insert don't add invalid tags Jani Nikula
` (3 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Jani Nikula @ 2014-02-23 16:55 UTC (permalink / raw)
To: notmuch, Rob Browning
This lets us check for forbidden tags consistently across the cli. No
functional changes.
---
tag-util.c | 9 +--------
tag-util.h | 12 ++++++++++++
2 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/tag-util.c b/tag-util.c
index 3bde4097372a..e2d5b795acc3 100644
--- a/tag-util.c
+++ b/tag-util.c
@@ -31,14 +31,7 @@ line_error (tag_parse_status_t status,
return status;
}
-/*
- * Test tags for some forbidden cases.
- *
- * return: NULL if OK,
- * explanatory message otherwise.
- */
-
-static const char *
+const char *
illegal_tag (const char *tag, notmuch_bool_t remove)
{
diff --git a/tag-util.h b/tag-util.h
index 4628f1630ad6..8a4074ce168f 100644
--- a/tag-util.h
+++ b/tag-util.h
@@ -90,6 +90,18 @@ parse_tag_command_line (void *ctx, int argc, char **argv,
char **query_str, tag_op_list_t *ops);
/*
+ * Test tags for some forbidden cases.
+ *
+ * Relax the checks if 'remove' is true to allow removal of previously
+ * added forbidden tags.
+ *
+ * return: NULL if OK,
+ * explanatory message otherwise.
+ */
+const char *
+illegal_tag (const char *tag, notmuch_bool_t remove);
+
+/*
* Create an empty list of tag operations
*
* ctx is passed to talloc
--
1.8.5.3
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 2/3] cli: make sure notmuch new and insert don't add invalid tags
2014-02-23 16:55 ` [PATCH 0/3] check new.tags for invalid tags Jani Nikula
2014-02-23 16:55 ` [PATCH 1/3] cli: export function for illegal tag checking Jani Nikula
@ 2014-02-23 16:55 ` Jani Nikula
2014-02-23 16:55 ` [PATCH 3/3] test: add tests for invalid new.tags Jani Nikula
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Jani Nikula @ 2014-02-23 16:55 UTC (permalink / raw)
To: notmuch, Rob Browning
Check new.tags configuration values before doing anything, and bail
out on invalid values.
---
notmuch-insert.c | 9 +++++++++
notmuch-new.c | 14 +++++++++++++-
2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/notmuch-insert.c b/notmuch-insert.c
index cd6de88f6891..6752fc8de255 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -431,6 +431,15 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
return EXIT_FAILURE;
}
for (i = 0; i < new_tags_length; i++) {
+ const char *error_msg;
+
+ error_msg = illegal_tag (new_tags[i], FALSE);
+ if (error_msg) {
+ fprintf (stderr, "Error: tag '%s' in new.tags: %s\n",
+ new_tags[i], error_msg);
+ return EXIT_FAILURE;
+ }
+
if (tag_op_list_append (tag_ops, new_tags[i], FALSE))
return EXIT_FAILURE;
}
diff --git a/notmuch-new.c b/notmuch-new.c
index 8529fdd3eac7..82acf695353e 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -19,6 +19,7 @@
*/
#include "notmuch-client.h"
+#include "tag-util.h"
#include <unistd.h>
@@ -918,7 +919,7 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])
struct sigaction action;
_filename_node_t *f;
int opt_index;
- int i;
+ unsigned int i;
notmuch_bool_t timer_is_active = FALSE;
notmuch_bool_t no_hooks = FALSE;
notmuch_bool_t quiet = FALSE, verbose = FALSE;
@@ -950,6 +951,17 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])
add_files_state.synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config);
db_path = notmuch_config_get_database_path (config);
+ for (i = 0; i < add_files_state.new_tags_length; i++) {
+ const char *error_msg;
+
+ error_msg = illegal_tag (add_files_state.new_tags[i], FALSE);
+ if (error_msg) {
+ fprintf (stderr, "Error: tag '%s' in new.tags: %s\n",
+ add_files_state.new_tags[i], error_msg);
+ return EXIT_FAILURE;
+ }
+ }
+
if (!no_hooks) {
ret = notmuch_run_hook (db_path, "pre-new");
if (ret)
--
1.8.5.3
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 3/3] test: add tests for invalid new.tags
2014-02-23 16:55 ` [PATCH 0/3] check new.tags for invalid tags Jani Nikula
2014-02-23 16:55 ` [PATCH 1/3] cli: export function for illegal tag checking Jani Nikula
2014-02-23 16:55 ` [PATCH 2/3] cli: make sure notmuch new and insert don't add invalid tags Jani Nikula
@ 2014-02-23 16:55 ` Jani Nikula
2014-02-23 18:21 ` [PATCH 0/3] check new.tags for invalid tags Tomi Ollila
2014-03-06 11:51 ` [PATCH 0/3] check new.tags for invalid tags David Bremner
4 siblings, 0 replies; 8+ messages in thread
From: Jani Nikula @ 2014-02-23 16:55 UTC (permalink / raw)
To: notmuch, Rob Browning
Similar tests for both notmuch new and insert.
---
test/T050-new.sh | 17 +++++++++++++++++
test/T070-insert.sh | 19 +++++++++++++++++++
2 files changed, 36 insertions(+)
diff --git a/test/T050-new.sh b/test/T050-new.sh
index b7668ff0c4bc..ad46ee6d51b6 100755
--- a/test/T050-new.sh
+++ b/test/T050-new.sh
@@ -263,4 +263,21 @@ notmuch search --format=text0 --output=files --offset=1 --limit=1 '*' | xargs -0
output=$(NOTMUCH_NEW --quiet)
test_expect_equal "$output" ""
+OLDCONFIG=$(notmuch config get new.tags)
+
+test_begin_subtest "Empty tags in new.tags are forbidden"
+notmuch config set new.tags "foo;;bar"
+output=$(NOTMUCH_NEW 2>&1)
+test_expect_equal "$output" "Error: tag '' in new.tags: empty tag forbidden"
+
+test_begin_subtest "Tags starting with '-' in new.tags are forbidden"
+notmuch config set new.tags "-foo;bar"
+output=$(NOTMUCH_NEW 2>&1)
+test_expect_equal "$output" "Error: tag '-foo' in new.tags: tag starting with '-' forbidden"
+
+test_expect_code 1 "Invalid tags set exit code" \
+ "NOTMUCH_NEW 2>&1"
+
+notmuch config set new.tags $OLDCONFIG
+
test_done
diff --git a/test/T070-insert.sh b/test/T070-insert.sh
index e8dc4c099ed1..b77c5e13c87f 100755
--- a/test/T070-insert.sh
+++ b/test/T070-insert.sh
@@ -164,4 +164,23 @@ gen_insert_msg
test_expect_code 1 "Insert message, create invalid subfolder" \
"notmuch insert --folder=../G --create-folder $gen_msg_filename"
+OLDCONFIG=$(notmuch config get new.tags)
+
+test_begin_subtest "Empty tags in new.tags are forbidden"
+notmuch config set new.tags "foo;;bar"
+gen_insert_msg
+output=$(notmuch insert $gen_msg_filename 2>&1)
+test_expect_equal "$output" "Error: tag '' in new.tags: empty tag forbidden"
+
+test_begin_subtest "Tags starting with '-' in new.tags are forbidden"
+notmuch config set new.tags "-foo;bar"
+gen_insert_msg
+output=$(notmuch insert $gen_msg_filename 2>&1)
+test_expect_equal "$output" "Error: tag '-foo' in new.tags: tag starting with '-' forbidden"
+
+test_expect_code 1 "Invalid tags set exit code" \
+ "notmuch insert $gen_msg_filename 2>&1"
+
+notmuch config set new.tags $OLDCONFIG
+
test_done
--
1.8.5.3
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 0/3] check new.tags for invalid tags
2014-02-23 16:55 ` [PATCH 0/3] check new.tags for invalid tags Jani Nikula
` (2 preceding siblings ...)
2014-02-23 16:55 ` [PATCH 3/3] test: add tests for invalid new.tags Jani Nikula
@ 2014-02-23 18:21 ` Tomi Ollila
2014-03-04 16:51 ` [PATCH] cli: add missing \n in error message Jani Nikula
2014-03-06 11:51 ` [PATCH 0/3] check new.tags for invalid tags David Bremner
4 siblings, 1 reply; 8+ messages in thread
From: Tomi Ollila @ 2014-02-23 18:21 UTC (permalink / raw)
To: Jani Nikula, notmuch, Rob Browning
On Sun, Feb 23 2014, Jani Nikula <jani@nikula.org> wrote:
> On Sun, 23 Feb 2014, Rob Browning <rlb@defaultvalue.org> wrote:
>> In the [new] section, "tags=;" will cause notmuch to create empty tags
>> that are fairly hard to remove from the command line.
>
> Clearly broken. This series fixes the issue at the cli
> level. (Forbidding empty tags at the lib level is slightly more
> complicated, as we would still have to ensure old dump files can be
> restored.)
>
>> After some help on #bup, here's what I came up with to remove them,
>> though it assumes that the empty tag "+ " will always be first in dump's
>> output:
>>
>> notmuch dump --format=batch-tag 'tag:""' | perl -pe 's/^\+ //' \
>> | notmuch restore --format=batch-tag
>>
>> And note that you have to use restore, "notmuch tag --batch" doesn't
>> appear to accept "- " as a tag, even though dump will produce "+ ".
>
> I didn't check this further, but the regular, non-batch notmuch tag
> should still work for removal of empty tags.
LGTM.
$ notmuch tag + -- id:edc2bc900f75bb2e72be2037e2df9105be7f0273.1393174108.git.jani@nikula.org
Error: empty tag forbiddenzsh: exit 1 notmuch tag + --
$ notmuch restore --accumulate
+ id:edc2bc900f75bb2e72be2037e2df9105be7f0273.1393174108.git.jani@nikula.org
<ctrl-d -- exit value 0> -- notmuch search shows space after ( to inform
there is empty tag.
$ notmuch tag - -- id:edc2bc900f75bb2e72be2037e2df9105be7f0273.1393174108.git.jani@nikula.org
<no output -- exit value 0>
notmuch search no longer shows space after (
Also, SomeBody(tm) should add '\n' to the fprintf() in tag-util.c:175 (line
number after applying these patches).
Tomi
>
> BR,
> Jani.
>
> Jani Nikula (3):
> cli: export function for illegal tag checking
> cli: make sure notmuch new and insert don't add invalid tags
> test: add tests for invalid new.tags
>
> notmuch-insert.c | 9 +++++++++
> notmuch-new.c | 14 +++++++++++++-
> tag-util.c | 9 +--------
> tag-util.h | 12 ++++++++++++
> test/T050-new.sh | 17 +++++++++++++++++
> test/T070-insert.sh | 19 +++++++++++++++++++
> 6 files changed, 71 insertions(+), 9 deletions(-)
>
> --
> 1.8.5.3
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH] cli: add missing \n in error message
2014-02-23 18:21 ` [PATCH 0/3] check new.tags for invalid tags Tomi Ollila
@ 2014-03-04 16:51 ` Jani Nikula
0 siblings, 0 replies; 8+ messages in thread
From: Jani Nikula @ 2014-03-04 16:51 UTC (permalink / raw)
To: notmuch
The error messages returned by illegal_tag() don't contain newlines.
---
tag-util.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tag-util.c b/tag-util.c
index e2d5b795acc3..343c161f471a 100644
--- a/tag-util.c
+++ b/tag-util.c
@@ -172,7 +172,7 @@ parse_tag_command_line (void *ctx, int argc, char **argv,
msg = illegal_tag (argv[i] + 1, is_remove);
if (msg) {
- fprintf (stderr, "Error: %s", msg);
+ fprintf (stderr, "Error: %s\n", msg);
return TAG_PARSE_INVALID;
}
--
1.8.5.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 0/3] check new.tags for invalid tags
2014-02-23 16:55 ` [PATCH 0/3] check new.tags for invalid tags Jani Nikula
` (3 preceding siblings ...)
2014-02-23 18:21 ` [PATCH 0/3] check new.tags for invalid tags Tomi Ollila
@ 2014-03-06 11:51 ` David Bremner
4 siblings, 0 replies; 8+ messages in thread
From: David Bremner @ 2014-03-06 11:51 UTC (permalink / raw)
To: Jani Nikula, notmuch, Rob Browning
Jani Nikula <jani@nikula.org> writes:
> Clearly broken. This series fixes the issue at the cli
> level. (Forbidding empty tags at the lib level is slightly more
> complicated, as we would still have to ensure old dump files can be
> restored.)
pushed, with the 4th fixup patch.
d
^ permalink raw reply [flat|nested] 8+ messages in thread