Failure in inn-CURRENT-20081115

[Julien ÉLIE]

Hi Fred,

> With the latest snapshot, you can't get an article by its message-ID
> unless you've sent one group command.  The faulty line seems to be :
>
> --- official/nnrpd/article.c Fri Dec  5 11:01:14 2008
> +++ local/nnrpd/article.c    Fri Dec  5 23:15:21 2008
> @@ -632,7 +632,7 @@
>     }
>
>     /* Check authorizations. */
> -    if (!ok || PERMgroupmadeinvalid) {
> +    if (!ok) {
>        Reply("%d Read access denied\r\n",
>               PERMcanauthenticate ? NNTP_FAIL_AUTH_NEEDED : NNTP_ERR_ACCESS);
>        return;
>
> Now, I don't know the exact implications of this patch... but it works
> around this small glitch.

No, this patch does not work.  Suppose that news.software.nntp is denied
to an user after authentication.  Then, the following thing occurs:

GROUP news.software.nntp
211 1886 149 5504 news.software.nntp
AUTHINFO USER test
381 Enter password
AUTHINFO PASS test
281 Authentication succeeded
ARTICLE
221 149 <468bf4ab$0$322$e4fe514c at news.xs4all.nl> article
[...] <--- *** here is the article ***
ARTICLE <468bf4ab$0$322$e4fe514c at news.xs4all.nl>
502 Read access denied for this article



The right way to solve that is to add a test I forgot (sorry...) when I added
support for making a group invalid.
Here is the right patch (available from CURRENT-20081206):


--- commands.c  (révision 8200)
+++ commands.c  (copie de travail)
@@ -31,14 +31,21 @@

 /*
 **  Check after a successful authentication if the currently selected
-**  newsgroup is still readable.  AUTHINFO SASL does not need it because
-**  the NNTP protocol is reset after it.
+**  newsgroup is still readable.  AUTHINFO SASL and STARTTLS do not need
+**  it because the NNTP protocol is reset after it.
+**
+**  Return true if the group must be made invalid.
 */
 static bool
 makeGroupInvalid(void) {
     bool hookpresent = false;
     char *grplist[2];

+    /* If no group has been selected yet, it is considered as valid. */
+    if (GRPcur == NULL) {
+        return false;
+    }
+
 #ifdef DO_PYTHON
     hookpresent = PY_use_dynamic;
     if (hookpresent) {



The result is now:

ARTICLE
502 Read access denied for this article
ARTICLE <468bf4ab$0$322$e4fe514c at news.xs4all.nl>
502 Read access denied for this article


That's fine.
And of course, what previously failed, now works.


Thanks for your bug report.  If you find other bugs, feel free to tell :-)

-- 
Julien ÉLIE

« -- Essayons d'interroger ce garde habilement sans éveiller ses soupçons...
  -- Hep ! Où est enfermé Assurancetourix ? » (Astérix)