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

Proposed by Michael Gorven
Status: Merged
Approved by: Jonathan Hitchcock
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
Stefano Rivera Approve
Review via email: mp+16843@code.launchpad.net
To post a comment you must log in.
lp:~mgorven/ibid/smtp-fixes updated
832. By Michael Gorven

Forgot the actual method...

Revision history for this message
Stefano Rivera (stefanor) wrote :

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

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

Set Content-Type headers for HTTP and email responses.

834. By Michael Gorven

Set References header.

835. By Michael Gorven

Use title case for outgoing header names.

836. By Michael Gorven

Correct header names when retrieving values.

Revision history for this message
Michael Gorven (mgorven) wrote :

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

Revision history for this message
Stefano Rivera (stefanor) wrote :

Still approve

review: Approve
Revision history for this message
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?

Revision history for this message
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
837. By Michael Gorven

Set References header properly.

Revision history for this message
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.

Revision history for this message
Jonathan Hitchcock (vhata) wrote :

Bon.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'ibid/source/http.py'
--- ibid/source/http.py 2009-02-23 20:29:44 +0000
+++ ibid/source/http.py 2010-01-07 08:31:19 +0000
@@ -50,6 +50,7 @@
5050
51 def respond(self, event, request):51 def respond(self, event, request):
52 output = '\n'.join([response['reply'].encode('utf-8') for response in event.responses])52 output = '\n'.join([response['reply'].encode('utf-8') for response in event.responses])
53 request.setHeader('Content-Type', 'text/plain; charset=utf-8')
53 request.write(output)54 request.write(output)
54 request.finish()55 request.finish()
55 self.log.debug(u"Responded to request from %s: %s", event.sender['connection'], output)56 self.log.debug(u"Responded to request from %s: %s", event.sender['connection'], output)
5657
=== modified file 'ibid/source/smtp.py'
--- ibid/source/smtp.py 2009-12-30 16:29:10 +0000
+++ ibid/source/smtp.py 2010-01-07 08:31:19 +0000
@@ -58,14 +58,13 @@
58 event.public = False58 event.public = False
59 event.addressed = True59 event.addressed = True
60 event.subject = unicode(mail['subject'], 'utf-8', 'replace')60 event.subject = unicode(mail['subject'], 'utf-8', 'replace')
61 event.headers = dict((i[0], unicode(i[1], 'utf-8', 'replace')) for i in mail.items())61 event.headers = dict((i[0].lower(), unicode(i[1], 'utf-8', 'replace')) for i in mail.items())
6262
63 message = mail.is_multipart() and mail.get_payload()[0].get_payload() or mail.get_payload()63 message = mail.is_multipart() and mail.get_payload()[0].get_payload() or mail.get_payload()
64 if len(message) > 0:64 if len(message) > 0:
65 event.message = stripsig.sub('', message).strip().replace('\n', ' ')65 event.message = stripsig.sub('', unicode(message, 'utf-8', 'replace')).strip().replace('\n', ' ')
66 else:66 else:
67 event.message = event.subject67 event.message = event.subject
68 event.message = unicode(event.message, 'utf-8', 'replace')
6968
70 self.log.debug(u"Received message from %s: %s", event.sender['connection'], event.message)69 self.log.debug(u"Received message from %s: %s", event.sender['connection'], event.message)
71 ibid.dispatcher.dispatch(event).addCallback(ibid.sources[self.name.lower()].respond)70 ibid.dispatcher.dispatch(event).addCallback(ibid.sources[self.name.lower()].respond)
@@ -113,15 +112,25 @@
113112
114 for message in messages.values():113 for message in messages.values():
115 if 'subject' not in message:114 if 'subject' not in message:
116 message['subject'] = 'Re: ' + event['subject']115 message['Subject'] = 'Re: ' + event['subject']
116 if 'message-id' in event.headers:
117 response['In-Reply-To'] = event.headers['message-id']
118 if 'references' in event.headers:
119 response['References'] = '%(references)s %(message-id)s' % event.headers
120 elif 'in-reply-to' in event.headers:
121 response['References'] = '%(in-reply-to)s %(message-id)s' % event.headers
122 else:
123 response['References'] = '%(message-id)s' % event.headers
124
117 self.send(message)125 self.send(message)
118126
119 def send(self, response):127 def send(self, response):
120 message = response['reply']128 message = response['reply']
121 response['to'] = response['target']129 response['To'] = response['target']
122 response['date'] = smtp.rfc822date()130 response['Date'] = smtp.rfc822date()
123 if 'subject' not in response:131 if 'Subject' not in response:
124 response['subject'] = 'Message from %s' % ibid.config['botname']132 response['Subject'] = 'Message from %s' % ibid.config['botname']
133 response['Content-Type'] = 'text/plain; charset=utf-8'
125134
126 del response['target']135 del response['target']
127 del response['source']136 del response['source']
@@ -133,7 +142,8 @@
133 body += '\n'142 body += '\n'
134 body += message143 body += message
135144
136 smtp.sendmail(self.relayhost, self.address, response['to'], body)145 port = ':' in self.relayhost and int(self.relayhost.split(':')[1]) or 25
137 self.log.debug(u"Sent email to %s: %s", response['to'], response['subject'])146 smtp.sendmail(self.relayhost.split(':')[0], self.address, response['To'], body.encode('utf-8'), port=port)
147 self.log.debug(u"Sent email to %s: %s", response['To'], response['Subject'])
138148
139# vi: set et sta sw=4 ts=4:149# vi: set et sta sw=4 ts=4:

Subscribers

People subscribed via source and target branches