# HG changeset patch # User bears # Date 989408023 -3600 # Node ID 09ca6eb5c7ff86d36bbeb7dfa7db233bcb8c705d # Parent f973675760dc633cc01d4dbf3fa09cdf246a9c60 [svn] * TODO,src/client.c,src/client.h,src/fetch.c,src/fetch.h,src/noffle.c: Improve error checking during fetches. A fetch is now aborted immediately if the connection times out or if an unexpected response arrives. This should fix problems with articles appearing in the wrong group, and possibly other mysterious happenings. diff -r f973675760dc -r 09ca6eb5c7ff ChangeLog --- a/ChangeLog Wed May 09 12:15:31 2001 +0100 +++ b/ChangeLog Wed May 09 12:33:43 2001 +0100 @@ -1,3 +1,11 @@ +Wed May 9 2001 Jim Hague + + * TODO,src/client.c,src/client.h,src/fetch.c,src/fetch.h,src/noffle.c: + Improve error checking during fetches. A fetch is now aborted immediately + if the connection times out or if an unexpected response arrives. + This should fix problems with articles appearing in the wrong group, + and possibly other mysterious happenings. + Tue May 8 2001 Jim Hague * src/post.c: If post-locally is set, only do immediate local posting diff -r f973675760dc -r 09ca6eb5c7ff TODO --- a/TODO Wed May 09 12:15:31 2001 +0100 +++ b/TODO Wed May 09 12:33:43 2001 +0100 @@ -14,10 +14,6 @@ Later ----- - * Make Noffle handle a connection break-down during a fetch more gracefully. - At present it still continues to fetch articles, generating a - retrieving failed article for each article left. - * Improve performance of group database. Using GDBM is a bad choice, better use a btree from the Berkeley db library in libc. This will be a good time for a redesign of the group.h interface diff -r f973675760dc -r 09ca6eb5c7ff src/client.c --- a/src/client.c Wed May 09 12:15:31 2001 +0100 +++ b/src/client.c Wed May 09 12:33:43 2001 +0100 @@ -1,7 +1,7 @@ /* client.c - $Id: client.c 269 2001-03-13 11:46:03Z enz $ + $Id: client.c 279 2001-05-09 11:33:43Z bears $ */ #if HAVE_CONFIG_H @@ -166,7 +166,7 @@ static int getStat( void ); -static Bool +static int performAuth( void ) { int stat; @@ -176,30 +176,27 @@ if ( strcmp( user, "" ) == 0 ) { Log_err( "No username for authentication set" ); - return FALSE; + return STAT_AUTH_REQUIRED; } putCmd( "AUTHINFO USER %s", user ); stat = getStat(); if ( stat == STAT_AUTH_ACCEPTED ) - return TRUE; + return stat; else if ( stat != STAT_MORE_AUTH_REQUIRED ) { Log_err( "Username rejected. Server stat: %s", client.lastStat ); - return FALSE; + return stat; } if ( strcmp( pass, "" ) == 0 ) { Log_err( "No password for authentication set" ); - return FALSE; + return STAT_AUTH_REQUIRED; } putCmd( "AUTHINFO PASS %s", pass ); stat = getStat(); if ( stat != STAT_AUTH_ACCEPTED ) - { Log_err( "Password rejected. Server status: %s", client.lastStat ); - return FALSE; - } - return TRUE; + return stat; } static int @@ -219,7 +216,8 @@ { client.auth = TRUE; strcpy( lastCmd, client.lastCmd ); - if ( performAuth() ) + result = performAuth(); + if ( result == STAT_AUTH_ACCEPTED ) { putCmd( lastCmd ); return getStat(); @@ -533,6 +531,13 @@ if ( ! putCmd( cmd ) ) return FALSE; stat = getStat(); + if ( stat == STAT_PROGRAM_FAULT ) + return FALSE; + + /* + * Try LIST instead of LIST ACTIVE in case server doesn't + * support LIST ACTIVE. + */ if ( pattern[ 0 ] != '\0' && stat != STAT_GRPS_FOLLOW ) { *noServerPattern = TRUE; @@ -589,6 +594,7 @@ int stat; DynStr *response; const char *lines; + Bool result; ASSERT( ! Lock_gotLock() ); Utl_cpyStr( cmd, "LIST NEWSGROUPS" ); @@ -602,6 +608,10 @@ if ( ! putCmd( cmd ) ) return FALSE; stat = getStat(); + if ( stat == STAT_PROGRAM_FAULT ) + return FALSE; + + /* Try without pattern in case server doesn't support patterns. */ if ( pattern[ 0 ] != '\0' && stat != STAT_GRPS_FOLLOW ) { *noServerPattern = TRUE; @@ -623,12 +633,14 @@ return FALSE; lines = DynStr_str( response ); + result = TRUE; while ( ( lines = Utl_getLn( line, lines) ) != NULL ) { if ( sscanf( line, "%s", name ) != 1 ) { Log_err( "Unknown reply to LIST NEWSGROUPS: %s", line ); - continue; + result = FALSE; + break; } if ( *noServerPattern && ! isGetGroup( name ) ) continue; @@ -641,7 +653,7 @@ } Lock_closeDatabases(); del_DynStr( response ); - return TRUE; + return result; } Bool @@ -664,7 +676,7 @@ break; } - if ( ! doneOne ) + if ( res && ! doneOne ) res = doGetDsc( "", &noServerPattern ); del_GrEn( ge ); @@ -909,6 +921,9 @@ rmtFirst, rmtLast ); newsgroups = collectTxt(); + if ( newsgroups == NULL ) + return FALSE; + groupLines = DynStr_str( newsgroups ); } else @@ -1063,19 +1078,21 @@ return ! err; } -void +Bool Client_retrieveArt( const char *msgId ) { + Bool res = FALSE; + ASSERT( Lock_gotLock() ); if ( ! Db_contains( msgId ) ) { Log_err( "Article '%s' not prepared in database. Skipping.", msgId ); - return; + return TRUE; } if ( ! ( Db_status( msgId ) & DB_NOT_DOWNLOADED ) ) { Log_inf( "Article '%s' already retrieved. Skipping.", msgId ); - return; + return TRUE; } Lock_closeDatabases(); @@ -1084,16 +1101,18 @@ else if ( getStat() != STAT_ART_FOLLOWS ) retrievingFailed( msgId, client.lastStat ); else - retrieveAndStoreArt( msgId, 0, 0 ); + res = retrieveAndStoreArt( msgId, 0, 0 ); Lock_openDatabases(); + return res; } -void +Bool Client_retrieveArtList( const char *list, int *artcnt, int artmax ) { Str msgId; DynStr *s; const char *p; + Bool res; ASSERT( Lock_gotLock() ); Log_inf( "Retrieving article list" ); @@ -1109,7 +1128,7 @@ { retrievingFailed( msgId, "Connection broke down" ); del_DynStr( s ); - return; + return FALSE; } else DynStr_appLn( s, msgId ); @@ -1119,15 +1138,20 @@ Log_dbg( "[S FLUSH]" ); p = DynStr_str( s ); - while ( ( p = Utl_getLn( msgId, p ) ) ) + res = TRUE; + while ( res && ( p = Utl_getLn( msgId, p ) ) ) { if ( getStat() != STAT_ART_FOLLOWS ) + { retrievingFailed( msgId, client.lastStat ); - else if ( ! retrieveAndStoreArt( msgId, ++(*artcnt), artmax ) ) - break; + res = FALSE; + } + else + res = retrieveAndStoreArt( msgId, ++(*artcnt), artmax ); } del_DynStr( s ); Lock_openDatabases(); + return res; } Bool @@ -1166,24 +1190,33 @@ } Bool -Client_postArt( const char *msgId, const char *artTxt, - Str errStr ) +Client_postArt( const char *msgId, const char *artTxt, Str errStr ) { + int stat; + + errStr[0] = '\0'; + if ( ! putCmd( "POST" ) ) return FALSE; - if ( getStat() != STAT_SEND_ART ) + stat = getStat(); + if ( stat == STAT_PROGRAM_FAULT ) + return FALSE; + else if ( stat != STAT_SEND_ART ) { Log_err( "Posting of %s not allowed: %s", msgId, client.lastStat ); strcpy( errStr, client.lastStat ); - return FALSE; + return TRUE; } putTxtBuf( artTxt ); putEndOfTxt(); - if ( getStat() != STAT_POST_OK ) + stat = getStat(); + if ( stat == STAT_PROGRAM_FAULT ) + return FALSE; + else if ( stat != STAT_POST_OK ) { Log_err( "Posting of %s failed: %s", msgId, client.lastStat ); strcpy( errStr, client.lastStat ); - return FALSE; + return TRUE; } Log_inf( "Posted %s (Status: %s)", msgId, client.lastStat ); return TRUE; diff -r f973675760dc -r 09ca6eb5c7ff src/client.h --- a/src/client.h Wed May 09 12:15:31 2001 +0100 +++ b/src/client.h Wed May 09 12:33:43 2001 +0100 @@ -3,7 +3,7 @@ Noffle acting as client to other NNTP-servers - $Id: client.h 183 2000-07-25 12:14:54Z bears $ + $Id: client.h 279 2001-05-09 11:33:43Z bears $ */ #ifndef CLIENT_H @@ -62,14 +62,14 @@ /* Retrieve full article text and store it into database. */ -void +Bool Client_retrieveArt( const char *msgId ); /* Same, but for a list of msgId's (new line after each msgId). All ARTICLE commands are sent and then all answers read. */ -void +Bool Client_retrieveArtList( const char *list, int *artcnt, int artmax ); /* diff -r f973675760dc -r 09ca6eb5c7ff src/fetch.c --- a/src/fetch.c Wed May 09 12:15:31 2001 +0100 +++ b/src/fetch.c Wed May 09 12:33:43 2001 +0100 @@ -1,7 +1,7 @@ /* fetch.c - $Id: fetch.c 236 2000-12-05 19:50:09Z enz $ + $Id: fetch.c 279 2001-05-09 11:33:43Z bears $ */ #if HAVE_CONFIG_H @@ -59,7 +59,7 @@ return TRUE; } -void +Bool Fetch_getNewGrps( void ) { time_t t; @@ -71,14 +71,14 @@ if ( ! Utl_getStamp( &t, file ) ) { Log_err( "Cannot read %s. Please run noffle --query groups", file ); - return; + return FALSE; } Log_inf( "Updating groupinfo" ); - Client_getNewgrps( &t ); + return Client_getNewgrps( &t ); } /* Databases open on entry, closed on exit. */ -static void +static Bool fetchNewArts( const char *name, FetchMode mode ) { int next, first, last, refetch; @@ -88,7 +88,7 @@ Log_err( "Could not change to group %s", name ); if ( Lock_gotLock() ) Lock_closeDatabases(); - return; + return FALSE; } Client_rmtFirstLast( &first, &last ); Cont_read( name ); @@ -101,7 +101,7 @@ Cont_write(); Grp_setFirstLast( name, Cont_first(), Cont_last() ); Lock_closeDatabases(); - return; + return TRUE; } if ( first == 0 && last == 0 ) { @@ -109,7 +109,7 @@ Cont_write(); Grp_setFirstLast( name, Cont_first(), Cont_last() ); Lock_closeDatabases(); - return; + return TRUE; } if ( next > last + 1 ) { @@ -136,21 +136,21 @@ Log_inf( "Getting remote overviews %lu-%lu for group %s", first, last, name ); Lock_closeDatabases(); - Client_getOver( name, first, last, mode ); + return Client_getOver( name, first, last, mode ); } -void +Bool Fetch_getNewArts( const char *name, FetchMode mode ) { if ( ! Lock_openDatabases() ) { Log_err( "Could not open message base" ); - return; + return FALSE; } - fetchNewArts( name, mode ); + return fetchNewArts( name, mode ); } -void +Bool Fetch_updateGrps( void ) { FetchMode mode; @@ -161,7 +161,7 @@ if ( ! Lock_openDatabases() ) { Log_err( "Could not open message base" ); - return; + return FALSE; } Fetchlist_read(); size = Fetchlist_size(); @@ -170,31 +170,35 @@ Fetchlist_element( &name, &mode, i ); if ( strcmp( Grp_server( name ), fetch.serv ) == 0 ) { - fetchNewArts( name, mode ); + if ( ! fetchNewArts( name, mode ) ) + return FALSE; if ( ! Lock_openDatabases() ) { Log_err( "Could not open message base" ); - return; + return FALSE; } } } Lock_closeDatabases(); + return TRUE; } -static void +static Bool fetchMessageList( const char *list, int *artcnt, int artmax ) { const char *p; Str msgId; ASSERT( Lock_gotLock() ); - Client_retrieveArtList( list, artcnt, artmax ); + if ( ! Client_retrieveArtList( list, artcnt, artmax ) ) + return FALSE; p = list; while ( ( p = Utl_getLn( msgId, p ) ) ) Req_remove( fetch.serv, msgId ); + return TRUE; } -void +Bool Fetch_getReq_( void ) { Str msgId; @@ -202,6 +206,7 @@ DynStr *fetchList; const char *p; int count = 0, artcnt = 0, artmax = 0; + Bool res; ASSERT( fetch.ready ); Log_dbg( "Retrieving articles marked for download" ); @@ -212,7 +217,7 @@ if ( list != NULL ) del_DynStr( list ); Log_err( "Out of memory in Fetch_get_Req_"); - return; + return FALSE; } /* @@ -223,7 +228,7 @@ if ( ! Lock_openDatabases() ) { Log_err( "Could not open message base" ); - return; + return FALSE; } if ( Req_first( fetch.serv, msgId ) ) @@ -239,20 +244,22 @@ /* Retrieve in groups of up to size MAX_ARTICLE_CMDS_QUEUED. */ p = DynStr_str( list ); - while ( ( p = Utl_getLn( msgId, p ) ) != NULL ) + res = TRUE; + while ( res && ( p = Utl_getLn( msgId, p ) ) != NULL ) { DynStr_appLn( fetchList, msgId ); if ( ++count % MAX_ARTICLE_CMDS_QUEUED == 0 ) { - fetchMessageList( DynStr_str( fetchList ), &artcnt, artmax ); + res = fetchMessageList( DynStr_str( fetchList ), &artcnt, artmax ); DynStr_clear( fetchList ); } } - fetchMessageList( DynStr_str( fetchList ), &artcnt, artmax ); + res = res && fetchMessageList( DynStr_str( fetchList ), &artcnt, artmax ); del_DynStr( fetchList ); del_DynStr( list ); Lock_closeDatabases(); + return res; } static void @@ -296,13 +303,15 @@ } } -void +Bool Fetch_postArts( void ) { DynStr *s; Str msgId, errStr, sender; const char *txt; + Bool res; + res = TRUE; s = new_DynStr( 10000 ); if ( Out_first( fetch.serv, msgId, s ) ) { @@ -310,24 +319,35 @@ do { txt = DynStr_str( s ); - Out_remove( fetch.serv, msgId ); - if ( ! Client_postArt( msgId, txt, errStr ) ) - { - Utl_cpyStr( sender, Cfg_mailTo() ); - if ( strcmp( sender, "" ) == 0 - && ! Prt_searchHeader( txt, "SENDER", sender ) - && ! Prt_searchHeader( txt, "X-NOFFLE-X-SENDER", - sender ) /* see server.c */ - && ! Prt_searchHeader( txt, "FROM", sender ) ) - Log_err( "Article %s has no From/Sender/X-Sender field", - msgId ); - else - returnArticleToSender( sender, errStr, txt ); - } + if ( ! Client_postArt( msgId, txt, errStr ) ) + { + res = FALSE; + break; + } + + /* + * OK, no server communication SNAFU during post. Now, do we + * get an error response? If so, try to return article to sender. + */ + Out_remove( fetch.serv, msgId ); + if ( errStr[0] != '\0' ) + { + Utl_cpyStr( sender, Cfg_mailTo() ); + if ( strcmp( sender, "" ) == 0 + && ! Prt_searchHeader( txt, "SENDER", sender ) + && ! Prt_searchHeader( txt, "X-NOFFLE-X-SENDER", + sender ) /* see server.c */ + && ! Prt_searchHeader( txt, "FROM", sender ) ) + Log_err( "Article %s has no From/Sender/X-Sender field", + msgId ); + else + returnArticleToSender( sender, errStr, txt ); + } } while ( Out_next( msgId, s ) ); } del_DynStr( s ); + return res; } Bool diff -r f973675760dc -r 09ca6eb5c7ff src/fetch.h --- a/src/fetch.h Wed May 09 12:15:31 2001 +0100 +++ b/src/fetch.h Wed May 09 12:33:43 2001 +0100 @@ -3,7 +3,7 @@ Do the daily business by using client.c - $Id: fetch.h 51 2000-05-05 23:49:38Z uh1763 $ + $Id: fetch.h 279 2001-05-09 11:33:43Z bears $ */ #ifndef FETCH_H @@ -23,20 +23,20 @@ void Fetch_close( void ); -void +Bool Fetch_getNewGrps( void ); -void +Bool Fetch_updateGrps( void ); -void +Bool Fetch_getReq_( void ); -void +Bool Fetch_postArts( void ); /* Get new articles in group "grp", using fetch mode "mode". */ -void +Bool Fetch_getNewArts( const char *grp, FetchMode mode ); #endif diff -r f973675760dc -r 09ca6eb5c7ff src/noffle.c --- a/src/noffle.c Wed May 09 12:15:31 2001 +0100 +++ b/src/noffle.c Wed May 09 12:33:43 2001 +0100 @@ -10,7 +10,7 @@ received for some seconds (to allow multiple clients connect at the same time). - $Id: noffle.c 261 2001-02-26 22:41:26Z mnalis $ + $Id: noffle.c 279 2001-05-09 11:33:43Z bears $ */ #if HAVE_CONFIG_H @@ -162,17 +162,19 @@ while ( Cfg_nextServ( serv ) ) if ( Fetch_init( serv ) ) { - Fetch_postArts(); + Bool connOK; + + connOK = Fetch_postArts(); - Fetch_getNewGrps(); + connOK = connOK && Fetch_getNewGrps(); /* Get overviews of new articles and store IDs of new articles that are to be fetched becase of FULL or THREAD mode in the request database. */ - Fetch_updateGrps(); + connOK = connOK && Fetch_updateGrps(); /* get requested articles */ - Fetch_getReq_(); + connOK = connOK && Fetch_getReq_(); Fetch_close(); } @@ -215,9 +217,11 @@ while ( Cfg_nextServ( serv ) ) if ( Fetch_init( serv ) ) { + Bool connOK = TRUE; + if ( noffle.queryGrps ) - Client_getGrps(); - if ( noffle.queryDsc ) + connOK = Client_getGrps(); + if ( connOK && noffle.queryDsc ) Client_getDsc(); Fetch_close(); }