These patches are hopefully non-controversial ones. I have hand-checked
each patch against the RFC 977 and RFC 2980, and they appear to be correct.
By doing this I checked each one, rather than apply what I was sent. In one
case, NEWNEWS, I found a missing inner loop.
There may certainly be MORE things to fix, but I do not see anything here
that should get in the way of other fixes.
I am building with these changes now, and will be testing with Outlook
Express. If people would PLEASE review these and comment, it would be
appreciated.
--- Noel
Specific changes fixed:
NNTPHandler.java:
1. Corrected message from Remote Manager to NNTP.
2. Corrected parsing of NEWNEWS to skip the
newsgroups (wildmat) before getting date.
3. Corrected NEWNEWS to iterate through the
matching newsgroups AND the new messages
within each.
NNTPArticleImpl.java:
1. Added missing ArticleNumber to writeOverview protocol response.
This is per RFC 2980, section 2.8 (emphasis mine):
"Each line of output will be formatted with the ARTICLE NUMBER,
FOLLOWED BY each of the headers in the overview database or the
article itself (when the data is not available in the overview
database) for that article separated by a tab character. The
sequence of fields must be in this order: SUBJECT, AUTHOR, DATE,
MESSAGE-ID, REFERENCES, BYTE COUNT, and LINE COUNT."
2. RFC 2980 states "Note that any tab and end-of-line characters
in any header data that is returned will be converted to a
space character." Based upon RFC 977 #2.3, "Each command line
must be terminated by a CR-LF (Carriage Return - Line Feed)
pair.", I added \r to cleanHeader() on the grounds that it is
safer to be conservative.
NNTPGroupImpl.java:
This code is a mess! I would have no concerns about seeing the entire thing
rewritten. In the meantime, all I did was address boundary conditions.
1. Boundary condition to handle negative numbers.
2. Boundary condition to handle initial posting.
This code is still broken because the last article number is not persistent,
and is based upon the highest article currently present in the current
group, regardless of what might have been present and deleted.
NNTPSpooler.java:
1. Made the debug message clearer.
2. Nulled out the list array, as agreed months ago during code freeze.
3. Make sure that input/ouput streams are closed.
4. Log error if we can't delete spoolFile.
That's it. Nothing controversial as far as I can see, nor anything that
should get in the way of further cleanup, which this code desperately needs.
--- Noel
Index: 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
--- src/java/org/apache/james/nntpserver/NNTPHandler.java 2 Nov 2002 09:03:52
-0000 1.25
+++ src/java/org/apache/james/nntpserver/NNTPHandler.java 9 Jan 2003 22:15:38
+-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,28 @@
* an argument.
*
* @param argument the argument passed in with the NEWNEWS command.
- * Should be a date.
+ * Should be NEWNEWS newsgroups date time [GMT] [<distribution>]
+ * see RFC 977 #3.8, RFC 2980 #4.5.
*/
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,15 +631,20 @@
writeLoggedFlushedResponse("501 Syntax error");
return;
}
- Iterator iter = theConfigData.getNNTPRepository().getArticlesSince(theDate);
+
writeLoggedFlushedResponse("230 list of new articles by message-id follows");
- while ( iter.hasNext() ) {
- StringBuffer iterBuffer =
- new StringBuffer(64)
- .append("<")
- .append(((NNTPArticle)iter.next()).getUniqueID())
- .append(">");
- writeLoggedResponse(iterBuffer.toString());
+
+ Iterator groups = theConfigData.getNNTPRepository().getMatchedGroups(wildmat);
+ while (groups.hasNext() ) {
+ Iterator articles =
+((NNTPGroup)(groups.next())).getArticlesSince(theDate);
+ while ( articles.hasNext() ) {
+ StringBuffer iterBuffer =
+ new StringBuffer(64)
+ .append("<")
+
+.append(((NNTPArticle)articles.next()).getUniqueID())
+ .append(">");
+ writeLoggedResponse(iterBuffer.toString());
+ }
}
writeLoggedFlushedResponse(".");
}
Index: 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
--- src/java/org/apache/james/nntpserver/repository/NNTPArticleImpl.java 26 Oct
2002 04:15:29 -0000 1.10
+++ src/java/org/apache/james/nntpserver/repository/NNTPArticleImpl.java 9 Jan
+2003 22:15:38 -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); }
}
@@ -180,7 +181,7 @@
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: 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
--- src/java/org/apache/james/nntpserver/repository/NNTPGroupImpl.java 26 Oct 2002
04:15:29 -0000 1.9
+++ src/java/org/apache/james/nntpserver/repository/NNTPGroupImpl.java 9 Jan 2003
+22:15:38 -0000
@@ -206,10 +206,19 @@
throws IOException {
File articleFile = null;
synchronized (this) {
- int artNum = getLastArticleNumber();
- articleFile = new File(root,(artNum + 1)+"");
+ int artNum;
+ if (numOfArticles < 0)
+ throw new IllegalStateException("NNTP Group is corrupt (articles <
+0).");
+ else if (numOfArticles == 0) {
+ firstArticle = 1;
+ artNum = 1;
+ } else {
+ artNum = getLastArticleNumber() + 1;
+ }
+
+ articleFile = new File(root, artNum + "");
articleFile.createNewFile();
- lastArticle++;
+ lastArticle = artNum;
numOfArticles++;
}
if (getLogger().isDebugEnabled()) {
Index: 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
--- src/java/org/apache/james/nntpserver/repository/NNTPSpooler.java 26 Oct 2002
04:15:29 -0000 1.11
+++ src/java/org/apache/james/nntpserver/repository/NNTPSpooler.java 9 Jan 2003
+22:15:38 -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,7 +202,7 @@
* 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();
@@ -219,7 +220,9 @@
lock.unlock(list[i]);
}
}
+ list[i] = null;
}
+ list = null;
// this is good for other non idle threads
try {
Thread.currentThread().sleep(threadIdleTime);
@@ -250,23 +253,35 @@
// 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 _) { /* ignore close error */ }
+ }
// 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();
+ if (spoolFile.delete() == 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 _) { /* ignore close error */ }
+ }
+
}
}
@@ -282,8 +297,14 @@
}
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 _) { /* ignore close error */ }
+ }
}
}
articleIDRepo.addArticle(articleID,prop);
--
To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]>
For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>
