[issue28971] nntplib is broken when responses are longer than _MAXLINE

[Matej Cepl]

Matej Cepl  added the comment:

I understood, and I was saying that if you kick nntplib out of the standard 
library, than I will just embed it into my program and I won't bother to 
maintain it publicly.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28971] nntplib is broken when responses are longer than _MAXLINE

[Christian Heimes]

Christian Heimes  added the comment:

I mean fork, as in maintain your own fork on PyPI and outside of Python core.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28971] nntplib is broken when responses are longer than _MAXLINE

[Matej Cepl]

Matej Cepl  added the comment:

If that was the price of keeping nntplib inside of the Python standard library, 
yes.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28971] nntplib is broken when responses are longer than _MAXLINE

[Christian Heimes]

Christian Heimes  added the comment:

Matej, would you be interested to fork nntplib and take over maintenance and 
responsibility?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28971] nntplib is broken when responses are longer than _MAXLINE

[Christian Heimes]

Christian Heimes  added the comment:

No, the module is still supported until EOL of Python 3.9. I expect 3.9 to go 
into security fix-only mode in 2024.

--
versions: +Python 3.8, Python 3.9 -Python 3.5, Python 3.6

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28971] nntplib is broken when responses are longer than _MAXLINE

[Matej Cepl]

Matej Cepl  added the comment:

@mbussonn That's exactly the point: I completely disagree with removal of 
nntplib from the standard library, so I went through all bugs here related to 
it.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28971] nntplib is broken when responses are longer than _MAXLINE

[Matthias Bussonnier]

Matthias Bussonnier  added the comment:

Would that be anyway closed by pep 594 
(https://www.python.org/dev/peps/pep-0594/#nntplib) which suggest the removal 
nntp ?

--
nosy: +mbussonn

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28971] nntplib is broken when responses are longer than _MAXLINE

[Xavier de Gaye]

Xavier de Gaye  added the comment:

What do you mean ?
What is "this" ?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28971] nntplib is broken when responses are longer than _MAXLINE

[Matej Cepl]

Matej Cepl  added the comment:

Could @xdegaye make a PR for this?

--
nosy: +mcepl

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28971] nntplib is broken when responses are longer than _MAXLINE

[Martin Panter]

Changes by Martin Panter :


Added file: http://bugs.python.org/file46020/max_over_line.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28971] nntplib is broken when responses are longer than _MAXLINE

[Martin Panter]

Changes by Martin Panter :


Removed file: http://bugs.python.org/file46019/max_over_line.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28971] nntplib is broken when responses are longer than _MAXLINE

[Martin Panter]

Martin Panter added the comment:

Max_over_line.patch is my attempt: Keep the original _MAXLINES = 2048 code, but 
override it with _MAX_OVER_LINE = 64000 when reading OVER response lines. I 
also added a test case.

--
Added file: http://bugs.python.org/file46019/max_over_line.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28971] nntplib is broken when responses are longer than _MAXLINE

[Martin Panter]

Martin Panter added the comment:

I will try to come up with something in a few days

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28971] nntplib is broken when responses are longer than _MAXLINE

[Xavier de Gaye]

Xavier de Gaye added the comment:

Martin in response to your last review, I still hold to my opinion stated in 
msg283294 but this should not prevent this high priority issue to progress. Can 
you propose another patch.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28971] nntplib is broken when responses are longer than _MAXLINE

[Martin Panter]

Martin Panter added the comment:

The first offending message I found is number 183465:

>>> s.group("comp.lang.python")
('211 4329 179178 183507 comp.lang.python', 4329, 179178, 183507, 
'comp.lang.python')
>>> s._putcmd("OVER 183465")
>>> s._getresp()
'224 Overview information for 183465 follows'
>>> line = s.file.readline()
>>> len(line)
2702
>>> [number, subject, from_header, date, message_id, references, bytes, lines, 
>>> extra] = line.split(b"\t", 8)
>>> message_id
b''
>>> len(references)
2483
>>> len(references.split())
36

Assuming the References value is the only large field in an OVER response line, 
I would guess that a reasonable limit per line might be

200 bytes/message-id * 200 messages/thread = 40,000 bytes per OVER line

I agree with closing the session whenever the client’s protocol state is 
broken, but only for a different, tangential reason. See Issue 22350. Even if 
you raised an explicit exception when using a closed session, like in 
nntplib_maxbytes.patch, won’t that still cause the subsequent tests to fail? I 
think the best solution is to stop sharing one connection across multiple 
tests: Issue 19756.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28971] nntplib is broken when responses are longer than _MAXLINE

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

On 32-bit platform the size of empty bytes object is 17 bytes not counting 
padding and GC links. Plus 4 bytes for a pointer in a list. On 64 bit platform 
numbers are about twice larger. Therefore add at least 20-40 bytes per line.

Is not 100 MiB too large for a list in memory? For files we could use larger 
limit, but the problem is that file can be io.BytesIO().

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28971] nntplib is broken when responses are longer than _MAXLINE

[Xavier de Gaye]

Xavier de Gaye added the comment:

> If sender sends a lot of empty lines and file is not None, LF or CRLF is 
> stripped from lines

Oh, I missed that. Maybe give a weight of 4 to 8 bytes or even more to a line, 
this value being added to the bytes count whether the line is empty or not.
Do you have another suggestion ?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28971] nntplib is broken when responses are longer than _MAXLINE

[Xavier de Gaye]

Xavier de Gaye added the comment:

I responded to your last review Serhiy, but the psf mail system reports a 
delivery failure, so you may have missed the notification as well as the other 
reviewers.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28971] nntplib is broken when responses are longer than _MAXLINE

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

If sender sends a lot of empty lines and file is not None, LF or CRLF is 
stripped from lines, and len(line) is 0. Every empty line increases the size of 
the lines list by 4 or 8 bytes. Since count is not changed, the loop is not 
bounded. Every LF byte sent by malicious sender increases memory consumption by 
4 or 8 bytes.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28971] nntplib is broken when responses are longer than _MAXLINE

[Xavier de Gaye]

Xavier de Gaye added the comment:

This patch defines a _MAXBYTES limit of 100 Mb. The limit is checked upon 
reading one line and also upon reading the multi-lines of the response to a 
user command.

--
Added file: http://bugs.python.org/file45939/nntplib_maxbytes.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28971] nntplib is broken when responses are longer than _MAXLINE

[Roundup Robot]

Roundup Robot added the comment:

New changeset 1386795d266e by Xavier de Gaye in branch '3.5':
Issue #28971: Temporarily skip test_over until a permanent solution is found
https://hg.python.org/cpython/rev/1386795d266e

New changeset a33047e08711 by Xavier de Gaye in branch '3.6':
Issue #28971: Merge 3.5
https://hg.python.org/cpython/rev/a33047e08711

New changeset b51914417b41 by Xavier de Gaye in branch 'default':
Issue #28971: Merge 3.6
https://hg.python.org/cpython/rev/b51914417b41

--
nosy: +python-dev

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28971] nntplib is broken when responses are longer than _MAXLINE

[Xavier de Gaye]

Xavier de Gaye added the comment:

> I'd be in favor of temporarily skipping the affected tests with a message 
> that points back to here until we have a permanent solution.

I am working on it.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28971] nntplib is broken when responses are longer than _MAXLINE

[Christian Heimes]

Christian Heimes added the comment:

A larger limit is totally fine. The check protects against DoS with hundreds of 
MB.

I'm currently travelling and won't be available much until next week.

Am 16. Dezember 2016 19:37:03 MEZ, schrieb Xiang Zhang :
>
>Xiang Zhang added the comment:
>
>Xavier's plan sounds good. We could increase the line length limitation
>to 64K and add another limitation of the maximum lines a multi-line
>block could contain. Any limitation is violated the connection is
>refused. This situation seems quite similar to
>http.client.parse_headers, which doesn't get a standard length
>limitation or number limitation either.
>
>--
>nosy: +xiang.zhang
>
>___
>Python tracker 
>
>___

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28971] nntplib is broken when responses are longer than _MAXLINE

[Xiang Zhang]

Xiang Zhang added the comment:

Xavier's plan sounds good. We could increase the line length limitation to 64K 
and add another limitation of the maximum lines a multi-line block could 
contain. Any limitation is violated the connection is refused. This situation 
seems quite similar to http.client.parse_headers, which doesn't get a standard 
length limitation or number limitation either.

--
nosy: +xiang.zhang

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28971] nntplib is broken when responses are longer than _MAXLINE

[Zachary Ware]

Zachary Ware added the comment:

This is causing all buildbots to fail again; I'd be in favor of temporarily 
skipping the affected tests with a message that points back to here until we 
have a permanent solution.

--
keywords: +buildbot
priority: normal -> high

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28971] nntplib is broken when responses are longer than _MAXLINE

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

The limit to readline() was added to prevent consuming an excessive amount of 
memory. But this doesn't help in case of long multiline responses, since all 
lines are accumulated in a list in memory. A malicious server could cause a 
client consuming an excessive amount of memory by sending large number of short 
lines instead of one long line.

Christian, what are you think about this?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28971] nntplib is broken when responses are longer than _MAXLINE

[Xavier de Gaye]

Xavier de Gaye added the comment:

It seems that the comment placed above the definition of _MAXLINE in the 
nntplib module is not correct:
"RFC 3977 limits NNTP line length to 512 characters, including CRLF. We 
have selected 2048 just to be on the safe side."
The 512 characters limit in RFC 3977 only applies to command lines and to the 
initial line of a response.

RC 3977 says instead:
"This document does not place any limit on the length of a line in a 
multi-line block.  However, the standards that define the format of articles 
may do so."

So I think _MAXLINE should have a large value (64 K ?) and its semantic is that 
a line whose length is above that value is considered by nntplib as a Dos 
attack (and not a protocol violation). In that case nntplib should behave in 
consequence and prevent any further reads from that connection (either by 
closing the connection or raising an exception on each of these attempts). IMHO 
this should be handled in the same issue because it is one single problem, and 
this may possibly be handled in two different changesets.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28971] nntplib is broken when responses are longer than _MAXLINE

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

It seems to me there are two issues:

1) The limit of line length is not large enough.
2) After raising an error on too long line the NNTP object is left in broken 
state.

The first issue can be solved by increasing the default limit or by patching 
the nntp module in tests.

For the second issue I suggest to open new tracker issue.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28971] nntplib is broken when responses are longer than _MAXLINE

[Xavier de Gaye]

Xavier de Gaye added the comment:

The patch:
1) Increases _MAXLINE to 4096.
2) Reverts issue 16040 and that is not correct, please ignore that part.

The changes made in issue 16040 limit the amount of data read by readline() and 
does not close the nntp session when the server sends a message whose size is 
greater than this limit. In that case, should not nntplib close the session as 
the session has become unusable as shown by the test_nntplib results in 
test_nntplib.log?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28971] nntplib is broken when responses are longer than _MAXLINE

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


--
nosy: +christian.heimes, serhiy.storchaka

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28971] nntplib is broken when responses are longer than _MAXLINE

[Martin Panter]

Martin Panter added the comment:

Just a quick note for the moment: It may not be wise to drop the limit to 
readline(). That is undoing Issue 16040. Maybe we need a better test if this 
change doesn't fail the test suite.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28971] nntplib is broken when responses are longer than _MAXLINE

[Xavier de Gaye]

Xavier de Gaye added the comment:

> I found that there were three other mails raising this same  
> NNTPDataError('line too long') in the 40 last mails.

Cannot reproduce it. These three exceptions must have been NNTPProtocolError 
instead, caused by the initial NNTPDataError.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28971] nntplib is broken when responses are longer than _MAXLINE

[Xavier de Gaye]

Xavier de Gaye added the comment:

When the server sends a line longer than _MAXLINE, nntplib reads only _MAXLINE 
+ 1 bytes leaving the remaining bytes left to be processed by the next command 
that will interpret those bytes as a protocol error. Hence the failing tests 
that follow the first NNTPDataError exception in the failing test_nntplib.

The patch increases _MAXLINE from 2048 to 4096 and fixes the above problem. It 
also takes care to read up to (and including) the terminator so that the 
following lines or the terminator is not interpreted as a protocol error by the 
next nntp command.

The patch does not include a test case.

--
keywords: +patch
stage:  -> patch review
title: nntplib fails on all buildbots -> nntplib is broken when responses are 
longer than _MAXLINE
versions: +Python 2.7
Added file: http://bugs.python.org/file45901/nntplib_maxline.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com