[net.news.b] bug in inews

guy@enmasse.UUCP (Guy K. Hillyer) (12/13/84)

My copy of inews always inserted a byte with the value 0xff
(value of EOF in stdio) into messages with 0 lines in the text
part (a characteristic of many control messages).  When such messages
show up in the input to bnproc, it gets confused because it thinks
it has seen an EOF in the middle of the file.  The result is that
subsequent messages in the batch file get thrown away.

Here is the offending code, from the function 'insert' in inews.c:

	/* Write article to temp file. */
	tfp = xfopen(mktemp(ARTICLE), "w");
	if ( (c=getc(infp)) == ' ' || c == '\t' ) {
		header.intnumlines++;
		sprintf(header.numlines,"%d",header.intnumlines);
	}
	lhwrite(&header, tfp);
	/* Kludge to get around article truncation problem */
	if (c == ' ' || c == '\t' )
		putc('\n', tfp);
	putc(c,tfp);
	while (fgets(bfr, BUFLEN, infp) != NULL)
		fputs(bfr, tfp);

There is no check for the call to getc returning EOF (which
happens if the message has only control headers and no
text lines), so EOF gets written into the message file.
I fixed it here by changing this line
	
	putc(c,tfp);

to 
	if (c != EOF) putc(c,tfp);

This is a kludge on top of a kludge, but I wanted to change
the code as little as possible.  What I'm wondering is how 
widespread the problem is.  Was this kludge part of the original
release, or did it surreptitiously sneak into the code en route
to here?

			-- Guy Hillyer
			{decvax,linus,harvard!wjh12}!genrad!enmasse!guy

dmmartindale@watcgl.UUCP (Dave Martindale) (12/14/84)

In article <224@enmasse.UUCP> guy@enmasse.UUCP (Guy K. Hillyer) writes:
>My copy of inews always inserted a byte with the value 0xff
>(value of EOF in stdio) into messages with 0 lines in the text
>part (a characteristic of many control messages).  When such messages
>show up in the input to bnproc, it gets confused because it thinks
>it has seen an EOF in the middle of the file.  The result is that
>subsequent messages in the batch file get thrown away.
>
>Here is the offending code, from the function 'insert' in inews.c:
>
>	/* Write article to temp file. */
>	tfp = xfopen(mktemp(ARTICLE), "w");
>	if ( (c=getc(infp)) == ' ' || c == '\t' ) {
>		header.intnumlines++;
>		sprintf(header.numlines,"%d",header.intnumlines);
>	}
>	lhwrite(&header, tfp);
>	/* Kludge to get around article truncation problem */
>	if (c == ' ' || c == '\t' )
>		putc('\n', tfp);
>	putc(c,tfp);
>	while (fgets(bfr, BUFLEN, infp) != NULL)
>		fputs(bfr, tfp);
>
>There is no check for the call to getc returning EOF (which
>happens if the message has only control headers and no
>text lines), so EOF gets written into the message file.
>I fixed it here by changing this line
>	
>	putc(c,tfp);
>
>to 
>	if (c != EOF) putc(c,tfp);
>
>This is a kludge on top of a kludge, but I wanted to change
>the code as little as possible.  What I'm wondering is how 
>widespread the problem is.  Was this kludge part of the original
>release, or did it surreptitiously sneak into the code en route
>to here?

(sorry for including the entire article as context, but it seemed necessary.)
This is a bug in the distributed code, and I posted a fix for it
(in net.news.b) a few weeks ago.  You suggested fix is partly correct,
but the declaration of "c" should also be changed from "char" to "int"
so that the code can distinguish properly between EOF and a legitimate 0xff
character.

Also note that if you install Larry Wall's patches to inews to generate
Xref: lines, another copy of this code is inserted further down in the
same function, and that copy needs to be fixed too.

Now, if bnproc thinks a 0xff character is really an EOF, it must have
the same declaration problem.  Hang on while I look at the sources....

Well, 2.10.2 doesn't have a "bnproc", just "unbatch" and it declares
the variable it uses as an "int", so the problem should not exist there.