Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Delta Between Two Patch Sets: sitescripts/formmail/web/formmail2.py

Issue 29352643: Issue 4377 - Add Configurable Form to Email Service (Closed)
Left Patch Set: adds test and fixed logic in handler Created Sept. 12, 2016, 1:58 p.m.
Right Patch Set: fixed make_error to properly build message and fix test to fail if message not built Created Sept. 12, 2016, 3:54 p.m.
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
Left: Side by side diff | Download
Right: Side by side diff | Download
« no previous file with change/comment | « sitescripts/formmail/test/test_formmail2.py ('k') | no next file » | no next file with change/comment »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
LEFTRIGHT
1 # This file is part of the Adblock Plus web scripts, 1 # This file is part of the Adblock Plus web scripts,
2 # Copyright (C) 2006-2016 Eyeo GmbH 2 # Copyright (C) 2006-2016 Eyeo GmbH
3 # 3 #
4 # Adblock Plus is free software: you can redistribute it and/or modify 4 # Adblock Plus is free software: you can redistribute it and/or modify
5 # it under the terms of the GNU General Public License version 3 as 5 # it under the terms of the GNU General Public License version 3 as
6 # published by the Free Software Foundation. 6 # published by the Free Software Foundation.
7 # 7 #
8 # Adblock Plus is distributed in the hope that it will be useful, 8 # Adblock Plus is distributed in the hope that it will be useful,
9 # but WITHOUT ANY WARRANTY; without even the implied warranty of 9 # but WITHOUT ANY WARRANTY; without even the implied warranty of
10 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the 10 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
11 # GNU General Public License for more details. 11 # GNU General Public License for more details.
12 # 12 #
13 # You should have received a copy of the GNU General Public License 13 # You should have received a copy of the GNU General Public License
14 # along with Adblock Plus. If not, see <http://www.gnu.org/licenses/>. 14 # along with Adblock Plus. If not, see <http://www.gnu.org/licenses/>.
15 15
16 import re 16 import re
17 import datetime 17 import datetime
18 import collections 18 import collections
19 from urlparse import parse_qsl 19 from urlparse import parse_qsl
20 from sitescripts.utils import get_config, sendMail, setupStderr 20 from sitescripts.utils import get_config, sendMail, setupStderr
Sebastian Noack 2016/09/12 22:11:49 There should be a blank line between corelib and o
Vasily Kuznetsov 2016/09/13 09:03:28 Done.
21 from sitescripts.web import registerUrlHandler 21 from sitescripts.web import registerUrlHandler
22 22
23 23
24 class BadRequestError(Exception): 24 class BadRequestError(Exception):
25 pass 25 pass
26 26
27 27
28 class ConfDict(collections.OrderedDict): 28 class ConfDict(collections.OrderedDict):
29 __slots__ = ('value',) 29 __slots__ = ('value',)
Sebastian Noack 2016/09/12 22:11:50 This micro optimization doesn't do anything, since
Vasily Kuznetsov 2016/09/13 07:15:53 Yeah, with OrderedDict __slots__ is pretty much us
Vasily Kuznetsov 2016/09/13 09:03:27 Done.
30 30
31 31
32 def get_config_items(): 32 def get_config_items():
33 config = get_config() 33 config = get_config()
34 default_keys = set(config.defaults()) 34 default_keys = set(config.defaults())
35 for name, value in config.items('formmail2'): 35 for name, value in config.items('formmail2'):
36 if name not in default_keys: 36 if name not in default_keys:
37 yield name, value 37 yield name, value
38 38
39 39
(...skipping 10 matching lines...) Expand all
50 def conf_parse(conf_items): 50 def conf_parse(conf_items):
51 conf_dict = ConfDict() 51 conf_dict = ConfDict()
52 for key, value in conf_items: 52 for key, value in conf_items:
53 path = key.split('.') 53 path = key.split('.')
54 store_value(conf_dict, path, value) 54 store_value(conf_dict, path, value)
55 return conf_dict 55 return conf_dict
56 56
57 57
58 def post_handler(handler): 58 def post_handler(handler):
59 def wrapped_handler(environ, start_response): 59 def wrapped_handler(environ, start_response):
60 setupStderr(environ['wsgi.errors']) 60 setupStderr(environ['wsgi.errors'])
Sebastian Noack 2016/09/12 22:11:49 setupStderr() is depreacted. You should actually s
Vasily Kuznetsov 2016/09/13 07:15:53 Yeah, I saw the warning. Since we're not writing a
Vasily Kuznetsov 2016/09/13 09:03:27 Done.
61 response_headers = [('Content-Type', 'text/plain; charset=utf-8')] 61 response_headers = [('Content-Type', 'text/plain; charset=utf-8')]
62 62
63 try: 63 try:
64 request_method = environ['REQUEST_METHOD'].upper() 64 request_method = environ['REQUEST_METHOD'].upper()
65 url_encoded = 'application/x-www-form-urlencoded' 65 url_encoded = 'application/x-www-form-urlencoded'
66 is_url_encoded = environ.get( 66 is_url_encoded = environ.get(
67 'CONTENT_TYPE', '').startswith(url_encoded) 67 'CONTENT_TYPE', '').startswith(url_encoded)
68 if request_method != 'POST' or not is_url_encoded: 68 if request_method != 'POST' or not is_url_encoded:
69 raise BadRequestError('Unsupported request method') 69 raise BadRequestError('Unsupported request method')
70 try: 70 try:
71 request_body_length = int(environ['CONTENT_LENGTH']) 71 request_body_length = int(environ['CONTENT_LENGTH'])
Sebastian Noack 2016/09/12 22:11:49 If CONTENT_LENGTH, is not given but required, 411
Vasily Kuznetsov 2016/09/13 07:15:52 Yes, that decorator does pretty much everything th
Vasily Kuznetsov 2016/09/13 09:03:27 In the end after removing all `form_handler` logic
72 except: 72 except:
73 raise BadRequestError( 73 raise BadRequestError(
74 'Invalid or missing Content-Length header') 74 'Invalid or missing Content-Length header')
75 request_body = environ['wsgi.input'].read(request_body_length) 75 request_body = environ['wsgi.input'].read(request_body_length)
76 params = {} 76 params = {}
77 for key, value in parse_qsl(request_body): 77 for key, value in parse_qsl(request_body):
78 params[key] = value.decode('utf-8').strip() 78 params[key] = value.decode('utf-8').strip()
79 79
80 response = handler(params) 80 response = handler(params)
81 except BadRequestError as error: 81 except BadRequestError as error:
82 start_response('400 Bad Request', response_headers) 82 start_response('400 Bad Request', response_headers)
83 return str(error) 83 return str(error)
84 start_response('200 OK', response_headers) 84 start_response('200 OK', response_headers)
85 return response 85 return response
86 return wrapped_handler 86 return wrapped_handler
87 87
88 88
89 def make_error(field_name, spec, check_type, error_message): 89 def make_error(spec, check_type, default_message):
90 if check_type in spec: 90 if check_type in spec:
91 return spec[field_name].value 91 return spec[check_type].value
92 return error_message.format(field_name) 92 return default_message
93 93
94 94
95 def make_handler(name, config): 95 def make_handler(name, config):
96 try: 96 try:
97 url = config['url'].value 97 url = config['url'].value
98 except (KeyError, AttributeError): 98 except (KeyError, AttributeError):
99 raise Exception('No URL configured for form handler:' + name) 99 raise Exception('No URL configured for form handler:' + name)
100 try: 100 try:
101 template = config['template'].value 101 template = config['template'].value
102 except (KeyError, AttributeError): 102 except (KeyError, AttributeError):
103 raise Exception('No template configured for form handler:' + name) 103 raise Exception('No template configured for form handler:' + name)
104 try: 104 try:
105 fields = config['fields'] 105 fields = config['fields']
106 for field, spec in fields.items(): 106 for field, spec in fields.items():
107 spec.value = set(spec.value.replace(' ', '').split(',')) 107 spec.value = set(spec.value.replace(' ', '').split(','))
Sebastian Noack 2016/09/12 22:11:49 It seems you want to ignore optional spaces around
Vasily Kuznetsov 2016/09/13 07:15:53 Perhaps regexes are a bit overkill here. How about
Vasily Kuznetsov 2016/09/13 09:03:27 Done.
108 except KeyError: 108 except KeyError:
109 raise Exception('No fields configured for form handler:' + name) 109 raise Exception('No fields configured for form handler:' + name)
110 if len(fields) == 0: 110 if len(fields) == 0:
111 raise Exception('No fields configured for form handler:' + name) 111 raise Exception('No fields configured for form handler:' + name)
112 112
113 @post_handler 113 @post_handler
114 def handler(params): 114 def handler(params):
115 email_regex = r'^\w[\w.+!-]+@\w[\w.-]+\.[a-zA-Z]{2,6}$' 115 email_regex = r'^\w[\w.+!-]+@\w[\w.-]+\.[a-zA-Z]{2,6}$'
Sebastian Noack 2016/09/12 22:11:50 There is utils.encode_email_address(), which we us
Vasily Kuznetsov 2016/09/13 07:15:52 Yeah, this regex was just copied from formmail.py.
Vasily Kuznetsov 2016/09/13 09:03:27 Done.
116 errors = [] 116 errors = []
117 for field, spec in fields.items(): 117 for field, spec in fields.items():
118 if 'mandatory' in spec.value: 118 if 'mandatory' in spec.value:
119 if field not in params.keys(): 119 if field not in params.keys():
120 errors.append((make_error(field, spec, 'mandatory', 120 errors.append(make_error(spec, 'mandatory',
121 'No {} entered'.format(field)))) 121 'No {} entered'.format(field)))
122 if 'email' in spec.value and field in params.keys(): 122 if 'email' in spec.value and field in params.keys():
123 if not re.search(email_regex, params[field]): 123 if not re.search(email_regex, params[field]):
124 errors.append(make_error(field, spec, 'email', 124 errors.append(make_error(spec, 'email',
125 'Invalid email')) 125 'Invalid email'))
126 if errors: 126 if errors:
127 raise BadRequestError('\n'.join(errors)) 127 raise BadRequestError('\n'.join(errors))
128 template_args = { 128 template_args = {
129 'time': datetime.datetime.now(), 129 'time': datetime.datetime.now(),
130 'fields': {field: params.get(field, '') for field in fields} 130 'fields': {field: params.get(field, '') for field in fields}
131 } 131 }
132 sendMail(template, template_args) 132 sendMail(template, template_args)
133 return '' 133 return ''
134 return url, handler 134 return url, handler
135 135
136
137 conf_dict = conf_parse(get_config_items()) 136 conf_dict = conf_parse(get_config_items())
138 for name, config in conf_dict.items(): 137 for name, config in conf_dict.items():
139 url, handler = make_handler(name, config) 138 url, handler = make_handler(name, config)
140 registerUrlHandler(url, handler) 139 registerUrlHandler(url, handler)
LEFTRIGHT

Powered by Google App Engine
This is Rietveld