[PATCH] Sanitize "Subject:" and "Author:" fields to not contain control characters in notmuch-search

2011-05-09 Thread Florian Friesdorf
On Sun, 8 May 2011 17:40:54 -0400, Austin Clements  wrote:
> Cool.  This seems very reasonable.
> 
> Just some style nits: The three places where you have
> "sanitize_string(", there should be a space between the function name
> and the paren.

fixed

> Relatedly, "for(;*loop;loop++){" should be spaced out
> like "for (; *loop; loop++) {".

fixed

> (..) 
> Also, existing code conventionally uses a variable named "local"
> for function-level talloc contexts such as your ctx_quote.

In notmuch-search.c there is no variable named "local", in the other
functions its also named ctx_quote. Should I rename all ctx_quote to
"local"?

Will send style fixes after we cleared this.

-- 
Florian Friesdorf 
  GPG FPR: 7A13 5EEE 1421 9FC2 108D  BAAF 38F8 99A3 0C45 F083
Jabber/XMPP: flo at chaoflow.net
IRC: chaoflow on freenode,ircnet,blafasel,OFTC
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: 



[PATCH] Sanitize "Subject:" and "Author:" fields to not contain control characters in notmuch-search

2011-05-08 Thread Austin Clements
On Sun, May 8, 2011 at 5:54 PM, Florian Friesdorf  wrote:
> On Sun, 8 May 2011 17:40:54 -0400, Austin Clements  
> wrote:
>> Also, existing code conventionally uses a variable named "local"
>> for function-level talloc contexts such as your ctx_quote.
>
> In notmuch-search.c there is no variable named "local", in the other
> functions its also named ctx_quote. Should I rename all ctx_quote to
> "local"?

Huh, sure enough.  Looks like the library uses "local" consistently,
but the CLI uses ctx_quote multiple times and "local" appears only
once, so, yeah, stick with ctx_quote.


[PATCH] Sanitize "Subject:" and "Author:" fields to not contain control characters in notmuch-search

2011-05-08 Thread Austin Clements
Cool.  This seems very reasonable.

Just some style nits: The three places where you have
"sanitize_string(", there should be a space between the function name
and the paren.  Relatedly, "for(;*loop;loop++){" should be spaced out
like "for (; *loop; loop++) {".  (Curiously, there seems to be
anti-consensus on whether the brace should be on the same line or the
next, but otherwise the notmuch code is quite consistent about
spacing.)  Also, existing code conventionally uses a variable named
"local" for function-level talloc contexts such as your ctx_quote.

On Sun, May 8, 2011 at 5:14 PM, Florian Friesdorf  wrote:
> From: Andreas Amann 
>
> When a Subject field contained encoded CRLF sequences, these sequences
> would appear unfiltered in the output of notmuch search. This confused
> the notmuch emacs interface leading to "Unexpected Output"
> messages. This is now fixed by replacing all characters with ASCII
> code less than 32 with a question mark.
> ---
> ?notmuch-search.c | ? 22 --
> ?1 files changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/notmuch-search.c b/notmuch-search.c
> index 5e39511..e7fc41a 100644
> --- a/notmuch-search.c
> +++ b/notmuch-search.c
> @@ -111,6 +111,20 @@ format_item_id_text (unused (const void *ctx),
> ? ? printf ("%s%s", item_type, item_id);
> ?}
>
> +static char *
> +sanitize_string(const void *ctx, const char *str)
> +{
> + ? ?char *out, *loop;
> +
> + ? ?loop = out = talloc_strdup (ctx, str);
> +
> + ? ?for(;*loop;loop++){
> + ? ? ? if ((unsigned char)(*loop) < 32)
> + ? ? ? ? ? *loop = '?';
> + ? ?}
> + ? ?return out;
> +}
> +
> ?static void
> ?format_thread_text (const void *ctx,
> ? ? ? ? ? ? ? ? ? ?const char *thread_id,
> @@ -120,13 +134,17 @@ format_thread_text (const void *ctx,
> ? ? ? ? ? ? ? ? ? ?const char *authors,
> ? ? ? ? ? ? ? ? ? ?const char *subject)
> ?{
> + ? ?void *ctx_quote = talloc_new (ctx);
> +
> ? ? printf ("thread:%s %12s [%d/%d] %s; %s",
> ? ? ? ? ? ?thread_id,
> ? ? ? ? ? ?notmuch_time_relative_date (ctx, date),
> ? ? ? ? ? ?matched,
> ? ? ? ? ? ?total,
> - ? ? ? ? ? authors,
> - ? ? ? ? ? subject);
> + ? ? ? ? ? sanitize_string(ctx_quote, authors),
> + ? ? ? ? ? sanitize_string(ctx_quote, subject));
> +
> + ? ?talloc_free (ctx_quote);
> ?}
>
> ?static void
> --
> 1.7.5.1
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
>


[PATCH] Sanitize "Subject:" and "Author:" fields to not contain control characters in notmuch-search

2011-05-08 Thread Jameson Graef Rollins
Hey, Florian.  I applied this patch to release-candidate/0.6, manually
adding in the formatting fixes in the same patch.

I also applied the test patch, modified to use the add_message test
suite function, which makes the patch a bit simpler.

Thanks for the fixes.

jamie.
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: 



[PATCH] Sanitize Subject: and Author: fields to not contain control characters in notmuch-search

2011-05-08 Thread Florian Friesdorf
From: Andreas Amann a.am...@ucc.ie

When a Subject field contained encoded CRLF sequences, these sequences
would appear unfiltered in the output of notmuch search. This confused
the notmuch emacs interface leading to Unexpected Output
messages. This is now fixed by replacing all characters with ASCII
code less than 32 with a question mark.
---
 notmuch-search.c |   22 --
 1 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/notmuch-search.c b/notmuch-search.c
index 5e39511..e7fc41a 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -111,6 +111,20 @@ format_item_id_text (unused (const void *ctx),
 printf (%s%s, item_type, item_id);
 }
 
+static char *
+sanitize_string(const void *ctx, const char *str)
+{
+char *out, *loop;
+
+loop = out = talloc_strdup (ctx, str);
+
+for(;*loop;loop++){
+   if ((unsigned char)(*loop)  32)
+   *loop = '?';
+}
+return out;
+}
+
 static void
 format_thread_text (const void *ctx,
const char *thread_id,
@@ -120,13 +134,17 @@ format_thread_text (const void *ctx,
const char *authors,
const char *subject)
 {
+void *ctx_quote = talloc_new (ctx);
+
 printf (thread:%s %12s [%d/%d] %s; %s,
thread_id,
notmuch_time_relative_date (ctx, date),
matched,
total,
-   authors,
-   subject);
+   sanitize_string(ctx_quote, authors),
+   sanitize_string(ctx_quote, subject));
+
+talloc_free (ctx_quote);
 }
 
 static void
-- 
1.7.5.1

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] Sanitize Subject: and Author: fields to not contain control characters in notmuch-search

2011-05-08 Thread Austin Clements
Cool.  This seems very reasonable.

Just some style nits: The three places where you have
sanitize_string(, there should be a space between the function name
and the paren.  Relatedly, for(;*loop;loop++){ should be spaced out
like for (; *loop; loop++) {.  (Curiously, there seems to be
anti-consensus on whether the brace should be on the same line or the
next, but otherwise the notmuch code is quite consistent about
spacing.)  Also, existing code conventionally uses a variable named
local for function-level talloc contexts such as your ctx_quote.

On Sun, May 8, 2011 at 5:14 PM, Florian Friesdorf f...@chaoflow.net wrote:
 From: Andreas Amann a.am...@ucc.ie

 When a Subject field contained encoded CRLF sequences, these sequences
 would appear unfiltered in the output of notmuch search. This confused
 the notmuch emacs interface leading to Unexpected Output
 messages. This is now fixed by replacing all characters with ASCII
 code less than 32 with a question mark.
 ---
  notmuch-search.c |   22 --
  1 files changed, 20 insertions(+), 2 deletions(-)

 diff --git a/notmuch-search.c b/notmuch-search.c
 index 5e39511..e7fc41a 100644
 --- a/notmuch-search.c
 +++ b/notmuch-search.c
 @@ -111,6 +111,20 @@ format_item_id_text (unused (const void *ctx),
     printf (%s%s, item_type, item_id);
  }

 +static char *
 +sanitize_string(const void *ctx, const char *str)
 +{
 +    char *out, *loop;
 +
 +    loop = out = talloc_strdup (ctx, str);
 +
 +    for(;*loop;loop++){
 +       if ((unsigned char)(*loop)  32)
 +           *loop = '?';
 +    }
 +    return out;
 +}
 +
  static void
  format_thread_text (const void *ctx,
                    const char *thread_id,
 @@ -120,13 +134,17 @@ format_thread_text (const void *ctx,
                    const char *authors,
                    const char *subject)
  {
 +    void *ctx_quote = talloc_new (ctx);
 +
     printf (thread:%s %12s [%d/%d] %s; %s,
            thread_id,
            notmuch_time_relative_date (ctx, date),
            matched,
            total,
 -           authors,
 -           subject);
 +           sanitize_string(ctx_quote, authors),
 +           sanitize_string(ctx_quote, subject));
 +
 +    talloc_free (ctx_quote);
  }

  static void
 --
 1.7.5.1

 ___
 notmuch mailing list
 notmuch@notmuchmail.org
 http://notmuchmail.org/mailman/listinfo/notmuch

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] Sanitize Subject: and Author: fields to not contain control characters in notmuch-search

2011-05-08 Thread Florian Friesdorf
On Sun, 8 May 2011 17:40:54 -0400, Austin Clements amdra...@mit.edu wrote:
 Cool.  This seems very reasonable.
 
 Just some style nits: The three places where you have
 sanitize_string(, there should be a space between the function name
 and the paren.

fixed

 Relatedly, for(;*loop;loop++){ should be spaced out
 like for (; *loop; loop++) {.

fixed

 (..) 
 Also, existing code conventionally uses a variable named local
 for function-level talloc contexts such as your ctx_quote.

In notmuch-search.c there is no variable named local, in the other
functions its also named ctx_quote. Should I rename all ctx_quote to
local?

Will send style fixes after we cleared this.

-- 
Florian Friesdorf f...@chaoflow.net
  GPG FPR: 7A13 5EEE 1421 9FC2 108D  BAAF 38F8 99A3 0C45 F083
Jabber/XMPP: f...@chaoflow.net
IRC: chaoflow on freenode,ircnet,blafasel,OFTC


pgpWtUE1HeJND.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] Sanitize Subject: and Author: fields to not contain control characters in notmuch-search

2011-05-08 Thread Jameson Graef Rollins
Hey, Florian.  I applied this patch to release-candidate/0.6, manually
adding in the formatting fixes in the same patch.

I also applied the test patch, modified to use the add_message test
suite function, which makes the patch a bit simpler.

Thanks for the fixes.

jamie.


pgpSo56tz6dHL.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch