Skip to content

Commit a0d2211

Browse files
author
Richard Jones
committed
ported CGI fixes from HEAD
1 parent 8972c59 commit a0d2211

File tree

3 files changed

+110
-16
lines changed

3 files changed

+110
-16
lines changed

‎CHANGES.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ are given with the most recent entry first.
77
- detect corrupted index and raise semi-useful exception (sf bug 666767)
88
- open server logfile unbuffered
99
- revert StringHTMLProperty to not hyperlink text by default
10+
- fixes to CGI form handling
1011

1112

1213
2003-01-10 0.5.4

‎roundup/cgi/client.py‎

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# $Id: client.py,v 1.65 2003-01-08 04:39:36 richard Exp $
1+
# $Id: client.py,v 1.65.2.1 2003-01-15 22:38:14 richard Exp $
22

33
__doc__ = """
44
WWW request handler (also used in the stand-alone server).
@@ -1150,6 +1150,7 @@ def fixNewlines(text):
11501150
text = text.replace('\r\n', '\n')
11511151
return text.replace('\r', '\n')
11521152

1153+
11531154
def parsePropsFromForm(db, cl, form, nodeid=0, num_re=re.compile('^\d+$')):
11541155
''' Pull properties for the given class out of the form.
11551156
@@ -1173,7 +1174,6 @@ def parsePropsFromForm(db, cl, form, nodeid=0, num_re=re.compile('^\d+$')):
11731174
props = {}
11741175
keys = form.keys()
11751176
properties = cl.getprops()
1176-
existing_cache = {}
11771177
for key in keys:
11781178
# see if we're performing a special multilink action
11791179
mlaction = 'set'
@@ -1188,9 +1188,10 @@ def parsePropsFromForm(db, cl, form, nodeid=0, num_re=re.compile('^\d+$')):
11881188

11891189
# does the property exist?
11901190
if not properties.has_key(propname):
1191-
if mlaction == 'remove':
1192-
raise ValueError, 'You have submitted a remove action for'\
1193-
' the property "%s" which doesn\'t exist'%propname
1191+
if mlaction != 'set':
1192+
raise ValueError, 'You have submitted a %s action for'\
1193+
' the property "%s" which doesn\'t exist'%(mlaction,
1194+
propname)
11941195
continue
11951196
proptype = properties[propname]
11961197

@@ -1208,6 +1209,9 @@ def parsePropsFromForm(db, cl, form, nodeid=0, num_re=re.compile('^\d+$')):
12081209
# it's a MiniFieldStorage, but may be a comma-separated list
12091210
# of values
12101211
value = [i.strip() for i in value.value.split(',')]
1212+
1213+
# filter out the empty bits
1214+
value = filter(None, value)
12111215
else:
12121216
# multiple values are not OK
12131217
if isinstance(value, type([])):
@@ -1218,8 +1222,6 @@ def parsePropsFromForm(db, cl, form, nodeid=0, num_re=re.compile('^\d+$')):
12181222
value = value.value.strip()
12191223

12201224
if isinstance(proptype, hyperdb.String):
1221-
if not value:
1222-
continue
12231225
# fix the CRLF/CR -> LF stuff
12241226
value = fixNewlines(value)
12251227
elif isinstance(proptype, hyperdb.Password):
@@ -1247,7 +1249,7 @@ def parsePropsFromForm(db, cl, form, nodeid=0, num_re=re.compile('^\d+$')):
12471249
value = None
12481250
elif isinstance(proptype, hyperdb.Link):
12491251
# see if it's the "no selection" choice
1250-
if value == '-1':
1252+
if value == '-1' or not value:
12511253
# if we're creating, just don't include this property
12521254
if not nodeid:
12531255
continue
@@ -1270,12 +1272,13 @@ def parsePropsFromForm(db, cl, form, nodeid=0, num_re=re.compile('^\d+$')):
12701272
elif isinstance(proptype, hyperdb.Multilink):
12711273
# perform link class key value lookup if necessary
12721274
link = proptype.classname
1275+
link_cl = db.classes[link]
12731276
l = []
12741277
for entry in value:
12751278
if not entry: continue
12761279
if not num_re.match(entry):
12771280
try:
1278-
entry = db.classes[link].lookup(entry)
1281+
entry = link_cl.lookup(entry)
12791282
except KeyError:
12801283
raise ValueError, _('property "%(propname)s": '
12811284
'"%(value)s" not an entry of %(classname)s')%{
@@ -1295,8 +1298,10 @@ def parsePropsFromForm(db, cl, form, nodeid=0, num_re=re.compile('^\d+$')):
12951298
# we're modifying the list - get the current list of ids
12961299
if props.has_key(propname):
12971300
existing = props[propname]
1298-
else:
1301+
elif nodeid:
12991302
existing = cl.get(nodeid, propname, [])
1303+
else:
1304+
existing = []
13001305

13011306
# now either remove or add
13021307
if mlaction == 'remove':
@@ -1335,10 +1340,21 @@ def parsePropsFromForm(db, cl, form, nodeid=0, num_re=re.compile('^\d+$')):
13351340
if not properties.has_key(propname):
13361341
raise
13371342

1343+
# existing may be None, which won't equate to empty strings
1344+
if not existing and not value:
1345+
continue
1346+
1347+
# existing will come out unsorted in some cases
1348+
if isinstance(proptype, hyperdb.Multilink):
1349+
existing.sort()
1350+
13381351
# if changed, set it
13391352
if value != existing:
13401353
props[propname] = value
13411354
else:
1355+
# don't bother setting empty/unset values
1356+
if not value:
1357+
continue
13421358
props[propname] = value
13431359

13441360
# see if all the required properties have been supplied
@@ -1350,5 +1366,3 @@ def parsePropsFromForm(db, cl, form, nodeid=0, num_re=re.compile('^\d+$')):
13501366
raise ValueError, 'Required %s %s not supplied'%(p, ', '.join(required))
13511367

13521368
return props
1353-
1354-

‎test/test_cgi.py‎

Lines changed: 83 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
# but WITHOUT ANY WARRANTY; without even the implied warranty of
99
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
1010
#
11-
# $Id: test_cgi.py,v 1.4 2003-01-15 11:14:01 richard Exp $
11+
# $Id: test_cgi.py,v 1.4.2.1 2003-01-15 22:38:14 richard Exp $
1212

1313
import unittest, os, shutil, errno, sys, difflib, cgi
1414

@@ -58,9 +58,14 @@ def testNothing(self):
5858
makeForm({})), {})
5959

6060
def testNothingWithRequired(self):
61-
form = makeForm({':required': 'title'})
6261
self.assertRaises(ValueError, client.parsePropsFromForm, self.db,
63-
self.db.issue, form)
62+
self.db.issue, makeForm({':required': 'title'}))
63+
self.assertRaises(ValueError, client.parsePropsFromForm, self.db,
64+
self.db.issue, makeForm({':required': 'title,status',
65+
'status':'1'}))
66+
self.assertRaises(ValueError, client.parsePropsFromForm, self.db,
67+
self.db.issue, makeForm({':required': ['title','status'],
68+
'status':'1'}))
6469

6570
#
6671
# String
@@ -87,6 +92,38 @@ def testEmptyStringSet(self):
8792
self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
8893
makeForm({'title': ' '}), nodeid), {'title': ''})
8994

95+
#
96+
# Link
97+
#
98+
def testEmptyLink(self):
99+
self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
100+
makeForm({'status': ''})), {})
101+
self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
102+
makeForm({'status': ' '})), {})
103+
self.assertRaises(ValueError, client.parsePropsFromForm, self.db,
104+
self.db.issue, makeForm({'status': ['', '']}))
105+
self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
106+
makeForm({'status': '-1'})), {})
107+
108+
def testSetLink(self):
109+
self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
110+
makeForm({'status': 'unread'})), {'status': '1'})
111+
self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
112+
makeForm({'status': '1'})), {'status': '1'})
113+
114+
def testUnsetLink(self):
115+
nodeid = self.db.issue.create(status='unread')
116+
self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
117+
makeForm({'status': '-1'}), nodeid), {'status': None})
118+
119+
def testInvalidLinkValue(self):
120+
# XXX This is not the current behaviour - should we enforce this?
121+
# self.assertRaises(IndexError, client.parsePropsFromForm, self.db,
122+
# self.db.issue, makeForm({'status': '4'}))
123+
self.assertRaises(ValueError, client.parsePropsFromForm, self.db,
124+
self.db.issue, makeForm({'status': 'frozzle'}))
125+
# XXX need a test for the TypeError where the link class doesn't define a key?
126+
90127
#
91128
# Multilink
92129
#
@@ -124,7 +161,7 @@ def testInvalidMultilinkValue(self):
124161
self.db.issue, makeForm({'nosy': 'frozzle'}))
125162
self.assertRaises(ValueError, client.parsePropsFromForm, self.db,
126163
self.db.issue, makeForm({'nosy': '1,frozzle'}))
127-
# XXX need a test for the TypeError (where the ML class doesn't define a key
164+
# XXX need a test for the TypeError (where the ML class doesn't define a key?
128165

129166
def testMultilinkAdd(self):
130167
nodeid = self.db.issue.create(nosy=['1'])
@@ -162,6 +199,22 @@ def testMultilinkRemove(self):
162199
self.assertRaises(ValueError, client.parsePropsFromForm, self.db,
163200
self.db.issue, makeForm({':remove:nosy': '4'}), nodeid)
164201

202+
def testMultilinkRetired(self):
203+
self.db.user.retire('2')
204+
self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
205+
makeForm({'nosy': ['2','3']})), {'nosy': ['2','3']})
206+
nodeid = self.db.issue.create(nosy=['1','2'])
207+
self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
208+
makeForm({':remove:nosy': '2'}), nodeid), {'nosy': ['1']})
209+
self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
210+
makeForm({':add:nosy': '3'}), nodeid), {'nosy': ['1','2','3']})
211+
212+
def testAddRemoveNonexistant(self):
213+
self.assertRaises(ValueError, client.parsePropsFromForm, self.db,
214+
self.db.issue, makeForm({':remove:foo': '2'}))
215+
self.assertRaises(ValueError, client.parsePropsFromForm, self.db,
216+
self.db.issue, makeForm({':add:foo': '2'}))
217+
165218
#
166219
# Password
167220
#
@@ -196,6 +249,32 @@ def testEmptyPasswordNOTSet(self):
196249
self.assertEqual(client.parsePropsFromForm(self.db, self.db.user,
197250
makeForm({'password': '', 'password:confirm': ''}), nodeid), {})
198251

252+
#
253+
# Boolean
254+
#
255+
# XXX this needs a property to work on.
256+
# def testEmptyBoolean(self):
257+
# self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
258+
# makeForm({'title': ''})), {})
259+
# self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
260+
# makeForm({'title': ' '})), {})
261+
# self.assertRaises(ValueError, client.parsePropsFromForm, self.db,
262+
# self.db.issue, makeForm({'title': ['', '']}))
263+
264+
# def testSetBoolean(self):
265+
# self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
266+
# makeForm({'title': 'foo'})), {'title': 'foo'})
267+
# self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
268+
# makeForm({'title': 'a\r\nb\r\n'})), {'title': 'a\nb'})
269+
270+
# def testEmptyBooleanSet(self):
271+
# nodeid = self.db.issue.create(title='foo')
272+
# self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
273+
# makeForm({'title': ''}), nodeid), {'title': ''})
274+
# nodeid = self.db.issue.create(title='foo')
275+
# self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
276+
# makeForm({'title': ' '}), nodeid), {'title': ''})
277+
199278

200279
def suite():
201280
l = [unittest.makeSuite(FormTestCase),

0 commit comments

Comments
 (0)