Hi,

I'm writing an NNTP based program that downloads articles from Usenet.
I've run into a weird problem. It seems as if there is a bug somewhere
that causes certain articles to download incorrectly (just by a byte or
two). It happens to the same articles and about 1 out of 5 have issues. So
everything seems to work ok, but somewhere something causes this weird
behavior.

The code that should work but doesn't (sometimes) downloads an article and
returns it.

Any help would be appreciated. What would be the "right" way to read the
data from the socket?

Matta

- - - - -

string NNTP::net_recv()
{
int numbytes;
string buffer;
char buf[MAXDATASIZE];

numbytes = recv(sockfd, buf, MAXDATASIZE-1, 0);

if (numbytes < 1) {
return "";
}

buf[numbytes] = '\0';

buffer = buf;
return buffer;
}

extern "C" string NNTP::get_article(string msgid)
{
string rdata, sendstr, tmp;
string::size_type pos;

sendstr = "ARTICLE ";
sendstr += msgid;
sendstr += "\r\n";

net_send(sendstr);

rdata = "";
do {
rdata += net_recv();
} while (rdata.substr(rdata.length()-5, 5) != "\r\n.\r\n");

rdata = rdata.substr(0, rdata.length() - 3);

pos = rdata.find("\r\n\r\n", 0);
rdata = rdata.substr(pos + 4, rdata.length() - (pos + 4));

if (rdata.substr(0, 2) == "..") {
rdata.erase(0, 1);
}

pos = 0;
while ((pos = rdata.find("\r\n..", pos)) != string::npos) {
rdata.erase(pos + 2, 1);
}

return rdata;
}

[ Post a follow-up to this message ]

"Matta" <matta@use.net> wrote in message
news:pan.2004.08.28.09.16.42.495384@use.net...
> Hi,
>
> I'm writing an NNTP based program that downloads articles from Usenet.
> I've run into a weird problem. It seems as if there is a bug somewhere
> that causes certain articles to download incorrectly (just by a byte or
> two). It happens to the same articles and about 1 out of 5 have issues. So
> everything seems to work ok, but somewhere something causes this weird
> behavior.
>
> The code that should work but doesn't (sometimes) downloads an article and
> returns it.
>
> Any help would be appreciated. What would be the "right" way to read the
> data from the socket?
>
> Matta
>
> - - - - -
>
> string NNTP::net_recv()
> {
> int numbytes;
> string buffer;
> char buf[MAXDATASIZE];
>
> numbytes = recv(sockfd, buf, MAXDATASIZE-1, 0);
>
> if (numbytes < 1) {
> return "";
> }
>
> buf[numbytes] = '\0';
>
> buffer = buf;
> return buffer;
> }
>
> extern "C" string NNTP::get_article(string msgid)
> {
> string rdata, sendstr, tmp;
> string::size_type pos;
>
> sendstr = "ARTICLE ";
> sendstr += msgid;
> sendstr += "\r\n";
>
> net_send(sendstr);
>
> rdata = "";
> do {
> rdata += net_recv();
> } while (rdata.substr(rdata.length()-5, 5) != "\r\n.\r\n");
>
> rdata = rdata.substr(0, rdata.length() - 3);
>
> pos = rdata.find("\r\n\r\n", 0);
> rdata = rdata.substr(pos + 4, rdata.length() - (pos + 4));
>
> if (rdata.substr(0, 2) == "..") {
> rdata.erase(0, 1);
> }
>
> pos = 0;
> while ((pos = rdata.find("\r\n..", pos)) != string::npos) {
> rdata.erase(pos + 2, 1);
> }
>
> return rdata;
> }
>
>

You cannot expect data to be neatly delivered in separate strings.  The
socket is a pipe.  Message boundaries are strictly arbitrary, and you might
receive several messages in a single read, or you might just receive a
fragment of a message.

--

Fletcher Glenn

[ Post a follow-up to this message ]

In article <Ip9Yc.12804$CV.11677@newssvr27.news.prodigy.com>,
"Fletcher Glenn" <fandmgiiNOSPAM@pacbell.net> wrote:

> "Matta" <matta@use.net> wrote in message
> news:pan.2004.08.28.09.16.42.495384@use.net... 
[vbcol=seagreen]
>
> You cannot expect data to be neatly delivered in separate strings.  The
> socket is a pipe.  Message boundaries are strictly arbitrary, and you migh
t
> receive several messages in a single read, or you might just receive a
> fragment of a message.

He seems to be handling that issue properly.  NNTP::get_article() calls
NNTP::net_recv() in a loop, appending what it gets to the rdata string,
as shown in the portion of the code I quoted.

--
Barry Margolin, barmar@alum.mit.edu
Arlington, MA
*** PLEASE post questions in newsgroups, not directly to me ***

[ Post a follow-up to this message ]

On Sat, 28 Aug 2004 12:16:42 +0300, Matta wrote:

> Hi,
>
> I'm writing an NNTP based program that downloads articles from Usenet.
> I've run into a weird problem. It seems as if there is a bug somewhere
> that causes certain articles to download incorrectly (just by a byte or
> two). It happens to the same articles and about 1 out of 5 have issues. So
> everything seems to work ok, but somewhere something causes this weird
> behavior.
>
> The code that should work but doesn't (sometimes) downloads an article and
> returns it.
>
> Any help would be appreciated. What would be the "right" way to read the
> data from the socket?

Well C++ isn't something I've used much. But...

> Matta
>
> - - - - -
>
> string NNTP::net_recv()
> {
> 	int numbytes;
> 	string buffer;
> 	char buf[MAXDATASIZE];
>
> 	numbytes = recv(sockfd, buf, MAXDATASIZE-1, 0);

You are just ignoring the error when connection closes etc. This is bad.

> 	if (numbytes < 1) {
> 		return "";
> 	}
>
> 	buf[numbytes] = '\0';
>
> 	buffer = buf;
> 	return buffer;
> }
>
> extern "C" string NNTP::get_article(string msgid) {
> 	string rdata, sendstr, tmp;
> 	string::size_type pos;
>
> 	sendstr = "ARTICLE ";
> 	sendstr += msgid;
> 	sendstr += "\r\n";
>
> 	net_send(sendstr);
>
> 	rdata = "";
> 	do {
> 		rdata += net_recv();
> 	} while (rdata.substr(rdata.length()-5, 5) != "\r\n.\r\n");

Think about what happens if rdata.length() < 5.

> 	rdata = rdata.substr(0, rdata.length() - 3);
>
> 	pos = rdata.find("\r\n\r\n", 0);
> 	rdata = rdata.substr(pos + 4, rdata.length() - (pos + 4));

I assume this is trying to find the end of the headers? And remove them?
personally I'd still check for string::npos.
Also if you do...

rdata = rdata.substr(pos + 4, string::npos);

...it defaults to from point X to the end of the string ... and that's the
default if the second parameter isn't passed.

> 	if (rdata.substr(0, 2) == "..") {
> 		rdata.erase(0, 1);
> 	}
>
> 	pos = 0;
> 	while ((pos = rdata.find("\r\n..", pos)) != string::npos) {
> 		rdata.erase(pos + 2, 1);
> 	}

This looks ok, although I'd have merged the if and the while.

> 	return rdata;
> }

--
James Antill -- james@and.org
Need an efficient and powerful string library for C?
http://www.and.org/vstr/

[ Post a follow-up to this message ]