bug#18198: Gnus fails to read ~/.newsrc with group names which are not symbols

bug#18198: Gnus fails to read ~/.newsrc with group names which are not symbols

[Ivan Shmakov]

Package:  emacs, gnus
Severity: minor
Tags:     patch

	As it seems, gnus-newsrc-to-gnus-format fails to correctly
	process ~/.newsrc entries with group names not being /complete/
	Emacs Lisp expressions (integers, symbols, etc.), as in:

[Hello]/World: 42
[[[This-fails: 1

	I believe that the problem is at the following point.

  2556	      (narrow-to-region
  2557	       (point)
  2558	       (progn (skip-chars-forward "^ \t!:\n") (point)))
  2559	      (goto-char (point-min))
  2560	      (setq symbol
  2561		    (and (/= (point-min) (point-max))
  2562			 (let ((obarray gnus-active-hashtb)) (read buf))))
  2563	      (widen)

	Here, the “group” field of .newsrc is read as an Emacs Lisp
	expression.  Which may result either in an error, or, in the
	case of [Hello]/World – and, similarly, (Hello)/World – in only
	the leading [Hello] or (Hello) being actually read.

	Instead, I believe that the entire string is to be read and
	interned, as in (untested):

   (setq symbol
         (let ((save (point)))
           (skip-chars-forward "^ \t!:\n")
           (and (> (point) save)
                (intern (buffer-substring-no-properties save (point))
                        gnus-active-hashtb))))

  2582	       (symbol
  2583		;; Group names can be just numbers.
  2584		(when (numberp symbol)
  2585		  (setq symbol (intern (int-to-string symbol) gnus-active-hashtb)))
  2586		(unless (boundp symbol)
  2587		  (set symbol nil))

	Naturally, the numberp check above will no longer be necessary
	after the change suggested.

	Please note that this change also prevents the boundp check here
	from raising an error should the group name be a valid Emacs
	Lisp non-symbol form (say, a vector or a list.)

-- 
FSF associate member #7257  http://boycottsystemd.org/  … 3013 B6A0 230E 334A

bug#18198: Gnus fails to read ~/.newsrc with group names which are not symbols

[Lars Ingebrigtsen]

Ivan Shmakov <ivan@siamics.net> writes:

> 	I believe that the problem is at the following point.
>
>   2556	      (narrow-to-region
>   2557	       (point)
>   2558	       (progn (skip-chars-forward "^ \t!:\n") (point)))
>   2559	      (goto-char (point-min))
>   2560	      (setq symbol
>   2561		    (and (/= (point-min) (point-max))
>   2562			 (let ((obarray gnus-active-hashtb)) (read buf))))
>   2563	      (widen)
>
> 	Here, the “group” field of .newsrc is read as an Emacs Lisp
> 	expression.  Which may result either in an error, or, in the
> 	case of [Hello]/World – and, similarly, (Hello)/World – in only
> 	the leading [Hello] or (Hello) being actually read.

Well, "[Hello]/World" isn't a valid newsgroup name, and I think only
newsgroups are supposed to be in ~/.newsrc?  How did you get such a
group name there?

-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/

bug#18198: Gnus fails to read ~/.newsrc with group names which are not symbols

[Ivan Shmakov]

>>>>> Lars Ingebrigtsen <larsi@gnus.org> writes:
>>>>> Ivan Shmakov <ivan@siamics.net> writes:

[…]

 >> Here, the “group” field of .newsrc is read as an Emacs Lisp
 >> expression.  Which may result either in an error, or, in the case of
 >> [Hello]/World – and, similarly, (Hello)/World – in only the leading
 >> [Hello] or (Hello) being actually read.

 > Well, "[Hello]/World" isn't a valid newsgroup name,

	Yet (Hello)/World is.  (Per the newsgroup-name and wildmat-exact
	productions given in section 9.8 of RFC 3977.)

 > and I think only newsgroups are supposed to be in ~/.newsrc?  How did
 > you get such a group name there?

	I didn’t.  The person who brought this issue to IRC has
	apparently used something like (nnimap "imap.gmail.com") for
	gnus-select-method.

	Though indeed, I know of no reason /not/ to set
	gnus-save-newsrc-file to nil in this case.

-- 
FSF associate member #7257  http://boycottsystemd.org/  … 3013 B6A0 230E 334A

bug#18198: Gnus fails to read ~/.newsrc with group names which are not symbols

[Eric Abrahamsen]

[-- Attachment #1: Type: text/plain, Size: 1199 bytes --]

Ivan Shmakov <ivan@siamics.net> writes:

>>>>>> Lars Ingebrigtsen <larsi@gnus.org> writes:
>>>>>> Ivan Shmakov <ivan@siamics.net> writes:
>
> […]
>
>  >> Here, the “group” field of .newsrc is read as an Emacs Lisp
>  >> expression.  Which may result either in an error, or, in the case of
>  >> [Hello]/World – and, similarly, (Hello)/World – in only the leading
>  >> [Hello] or (Hello) being actually read.
>
>  > Well, "[Hello]/World" isn't a valid newsgroup name,
>
> 	Yet (Hello)/World is.  (Per the newsgroup-name and wildmat-exact
> 	productions given in section 9.8 of RFC 3977.)
>
>  > and I think only newsgroups are supposed to be in ~/.newsrc?  How did
>  > you get such a group name there?
>
> 	I didn’t.  The person who brought this issue to IRC has
> 	apparently used something like (nnimap "imap.gmail.com") for
> 	gnus-select-method.
>
> 	Though indeed, I know of no reason /not/ to set
> 	gnus-save-newsrc-file to nil in this case.

This was discussed on the gnus.general list, and I produced a fix for it
(at least, I'm 95% sure it's the same issue), and then apparently never
sent the patch to the list, despite having thought I did. Attaching the
patch here.

Eric


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Handle-unquoted-IMAP-group-names.patch --]
[-- Type: text/x-diff, Size: 1064 bytes --]

From cfb7b8feefeef69ed855e90f700aef6d1b7fda39 Mon Sep 17 00:00:00 2001
From: "Eric (clem)" <eric@ericabrahamsen.net>
Date: Sat, 14 Feb 2015 18:39:10 +0800
Subject: [PATCH] Handle unquoted IMAP group names

* lisp/nnimap.el (nnimap-get-groups): Correctly read unquoted group
  names from the server LIST response.
---
 lisp/nnimap.el | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/lisp/nnimap.el b/lisp/nnimap.el
index d8b49f7..2d6a55e 100644
--- a/lisp/nnimap.el
+++ b/lisp/nnimap.el
@@ -1262,7 +1262,12 @@ If LIMIT, first try to limit the search to the N last articles."
     (while (search-forward "* LIST " nil t)
       (let ((flags (read (current-buffer)))
 	    (separator (read (current-buffer)))
-	    (group (read (current-buffer))))
+	    (group (buffer-substring-no-properties
+		    (progn (skip-chars-forward " \"")
+			   (point))
+		    (progn (move-end-of-line 1)
+			   (skip-chars-backward " 
\"")
+			   (point)))))
 	(unless (member '%NoSelect flags)
 	  (push (utf7-decode (if (stringp group)
 				 group
-- 
2.3.0

bug#18198: Gnus fails to read ~/.newsrc with group names which are not symbols

[Ivan Shmakov]

>>>>> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>>>>> Ivan Shmakov <ivan@siamics.net> writes:

[…]

 >> I didn’t.  The person who brought this issue to IRC has apparently
 >> used something like (nnimap "imap.gmail.com") for
 >> gnus-select-method.

 >> Though indeed, I know of no reason /not/ to set
 >> gnus-save-newsrc-file to nil in this case.

 > This was discussed on the gnus.general list, and I produced a fix for
 > it (at least, I'm 95% sure it's the same issue),

	It isn’t, – the one #18198 is about is that Gnus fails to read
	~/.newsrc it produces back; the one you suggest a patch for is
	about the (mis)use of ‘read’ in the Gnus IMAP implementation.

 > and then apparently never sent the patch to the list, despite having
 > thought I did.  Attaching the patch here.

	I believe that the information regarding (possible) bugs –
	including patches – should be sent to the BTS /first./

	This way, it’s easier to add late follow-ups, /and/ it still
	ends up in a mailing list (the bug-gnu-emacs@ one, that is.)

-- 
FSF associate member #7257  http://boycottsystemd.org/  … 3013 B6A0 230E 334A

bug#18198: Gnus fails to read ~/.newsrc with group names which are not symbols

[Eric Abrahamsen]

Ivan Shmakov <ivan@siamics.net> writes:

>>>>>> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>>>>>> Ivan Shmakov <ivan@siamics.net> writes:
>
> […]
>
>  >> I didn’t.  The person who brought this issue to IRC has apparently
>  >> used something like (nnimap "imap.gmail.com") for
>  >> gnus-select-method.
>
>  >> Though indeed, I know of no reason /not/ to set
>  >> gnus-save-newsrc-file to nil in this case.
>
>  > This was discussed on the gnus.general list, and I produced a fix for
>  > it (at least, I'm 95% sure it's the same issue),
>
> 	It isn’t, – the one #18198 is about is that Gnus fails to read
> 	~/.newsrc it produces back; the one you suggest a patch for is
> 	about the (mis)use of ‘read’ in the Gnus IMAP implementation.

Ah, my apologies -- just a similar-looking problem, then.

>  > and then apparently never sent the patch to the list, despite having
>  > thought I did.  Attaching the patch here.
>
> 	I believe that the information regarding (possible) bugs –
> 	including patches – should be sent to the BTS /first./
>
> 	This way, it’s easier to add late follow-ups, /and/ it still
> 	ends up in a mailing list (the bug-gnu-emacs@ one, that is.)

Sorry, didn't quite understand this -- did you mean just start another
bug report?

bug#18198: Gnus fails to read ~/.newsrc with group names which are not symbols

[Lars Ingebrigtsen]

Ivan Shmakov <ivan@siamics.net> writes:

> 	I didn’t.  The person who brought this issue to IRC has
> 	apparently used something like (nnimap "imap.gmail.com") for
> 	gnus-select-method.
>
> 	Though indeed, I know of no reason /not/ to set
> 	gnus-save-newsrc-file to nil in this case.

There should only be newsgroups in the .newsrc file -- it's for
interoperability with other newsreaders, and putting non-newsgroups in
there makes no sense.  So that does sound like a bug.

-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/

bug#18198: Gnus fails to read ~/.newsrc with group names which are not symbols

[Ivan Shmakov]

>>>>> Lars Ingebrigtsen <larsi@gnus.org> writes:
>>>>> Ivan Shmakov <ivan@siamics.net> writes:
>>>>> Lars Ingebrigtsen <larsi@gnus.org> writes:

 >>> Well, "[Hello]/World" isn't a valid newsgroup name,

 >> Yet (Hello)/World is.  (Per the newsgroup-name and wildmat-exact
 >> productions given in section 9.8 of RFC 3977.)

[…]

 >> The person who brought this issue to IRC has apparently used
 >> something like (nnimap "imap.gmail.com") for gnus-select-method.

 >> Though indeed, I know of no reason /not/ to set
 >> gnus-save-newsrc-file to nil in this case.

 > There should only be newsgroups in the .newsrc file -- it's for
 > interoperability with other newsreaders, and putting non-newsgroups
 > in there makes no sense.

	There still may be RFC-compliant newsgroup names which contain
	parentheses; and alt.foo.(bar) would fail just the same as
	[Hello]/World.

	Granted, I know of no such /public/ newsgroups, but whoever
	hosts an NNTP server could probably add a local one with ease.

 > So that does sound like a bug.

	As long as there’s no clear indication from Gnus that ~/.newsrc
	could easily break when used together with nnimap (and possibly
	other similar select methods), I’d rather deem it a bug in Gnus.

-- 
FSF associate member #7257  np. Bounzie boom — S. A. Kiviniemi B6A0 230E 334A

bug#18198: Gnus fails to read ~/.newsrc with group names which are not symbols

[Lars Ingebrigtsen]

Ivan Shmakov <ivan@siamics.net> writes:

> 	As long as there\x19s no clear indication from Gnus that ~/.newsrc
> 	could easily break when used together with nnimap (and possibly
> 	other similar select methods), I\x19d rather deem it a bug in Gnus.

It should perhaps not create a .newsrc file at all when
gnus-select-method isn't an NNTP method?  I think I'll just make that
change instead.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no