Browse Source

[FIX] mail_tracking_mailgun: json.load() won't swallow bytes (#269)

- json.load() in python under 3.6 doesn't support binary input.
- https://docs.python.org/3/whatsnew/3.6.html#json
- This way, we let requests to decode the response itself.
pull/351/head
David Vidal 6 years ago
committed by Jairo Llopis
parent
commit
430cb5f4f7
  1. 2
      mail_tracking_mailgun/__manifest__.py
  2. 5
      mail_tracking_mailgun/models/mail_tracking_email.py
  3. 3
      mail_tracking_mailgun/models/res_partner.py
  4. 25
      mail_tracking_mailgun/tests/test_mailgun.py

2
mail_tracking_mailgun/__manifest__.py

@ -6,7 +6,7 @@
{ {
"name": "Mail tracking for Mailgun", "name": "Mail tracking for Mailgun",
"summary": "Mail tracking and Mailgun webhooks integration", "summary": "Mail tracking and Mailgun webhooks integration",
"version": "11.0.1.0.0",
"version": "11.0.1.0.1",
"category": "Social Network", "category": "Social Network",
"website": "https://github.com/OCA/social", "website": "https://github.com/OCA/social",
"author": "Tecnativa, " "author": "Tecnativa, "

5
mail_tracking_mailgun/models/mail_tracking_email.py

@ -5,7 +5,6 @@
import hashlib import hashlib
import hmac import hmac
import json
import requests import requests
from datetime import datetime from datetime import datetime
from odoo import _, api, fields, models from odoo import _, api, fields, models
@ -115,7 +114,7 @@ class MailTrackingEmail(models.Model):
ts = event.get('timestamp', False) ts = event.get('timestamp', False)
try: try:
ts = float(ts) ts = float(ts)
except:
except Exception:
ts = False ts = False
if ts: if ts:
dt = datetime.utcfromtimestamp(ts) dt = datetime.utcfromtimestamp(ts)
@ -238,7 +237,7 @@ class MailTrackingEmail(models.Model):
if not res or res.status_code != 200: if not res or res.status_code != 200:
raise ValidationError(_( raise ValidationError(_(
"Couldn't retrieve Mailgun information")) "Couldn't retrieve Mailgun information"))
content = json.loads(res.content)
content = res.json()
if "items" not in content: if "items" not in content:
raise ValidationError(_("Event information not longer stored")) raise ValidationError(_("Event information not longer stored"))
for item in content["items"]: for item in content["items"]:

3
mail_tracking_mailgun/models/res_partner.py

@ -6,7 +6,6 @@
# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
import requests import requests
import json
from odoo import _, api, models from odoo import _, api, models
from odoo.exceptions import UserError from odoo.exceptions import UserError
@ -57,7 +56,7 @@ class ResPartner(models.Model):
raise UserError(_( raise UserError(_(
'Error %s trying to ' 'Error %s trying to '
'check mail' % res.status_code or 'of connection')) 'check mail' % res.status_code or 'of connection'))
content = json.loads(res.content)
content = res.json()
if 'mailbox_verification' not in content: if 'mailbox_verification' not in content:
if not self.env.context.get('mailgun_auto_check'): if not self.env.context.get('mailgun_auto_check'):
raise UserError( raise UserError(

25
mail_tracking_mailgun/tests/test_mailgun.py

@ -7,7 +7,6 @@ from odoo.tools import mute_logger
from odoo.tests.common import TransactionCase from odoo.tests.common import TransactionCase
from odoo.exceptions import UserError, ValidationError from odoo.exceptions import UserError, ValidationError
import mock import mock
import json
_packagepath = 'odoo.addons.mail_tracking_mailgun' _packagepath = 'odoo.addons.mail_tracking_mailgun'
@ -336,10 +335,10 @@ class TestMailgun(TransactionCase):
self.partner.email_bounced = False self.partner.email_bounced = False
mock_request.get.return_value.apparent_encoding = 'ascii' mock_request.get.return_value.apparent_encoding = 'ascii'
mock_request.get.return_value.status_code = 200 mock_request.get.return_value.status_code = 200
mock_request.get.return_value.content = json.dumps({
mock_request.get.return_value.json.return_value = {
'is_valid': True, 'is_valid': True,
'mailbox_verification': 'true', 'mailbox_verification': 'true',
}, ensure_ascii=True)
}
# Trigger email auto validation in partner # Trigger email auto validation in partner
self.env['ir.config_parameter'].set_param( self.env['ir.config_parameter'].set_param(
'mailgun.auto_check_partner_email', 'True') 'mailgun.auto_check_partner_email', 'True')
@ -347,24 +346,24 @@ class TestMailgun(TransactionCase):
self.assertFalse(self.partner.email_bounced) self.assertFalse(self.partner.email_bounced)
self.partner.email = 'xoxoxoxo@tecnativa.com' self.partner.email = 'xoxoxoxo@tecnativa.com'
# Not a valid mailbox # Not a valid mailbox
mock_request.get.return_value.content = json.dumps({
mock_request.get.return_value.json.return_value = {
'is_valid': True, 'is_valid': True,
'mailbox_verification': 'false', 'mailbox_verification': 'false',
}, ensure_ascii=True)
}
with self.assertRaises(UserError): with self.assertRaises(UserError):
self.partner.check_email_validity() self.partner.check_email_validity()
# Not a valid mail address # Not a valid mail address
mock_request.get.return_value.content = json.dumps({
mock_request.get.return_value.json.return_value = {
'is_valid': False, 'is_valid': False,
'mailbox_verification': 'false', 'mailbox_verification': 'false',
}, ensure_ascii=True)
}
with self.assertRaises(UserError): with self.assertRaises(UserError):
self.partner.check_email_validity() self.partner.check_email_validity()
# Unable to fully validate # Unable to fully validate
mock_request.get.return_value.content = json.dumps({
mock_request.get.return_value.json.return_value = {
'is_valid': True, 'is_valid': True,
'mailbox_verification': 'unknown', 'mailbox_verification': 'unknown',
}, ensure_ascii=True)
}
with self.assertRaises(UserError): with self.assertRaises(UserError):
self.partner.check_email_validity() self.partner.check_email_validity()
self.assertTrue(self.partner.email_bounced) self.assertTrue(self.partner.email_bounced)
@ -402,9 +401,7 @@ class TestMailgun(TransactionCase):
@mock.patch(_packagepath + '.models.mail_tracking_email.requests') @mock.patch(_packagepath + '.models.mail_tracking_email.requests')
def test_manual_check(self, mock_request): def test_manual_check(self, mock_request):
mock_request.get.return_value.content = json.dumps(self.response,
ensure_ascii=True)
mock_request.get.return_value.apparent_encoding = 'ascii'
mock_request.get.return_value.json.return_value = self.response
mock_request.get.return_value.status_code = 200 mock_request.get.return_value.status_code = 200
self.tracking_email.action_manual_check_mailgun() self.tracking_email.action_manual_check_mailgun()
event = self.env['mail.tracking.event'].search( event = self.env['mail.tracking.event'].search(
@ -417,8 +414,6 @@ class TestMailgun(TransactionCase):
with self.assertRaises(ValidationError): with self.assertRaises(ValidationError):
self.tracking_email.action_manual_check_mailgun() self.tracking_email.action_manual_check_mailgun()
mock_request.get.return_value.status_code = 200 mock_request.get.return_value.status_code = 200
mock_request.get.return_value.content = json.dumps('{}',
ensure_ascii=True)
mock_request.get.return_value.apparent_encoding = 'ascii'
mock_request.get.return_value.json.return_value = {}
with self.assertRaises(ValidationError): with self.assertRaises(ValidationError):
self.tracking_email.action_manual_check_mailgun() self.tracking_email.action_manual_check_mailgun()
Loading…
Cancel
Save