[PATCH] notmuch-new.c infinite recursion symlink bug

2011-06-21 Thread Carl Worth 
On Tue, 21 Jun 2011 12:01:17 -0400, Austin Clements  wrote:
> Alternatively, because of folder search, it might be better to keep a
> stack of inode numbers to eliminate loops while retaining notmuch's
> current approach of repeatedly indexing mail that's symlinked in
> multiple folders.

Yes, that sounds like the right approach.

-Carl

-- 
carl.d.worth at intel.com
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL:

[PATCH] notmuch-new.c infinite recursion symlink bug

2011-06-21 Thread Austin Clements 
On Tue, Jun 21, 2011 at 8:42 AM, Daniel Kahn Gillmor
 wrote:
> My point is: there are lots of ways to get infinite recursions via
> symlinks; ?hard-coding a check for one specific way seems like a
> sub-optimal approach, because it leaves the other paths still present,
> and introduces an unexpected/surprising asymmetry.
>
> I'm not sure what the specific right way is to solve the problem you
> identified, though.

A simple solution to this problem would be to record the inode numbers
of each visited directory, probably in a hash table in
add_files_state_t.  At the beginning of add_files_recursive, right
after it stat's the directory, check if st.st_ino is in this hash
table; if it is, return immediately, otherwise add it to the hash
table and proceed as usual.

Alternatively, because of folder search, it might be better to keep a
stack of inode numbers to eliminate loops while retaining notmuch's
current approach of repeatedly indexing mail that's symlinked in
multiple folders.

[PATCH] notmuch-new.c infinite recursion symlink bug

2011-06-21 Thread Daniel Kahn Gillmor 
On 06/10/2011 03:32 AM, Taylor Carpenter wrote:
> If a symlink points to . then there will be an infinite recursion.  The 
> included patch fixes that.

what about a sub-directory that contains a symlink to ".." ?

or a directory that contains both:

 ./a/foo ? ../b
 ./b/foo ? ../a

or ...

My point is: there are lots of ways to get infinite recursions via
symlinks;  hard-coding a check for one specific way seems like a
sub-optimal approach, because it leaves the other paths still present,
and introduces an unexpected/surprising asymmetry.

I'm not sure what the specific right way is to solve the problem you
identified, though.

--dkg

-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 1030 bytes
Desc: OpenPGP digital signature
URL:

Re: [PATCH] notmuch-new.c infinite recursion symlink bug

2011-06-21 Thread Austin Clements 
On Tue, Jun 21, 2011 at 8:42 AM, Daniel Kahn Gillmor
d...@fifthhorseman.net wrote:
 My point is: there are lots of ways to get infinite recursions via
 symlinks;  hard-coding a check for one specific way seems like a
 sub-optimal approach, because it leaves the other paths still present,
 and introduces an unexpected/surprising asymmetry.

 I'm not sure what the specific right way is to solve the problem you
 identified, though.

A simple solution to this problem would be to record the inode numbers
of each visited directory, probably in a hash table in
add_files_state_t.  At the beginning of add_files_recursive, right
after it stat's the directory, check if st.st_ino is in this hash
table; if it is, return immediately, otherwise add it to the hash
table and proceed as usual.

Alternatively, because of folder search, it might be better to keep a
stack of inode numbers to eliminate loops while retaining notmuch's
current approach of repeatedly indexing mail that's symlinked in
multiple folders.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] notmuch-new.c infinite recursion symlink bug

2011-06-21 Thread Carl Worth 
On Tue, 21 Jun 2011 12:01:17 -0400, Austin Clements amdra...@mit.edu wrote:
 Alternatively, because of folder search, it might be better to keep a
 stack of inode numbers to eliminate loops while retaining notmuch's
 current approach of repeatedly indexing mail that's symlinked in
 multiple folders.

Yes, that sounds like the right approach.

-Carl

-- 
carl.d.wo...@intel.com


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

[PATCH] notmuch-new.c infinite recursion symlink bug

2011-06-20 Thread Taylor Carpenter 
If a symlink points to . then there will be an infinite recursion.  The 
included patch fixes that.


--- notmuch-new.c.orig  2011-06-10 00:03:09.0 -0500
+++ notmuch-new.c 2011-06-10 02:10:37.0 -0500
@@ -233,6 +233,8 @@
 struct stat st;
 notmuch_bool_t is_maildir, new_directory;
 const char **tag;
+char lpath[PATH_MAX], filepath[PATH_MAX];
+size_t len;
 
 if (stat (path, st)) {
  fprintf (stderr, Error reading directory %s: %s\n,
@@ -296,6 +298,14 @@
   */
  /* XXX: Eventually we'll want more sophistication to let the
   * user specify files to be ignored. */
+
+ if (entry-d_type == DT_LNK) {
+ snprintf(filepath, sizeof(filepath), %s/%s, path, entry-d_name);
+ if ((len = readlink(filepath, lpath, sizeof(lpath)))  0)
+ if (strncmp(lpath, ., len-1) == 0)
+ continue;
+ }
+
  if (strcmp (entry-d_name, .) == 0 ||
  strcmp (entry-d_name, ..) == 0 ||
  (is_maildir  strcmp (entry-d_name, tmp) == 0) ||

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

Re: [PATCH] notmuch-new.c infinite recursion symlink bug

2011-06-20 Thread Taylor Carpenter 
On 06/10/11 at 02:32P, Taylor Carpenter wrote:
 If a symlink points to . then there will be an infinite recursion.  The 
 included patch fixes that.


I did not realize this was needed in the count function as well.  New
patch included that does both.

--- notmuch-new.c.orig  2011-06-10 00:03:09.0 -0500
+++ notmuch-new.c 2011-06-10 02:46:18.0 -0500
@@ -233,6 +233,8 @@
 struct stat st;
 notmuch_bool_t is_maildir, new_directory;
 const char **tag;
+char lpath[PATH_MAX], filepath[PATH_MAX];
+size_t len;
 
 if (stat (path, st)) {
  fprintf (stderr, Error reading directory %s: %s\n,
@@ -296,6 +298,14 @@
   */
  /* XXX: Eventually we'll want more sophistication to let the
   * user specify files to be ignored. */
+
+ if (entry-d_type == DT_LNK) {
+ snprintf(filepath, sizeof(filepath), %s/%s, path, entry-d_name);
+ if ((len = readlink(filepath, lpath, sizeof(lpath)))  0)
+ if (strncmp(lpath, ., len-1) == 0)
+ continue;
+ }
+
  if (strcmp (entry-d_name, .) == 0 ||
  strcmp (entry-d_name, ..) == 0 ||
  (is_maildir  strcmp (entry-d_name, tmp) == 0) ||
@@ -615,6 +625,8 @@
 struct dirent **fs_entries = NULL;
 int num_fs_entries = scandir (path, fs_entries, 0, dirent_sort_inode);
 int i = 0;
+char lpath[PATH_MAX], filepath[PATH_MAX];
+size_t len;
 
 if (num_fs_entries == -1) {
  fprintf (stderr, Warning: failed to open directory %s: %s\n,
@@ -633,6 +645,13 @@
   */
  /* XXX: Eventually we'll want more sophistication to let the
   * user specify files to be ignored. */
+ if (entry-d_type == DT_LNK) {
+ snprintf(filepath, sizeof(filepath), %s/%s, path, entry-d_name);
+ if ((len = readlink(filepath, lpath, sizeof(lpath)))  0)
+ if (strncmp(lpath, ., len-1) == 0)
+ continue;
+ }
+
  if (strcmp (entry-d_name, .) == 0 ||
  strcmp (entry-d_name, ..) == 0 ||
  strcmp (entry-d_name, .notmuch) == 0)




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

[PATCH] notmuch-new.c infinite recursion symlink bug

2011-06-10 Thread Taylor Carpenter 
On 06/10/11 at 02:32P, Taylor Carpenter wrote:
> If a symlink points to . then there will be an infinite recursion.  The 
> included patch fixes that.


I did not realize this was needed in the count function as well.  New
patch included that does both.

--- notmuch-new.c.orig  2011-06-10 00:03:09.0 -0500
+++ notmuch-new.c 2011-06-10 02:46:18.0 -0500
@@ -233,6 +233,8 @@
 struct stat st;
 notmuch_bool_t is_maildir, new_directory;
 const char **tag;
+char lpath[PATH_MAX], filepath[PATH_MAX];
+size_t len;

 if (stat (path, )) {
  fprintf (stderr, "Error reading directory %s: %s\n",
@@ -296,6 +298,14 @@
   */
  /* XXX: Eventually we'll want more sophistication to let the
   * user specify files to be ignored. */
+
+ if (entry->d_type == DT_LNK) {
+ snprintf(filepath, sizeof(filepath), "%s/%s", path, entry->d_name);
+ if ((len = readlink(filepath, lpath, sizeof(lpath))) > 0)
+ if (strncmp(lpath, ".", len-1) == 0)
+ continue;
+ }
+
  if (strcmp (entry->d_name, ".") == 0 ||
  strcmp (entry->d_name, "..") == 0 ||
  (is_maildir && strcmp (entry->d_name, "tmp") == 0) ||
@@ -615,6 +625,8 @@
 struct dirent **fs_entries = NULL;
 int num_fs_entries = scandir (path, _entries, 0, dirent_sort_inode);
 int i = 0;
+char lpath[PATH_MAX], filepath[PATH_MAX];
+size_t len;

 if (num_fs_entries == -1) {
  fprintf (stderr, "Warning: failed to open directory %s: %s\n",
@@ -633,6 +645,13 @@
   */
  /* XXX: Eventually we'll want more sophistication to let the
   * user specify files to be ignored. */
+ if (entry->d_type == DT_LNK) {
+ snprintf(filepath, sizeof(filepath), "%s/%s", path, entry->d_name);
+ if ((len = readlink(filepath, lpath, sizeof(lpath))) > 0)
+ if (strncmp(lpath, ".", len-1) == 0)
+ continue;
+ }
+
  if (strcmp (entry->d_name, ".") == 0 ||
  strcmp (entry->d_name, "..") == 0 ||
  strcmp (entry->d_name, ".notmuch") == 0)

[PATCH] notmuch-new.c infinite recursion symlink bug

2011-06-10 Thread Taylor Carpenter 
If a symlink points to . then there will be an infinite recursion.  The 
included patch fixes that.


--- notmuch-new.c.orig  2011-06-10 00:03:09.0 -0500
+++ notmuch-new.c 2011-06-10 02:10:37.0 -0500
@@ -233,6 +233,8 @@
 struct stat st;
 notmuch_bool_t is_maildir, new_directory;
 const char **tag;
+char lpath[PATH_MAX], filepath[PATH_MAX];
+size_t len;

 if (stat (path, )) {
  fprintf (stderr, "Error reading directory %s: %s\n",
@@ -296,6 +298,14 @@
   */
  /* XXX: Eventually we'll want more sophistication to let the
   * user specify files to be ignored. */
+
+ if (entry->d_type == DT_LNK) {
+ snprintf(filepath, sizeof(filepath), "%s/%s", path, entry->d_name);
+ if ((len = readlink(filepath, lpath, sizeof(lpath))) > 0)
+ if (strncmp(lpath, ".", len-1) == 0)
+ continue;
+ }
+
  if (strcmp (entry->d_name, ".") == 0 ||
  strcmp (entry->d_name, "..") == 0 ||
  (is_maildir && strcmp (entry->d_name, "tmp") == 0) ||