Merge lp:~mgorven/ibid/smtp-fixes into lp:~ibid-core/ibid/old-trunk-1.6

Proposed by Michael Gorven on 2010-01-05
Status: Merged
Approved by: Jonathan Hitchcock on 2010-01-07
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~mgorven/ibid/smtp-fixes
Merge into: lp:~ibid-core/ibid/old-trunk-1.6
Diff against target: 74 lines (+21/-10)
2 files modified
ibid/source/http.py (+1/-0)
ibid/source/smtp.py (+20/-10)
To merge this branch: bzr merge lp:~mgorven/ibid/smtp-fixes
Reviewer Review Type Date Requested Status
Jonathan Hitchcock Approve on 2010-01-07
Stefano Rivera 2010-01-05 Approve on 2010-01-06
Review via email: mp+16843@code.launchpad.net
To post a comment you must log in.
lp:~mgorven/ibid/smtp-fixes updated on 2010-01-05
832. By Michael Gorven on 2010-01-05

Forgot the actual method...

Stefano Rivera (stefanor) wrote :

Seems fine, although I've never used/tested SMTP

review: Approve
lp:~mgorven/ibid/smtp-fixes updated on 2010-01-06
833. By Michael Gorven on 2010-01-06

Set Content-Type headers for HTTP and email responses.

834. By Michael Gorven on 2010-01-06

Set References header.

835. By Michael Gorven on 2010-01-06

Use title case for outgoing header names.

836. By Michael Gorven on 2010-01-06

Correct header names when retrieving values.

Michael Gorven (mgorven) wrote :

Set Content-Type headers for HTTP and email, and add References header in
replies.

Stefano Rivera (stefanor) wrote :

Still approve

review: Approve
Jonathan Hitchcock (vhata) wrote :

Line 40: isn't "References" supposed to be the previous mail's References + the previous mail's Message-ID?

Line 52: why are you checking for camelcase "Subject" now? Should you not be case-insensitive?

Michael Gorven (mgorven) wrote :

On Thursday 07 January 2010 00:23:45 Jonathan Hitchcock wrote:
> Line 40: isn't "References" supposed to be the previous mail's References
> + the previous mail's Message-ID?

Technically yes, but then you also need proper length handling and whatnot.
Since most of the time the bot will be responding to the first message in a
thread it would hardly be used.

> Line 52: why are you checking for camelcase "Subject" now? Should you not
> be case-insensitive?

Because I set it as "Subject" earlier. Should probably be case-insensitive,
but it's not really necessary.

lp:~mgorven/ibid/smtp-fixes updated on 2010-01-07
837. By Michael Gorven on 2010-01-07

Set References header properly.

Michael Gorven (mgorven) wrote :

On Thursday 07 January 2010 10:15:25 Michael Gorven wrote:
> On Thursday 07 January 2010 00:23:45 Jonathan Hitchcock wrote:
> > Line 40: isn't "References" supposed to be the previous mail's
> > References + the previous mail's Message-ID?
>
> Technically yes, but then you also need proper length handling and whatnot.
> Since most of the time the bot will be responding to the first message in a
> thread it would hardly be used.

r837. I haven't done the length handling -- RFC 2822 doesn't require it.

Jonathan Hitchcock (vhata) wrote :

Bon.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'ibid/source/http.py'
2--- ibid/source/http.py 2009-02-23 20:29:44 +0000
3+++ ibid/source/http.py 2010-01-07 08:31:19 +0000
4@@ -50,6 +50,7 @@
5
6 def respond(self, event, request):
7 output = '\n'.join([response['reply'].encode('utf-8') for response in event.responses])
8+ request.setHeader('Content-Type', 'text/plain; charset=utf-8')
9 request.write(output)
10 request.finish()
11 self.log.debug(u"Responded to request from %s: %s", event.sender['connection'], output)
12
13=== modified file 'ibid/source/smtp.py'
14--- ibid/source/smtp.py 2009-12-30 16:29:10 +0000
15+++ ibid/source/smtp.py 2010-01-07 08:31:19 +0000
16@@ -58,14 +58,13 @@
17 event.public = False
18 event.addressed = True
19 event.subject = unicode(mail['subject'], 'utf-8', 'replace')
20- event.headers = dict((i[0], unicode(i[1], 'utf-8', 'replace')) for i in mail.items())
21+ event.headers = dict((i[0].lower(), unicode(i[1], 'utf-8', 'replace')) for i in mail.items())
22
23 message = mail.is_multipart() and mail.get_payload()[0].get_payload() or mail.get_payload()
24 if len(message) > 0:
25- event.message = stripsig.sub('', message).strip().replace('\n', ' ')
26+ event.message = stripsig.sub('', unicode(message, 'utf-8', 'replace')).strip().replace('\n', ' ')
27 else:
28 event.message = event.subject
29- event.message = unicode(event.message, 'utf-8', 'replace')
30
31 self.log.debug(u"Received message from %s: %s", event.sender['connection'], event.message)
32 ibid.dispatcher.dispatch(event).addCallback(ibid.sources[self.name.lower()].respond)
33@@ -113,15 +112,25 @@
34
35 for message in messages.values():
36 if 'subject' not in message:
37- message['subject'] = 'Re: ' + event['subject']
38+ message['Subject'] = 'Re: ' + event['subject']
39+ if 'message-id' in event.headers:
40+ response['In-Reply-To'] = event.headers['message-id']
41+ if 'references' in event.headers:
42+ response['References'] = '%(references)s %(message-id)s' % event.headers
43+ elif 'in-reply-to' in event.headers:
44+ response['References'] = '%(in-reply-to)s %(message-id)s' % event.headers
45+ else:
46+ response['References'] = '%(message-id)s' % event.headers
47+
48 self.send(message)
49
50 def send(self, response):
51 message = response['reply']
52- response['to'] = response['target']
53- response['date'] = smtp.rfc822date()
54- if 'subject' not in response:
55- response['subject'] = 'Message from %s' % ibid.config['botname']
56+ response['To'] = response['target']
57+ response['Date'] = smtp.rfc822date()
58+ if 'Subject' not in response:
59+ response['Subject'] = 'Message from %s' % ibid.config['botname']
60+ response['Content-Type'] = 'text/plain; charset=utf-8'
61
62 del response['target']
63 del response['source']
64@@ -133,7 +142,8 @@
65 body += '\n'
66 body += message
67
68- smtp.sendmail(self.relayhost, self.address, response['to'], body)
69- self.log.debug(u"Sent email to %s: %s", response['to'], response['subject'])
70+ port = ':' in self.relayhost and int(self.relayhost.split(':')[1]) or 25
71+ smtp.sendmail(self.relayhost.split(':')[0], self.address, response['To'], body.encode('utf-8'), port=port)
72+ self.log.debug(u"Sent email to %s: %s", response['To'], response['Subject'])
73
74 # vi: set et sta sw=4 ts=4:

Subscribers

People subscribed via source and target branches