I have received a patch for NNTP with a number of corrections to fix
multiple issues:
1) NEWNEWS command was wrong - didn't expect the distributions variable
2) Log entry bug
3) Added the missing article number in the XOVER
4) Fixed article numbers to handle the case where an article is added to
an empty group.
Note that this needs further work to handle articles being deleted from
groups
5) Aggressively nulled out entries in the spooler loop
6) Added finally clauses to ensure streams are closed
7) Added a close to deal with a race condition for spool file deletion
My plan is to apply these after reviewing and testing, which should be next
week. If anyone plans to work on NNTP, please try using these patches
first.
--- Noel
Index: jakarta-james/src/java/org/apache/james/nntpserver/NNTPHandler.java
===================================================================
RCS file:
/home/cvs/jakarta-james/src/java/org/apache/james/nntpserver/NNTPHandler.java,v
retrieving revision 1.25
diff -u -r1.25 NNTPHandler.java
--- jakarta-james/src/java/org/apache/james/nntpserver/NNTPHandler.java 2 Nov 2002
09:03:52 -0000 1.25
+++ jakarta-james/src/java/org/apache/james/nntpserver/NNTPHandler.java 30 Dec 2002
+03:48:48 -0000
@@ -298,7 +298,7 @@
*/
void idleClose() {
if (getLogger() != null) {
- getLogger().error("Remote Manager Connection has idled out.");
+ getLogger().error("NNTP Connection has idled out.");
}
try {
if (socket != null) {
@@ -601,10 +601,29 @@
* an argument.
*
* @param argument the argument passed in with the NEWNEWS command.
- * Should be a date.
+ * Should be a wildmat followed by a date.
*/
private void doNEWNEWS(String argument) {
// see section 11.4
+
+ String wildmat = "*";
+
+ if (argument != null) {
+ int spaceIndex = argument.indexOf(" ");
+ if (spaceIndex >= 0) {
+ wildmat = argument.substring(0, spaceIndex);
+ argument = argument.substring(spaceIndex + 1);
+ } else {
+ getLogger().error("NEWNEWS had an invalid argument");
+ writeLoggedFlushedResponse("501 Syntax error");
+ return;
+ }
+ } else {
+ getLogger().error("NEWNEWS had a null argument");
+ writeLoggedFlushedResponse("501 Syntax error");
+ return;
+ }
+
Date theDate = null;
try {
theDate = getDateFrom(argument);
@@ -613,14 +632,14 @@
writeLoggedFlushedResponse("501 Syntax error");
return;
}
- Iterator iter = theConfigData.getNNTPRepository().getArticlesSince(theDate);
+
writeLoggedFlushedResponse("230 list of new articles by message-id follows");
- while ( iter.hasNext() ) {
+ Iterator groupIter =
+theConfigData.getNNTPRepository().getMatchedGroups(wildmat);
+ while ( groupIter.hasNext() ) {
+ Iterator articleIter =
+((NNTPGroup)(groupIter.next())).getArticlesSince(theDate);
StringBuffer iterBuffer =
new StringBuffer(64)
- .append("<")
- .append(((NNTPArticle)iter.next()).getUniqueID())
- .append(">");
+ .append(((NNTPArticle)articleIter.next()).getUniqueID());
writeLoggedResponse(iterBuffer.toString());
}
writeLoggedFlushedResponse(".");
Index:
jakarta-james/src/java/org/apache/james/nntpserver/repository/NNTPArticleImpl.java
===================================================================
RCS file:
/home/cvs/jakarta-james/src/java/org/apache/james/nntpserver/repository/NNTPArticleImpl.java,v
retrieving revision 1.10
diff -u -r1.10 NNTPArticleImpl.java
--- jakarta-james/src/java/org/apache/james/nntpserver/repository/NNTPArticleImpl.java
26 Oct 2002 04:15:29 -0000 1.10
+++ jakarta-james/src/java/org/apache/james/nntpserver/repository/NNTPArticleImpl.java
+ 30 Dec 2002 03:48:48 -0000
@@ -140,14 +140,15 @@
String references = hdr.getHeader("References",null);
long byteCount = articleFile.length();
long lineCount = -1;
- StringBuffer line=new StringBuffer(128)
+ StringBuffer line=new StringBuffer(256)
+ .append(getArticleNumber()) .append("\t")
.append(cleanHeader(subject)) .append("\t")
.append(cleanHeader(author)) .append("\t")
.append(cleanHeader(date)) .append("\t")
.append(cleanHeader(msgId)) .append("\t")
.append(cleanHeader(references)) .append("\t")
- .append(byteCount + "\t")
- .append(lineCount + "");
+ .append(byteCount) .append("\t")
+ .append(lineCount);
prt.println(line.toString());
} catch(Exception ex) { throw new NNTPException(ex); }
}
@@ -175,12 +176,13 @@
* @return the cleaned string
*/
private String cleanHeader(String field) {
- if ( field == null )
+ if ( field == null ) {
field = "";
+ }
StringBuffer sb = new StringBuffer(field);
for( int i=0 ; i<sb.length() ; i++ ) {
char c = sb.charAt(i);
- if( (c=='\n') || (c=='\t') ) {
+ if( (c=='\n') || (c=='\t') || (c=='\r')) {
sb.setCharAt(i, ' ');
}
}
Index: jakarta-james/src/java/org/apache/james/nntpserver/repository/NNTPGroupImpl.java
===================================================================
RCS file:
/home/cvs/jakarta-james/src/java/org/apache/james/nntpserver/repository/NNTPGroupImpl.java,v
retrieving revision 1.9
diff -u -r1.9 NNTPGroupImpl.java
--- jakarta-james/src/java/org/apache/james/nntpserver/repository/NNTPGroupImpl.java
26 Oct 2002 04:15:29 -0000 1.9
+++ jakarta-james/src/java/org/apache/james/nntpserver/repository/NNTPGroupImpl.java
+ 30 Dec 2002 03:48:49 -0000
@@ -101,7 +101,7 @@
int last = -1;
for ( int i = 0 ; i < list.length ; i++ ) {
int num = Integer.parseInt(list[i]);
- if ( first == -1 || num < first ) {
+ if ( (first == -1) || (num < first) ) {
first = num;
}
if ( num > last ) {
@@ -206,14 +206,22 @@
throws IOException {
File articleFile = null;
synchronized (this) {
- int artNum = getLastArticleNumber();
- articleFile = new File(root,(artNum + 1)+"");
+ if (numOfArticles < 0) {
+ throw new IllegalStateException("NNTP Group implementation is
+corrupt.");
+ }
+ int artNum = 1;
+ if (numOfArticles == 0) {
+ firstArticle = 1;
+ } else {
+ artNum = getLastArticleNumber() + 1;
+ }
+ articleFile = new File(root, (artNum + ""));
articleFile.createNewFile();
- lastArticle++;
+ lastArticle = artNum;
numOfArticles++;
}
if (getLogger().isDebugEnabled()) {
- getLogger().debug("Copying message to: "+articleFile.getAbsolutePath());
+ getLogger().debug("Copying message to: " + articleFile.getAbsolutePath());
}
FileOutputStream fout = null;
try {
Index: jakarta-james/src/java/org/apache/james/nntpserver/repository/NNTPSpooler.java
===================================================================
RCS file:
/home/cvs/jakarta-james/src/java/org/apache/james/nntpserver/repository/NNTPSpooler.java,v
retrieving revision 1.11
diff -u -r1.11 NNTPSpooler.java
--- jakarta-james/src/java/org/apache/james/nntpserver/repository/NNTPSpooler.java
26 Oct 2002 04:15:29 -0000 1.11
+++ jakarta-james/src/java/org/apache/james/nntpserver/repository/NNTPSpooler.java
+ 30 Dec 2002 03:48:49 -0000
@@ -23,6 +23,7 @@
import java.io.File;
import java.io.FileInputStream;
import java.io.FileOutputStream;
+import java.io.IOException;
import java.util.Properties;
/**
@@ -201,15 +202,15 @@
* if it loses it tries to lock and process the next article.
*/
public void run() {
- getLogger().debug("in spool thread");
+ getLogger().debug(Thread.currentThread().getName() + " is the NNTP
+spooler thread.");
try {
while ( Thread.currentThread().interrupted() == false ) {
String[] list = spoolPath.list();
for ( int i = 0 ; i < list.length ; i++ ) {
- getLogger().debug("Files to process: "+list.length);
+ getLogger().debug("Files to process: " + list.length);
if ( lock.lock(list[i]) ) {
- File f = new File(spoolPath,list[i]).getAbsoluteFile();
- getLogger().debug("Processing file:
"+f.getAbsolutePath());
+ File f = new File(spoolPath, list[i]).getAbsoluteFile();
+ getLogger().debug("Processing file: " +
+f.getAbsolutePath());
try {
process(f);
} catch(Exception ex) {
@@ -219,7 +220,10 @@
lock.unlock(list[i]);
}
}
+ list[i] = null;
}
+ list = null;
+
// this is good for other non idle threads
try {
Thread.currentThread().sleep(threadIdleTime);
@@ -228,6 +232,7 @@
}
}
} finally {
+ // Reset the interrupt state
Thread.currentThread().interrupted();
}
}
@@ -250,23 +255,40 @@
// TODO: Why is this a block?
{ // Get the message for copying to destination groups.
FileInputStream fin = new FileInputStream(spoolFile);
- msg = new MimeMessage(null,fin);
- fin.close();
+ try {
+ msg = new MimeMessage(null,fin);
+ } finally {
+ try {
+ fin.close();
+ } catch (IOException ioe) {
+ // Ignore error on close.
+ }
+ }
// ensure no duplicates exist.
String[] idheader = msg.getHeader("Message-Id");
articleID = ((idheader != null && (idheader.length > 0))? idheader[0]
: null);
if ((articleID != null) && ( articleIDRepo.isExists(articleID))) {
getLogger().debug("Message already exists: "+articleID);
- spoolFile.delete();
+ boolean delSuccess = spoolFile.delete();
+ if ( delSuccess == false ) {
+ getLogger().error("Could not delete duplicate message from
+spool: " + spoolFile.getAbsolutePath());
+ }
return;
}
if ( articleID == null ) {
articleID = articleIDRepo.generateArticleID();
msg.setHeader("Message-Id", articleID);
FileOutputStream fout = new FileOutputStream(spoolFile);
- msg.writeTo(fout);
- fout.close();
+ try {
+ msg.writeTo(fout);
+ } finally {
+ try {
+ fout.close();
+ } catch (IOException ignored) {
+ // Ignore this cleanup exception
+ }
+ }
}
}
@@ -274,7 +296,7 @@
Properties prop = new Properties();
if (headers != null) {
for ( int i = 0 ; i < headers.length ; i++ ) {
- getLogger().debug("Copying message to group: "+headers[i]);
+ getLogger().debug("Copying message to group: " + headers[i]);
NNTPGroup group = repo.getGroup(headers[i]);
if ( group == null ) {
getLogger().error("Couldn't add article with article ID " +
articleID + " to group " + headers[i] + " - group not found.");
@@ -282,8 +304,16 @@
}
FileInputStream newsStream = new FileInputStream(spoolFile);
- NNTPArticle article = group.addArticle(newsStream);
- prop.setProperty(group.getName(),article.getArticleNumber() + "");
+ try {
+ NNTPArticle article = group.addArticle(newsStream);
+ prop.setProperty(group.getName(),article.getArticleNumber() +
+"");
+ } finally {
+ try {
+ newsStream.close();
+ } catch (IOException ioe) {
+ // Ignored
+ }
+ }
}
}
articleIDRepo.addArticle(articleID,prop);
--
To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]>
For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>
