Re: [leafnode-list] More embedded NULLs in posts

[Matthias Andree <ma@xxxxxxxxxxxxxxxxxxxxxxxxxxxx>]

Mike Castle <dalgoda@xxxxxxxxxxxxx> writes:

> On Sat, Aug 12, 2000 at 02:49:25PM -0500, Mike Castle wrote:
> > Consider this following extract from strace -s 4096 telnet newsserver nntp:
> > recv(4, ".....\r\nend\r\n\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\r\n\r\n.\r\n", 7819, 0) = 1468
> 
> [deleted]
> 
> > When the getaline() stuff came up, someone mentioned a potential missed
> > patch for the 1.9 series that I pretty much duplicated.  Would this fall
> > into the same situation?  I'm beginning to think my patch wasn't
> > sufficient.
> 
> Ok, getaline() is still broken.  Less broken, but still broken.
> 
> Just because line is zero length, it doesn't mean it's an error or end of
> file.

A line cannot be zero length unless you strip the LF.

If you get a zero length, it means that either fgets has returned NULL
(EOF/error condition without read) or fgets has returned a 0 byte as the
first byte read, on which we puke.

I know this sucks, but \0s don't belong in news in the first
place. leafnode should move the offending file out of the way, or if
it's getting news via network, discard the offending article noisily.

Since fgets does not return the count of bytes actually read, it's not
suitable for binary data (data containing NUL characters). You could get
around by switching to fread, in which case you needed to handle the
line ends yourself. Sucks even more, plus, it does not fix the actual
problem: illegal characters in article or file.

Would you care to elaborate what the rest of leafnode should do if you
return the empty string instead of the null pointer? Actually, the
entire software should bail out and give up on that article. I'm not
sure if the callers of getaline() handle this. I haven't looked. I have
the imagination it would be a caller problem, not a callee function,
thus, fixing getaline is operating on the wrong patient.

I understand that after your patch, you're silently discarding the
entire line between the NUL byte and the LF and return success. I think,
returning error condition is better in this case.



man fgets(3):

       fgets()  reads  in  at  most one less than size characters
       from stream and stores them into the buffer pointed to  by
       s.  Reading stops after an EOF or a newline.  If a newline
       is read, it is stored into the buffer.  A '\0'  is  stored
       after the last character in the buffer.

-- 
Matthias Andree

                Where do you think you're going today?

-- 
leafnode-list@xxxxxxxxxxxxxxxxxxxxxxxxxxxx -- mailing list for leafnode
To unsubscribe, send mail with "unsubscribe" in the subject to the list