Opened 13 years ago

Closed 12 years ago

#4049 defect closed fixed (fixed)

Error IMAP4Client.fetchSpecific an HTML body part

Reported by: Ilpo Nyyssönen Owned by:
Priority: normal Milestone:
Component: mail Keywords:
Cc: Thijs Triemstra Branch: branches/imap-fetch-specific-html-4049
branch-diff, diff-cov, branch-cov, buildbot
Author: biny, exarkun

Description

New test for twisted.mail.test.test_imap.IMAP4ClientFetchTests:

    def test_fetchSpecificHTML(self):
        d = self.client.fetchSpecific('7')
        self.assertEquals(
            self.transport.value(), '0001 FETCH 7 BODY[]\r\n')
        self.client.lineReceived('* 7 FETCH (BODY[] "<html>test</html>")')
        self.client.lineReceived('0001 OK FETCH completed')
        self.assertEquals(
            self._extractDeferredResult(d), {7: [['BODY', [], "<html>test</html>"]]})
[ERROR]: twisted.mail.test.test_imap.IMAP4ClientFetchTests.test_fetchSpecificHTML

Traceback (most recent call last):
  File "/home/biny/packages/repositories/twisted/twisted/mail/imap4.py", line 3419, in _cbFetch
    mapping = self._parseFetchPairs(values[0])
  File "/home/biny/packages/repositories/twisted/twisted/mail/imap4.py", line 3401, in _parseFetchPairs
    "Not enough arguments", fetchResponseList)
twisted.mail.imap4.IllegalServerResponse: ('Not enough arguments', ['BODY', [], '<html>test</html>'])

Attachments (1)

imap4-parser-changes.patch (11.5 KB) - added by Jean-Paul Calderone 13 years ago.
Some stuff I did, not being proposed to be committed anyplace, just sayin'

Download all attachments as: .zip

Change History (14)

comment:1 Changed 13 years ago by Ilpo Nyyssönen

Summary: Error fetchSpecific an HTML body partError IMAP4Client.fetchSpecific an HTML body part

comment:2 Changed 13 years ago by Thijs Triemstra

Cc: Thijs Triemstra added
Owner: changed from Jean-Paul Calderone to Ilpo Nyyssönen

Thanks, can you provide a unified diff against the twisted trunk? this makes it possible for us to test this change against many platforms using our build farm.

comment:3 Changed 13 years ago by Ilpo Nyyssönen

Owner: changed from Ilpo Nyyssönen to Thijs Triemstra

There isn't any change here. Only a new failing test.

comment:4 in reply to:  3 Changed 13 years ago by Thijs Triemstra

Keywords: review added
Owner: Thijs Triemstra deleted

Replying to biny:

There isn't any change here. Only a new failing test.

Right, and that test can be added by supplying a diff.. I'll put the ticket up for review anyway.

comment:5 Changed 13 years ago by therve

Author: therve
Branch: branches/image-fetch-specific-html-4049

(In [27371]) Branching to 'image-fetch-specific-html-4049'

comment:6 Changed 13 years ago by therve

Branch: branches/image-fetch-specific-html-4049branches/imap-fetch-specific-html-4049

(In [27373]) Branching to 'imap-fetch-specific-html-4049'

comment:7 Changed 13 years ago by therve

Keywords: review removed

I committed the test in the branch.

comment:8 Changed 13 years ago by Jean-Paul Calderone

(In [27553]) Recognize some non-range values and avoid treating them as a range value

refs #4049

comment:9 Changed 13 years ago by Jean-Paul Calderone

Author: thervebiny, exarkun
Keywords: review added

r27553 is a terrible hack which partially resolves this issue. I'm also attaching a patch which randomly changes a bunch of things around: this is the result of quite a few hours of trying to fix the parser for real. My conclusion at this point is that there are too many hacks stacked on top of each other in this code. See #4124.

Changed 13 years ago by Jean-Paul Calderone

Attachment: imap4-parser-changes.patch added

Some stuff I did, not being proposed to be committed anyplace, just sayin'

comment:10 Changed 12 years ago by Glyph

Owner: set to Jean-Paul Calderone

This looks OK to me, except the test has no docstring. Write one, then merge. Also, make it a good one.

comment:11 Changed 12 years ago by Glyph

Keywords: review removed

comment:12 Changed 12 years ago by Jean-Paul Calderone

Resolution: fixed
Status: newclosed

(In [27670]) Merge imap-fetch-specific-html-4049

Author: biny, exarkun Reviewer: glyph Fixes: #4049

Fix a bug in the IMAP4 client parsing code which would misinterpret many html-formatted message bodies as instead being part of the surrounding protocol bits and thus fail to actually make the body content available in the result.

comment:13 Changed 11 years ago by <automation>

Owner: Jean-Paul Calderone deleted
Note: See TracTickets for help on using tickets.