Skip to content

Commit f604b43

Browse files
committed
issue2550929 fix for my use case, need Ralf to check for his.
While working from Ralf's latest notes, I identified three cases: 1) File exists but content is empty. Remove content from form so file contents are not clobbered. 2) New file to be created (id is -1, -2 ...) but there is no content field defined in form. Remove all fields referencing the new id. 3) New file to be created (id is -1, -2 ...) and there is a content field defined in form. Let it be processed normally with no changes. Do this even if the content value is empty (allow uploading empty files). 3 may be controversial. We can't remove the content field as in 1. For new files there MUST be a content field otherwise we generate the "Edit Error: 'content'" message to the user. We can't treating the entry as in 2 and transparently remove the reference to the new file id when content is empty, This will stop the file from being created and will be confusing as the user won't see the (0 length) file created and there will be no feedback. We could: Return an error to the user telling them they can't upload an empty file. But why ask the user to resubmit just because of a 0 length file? Also although Ralf did a great job retaining all form values on an error, it looks like the file upload element of the form doesn't get it's values retained. (Indeed this may be impossible to do. I don't want the server to be able to create a hidden file upload button pre-filled with file names just waiting for me to submit the form.) So accepting the 0 length file I think is the best alternative. There are no tests as I am not sure how to make/run tests cases with alternate schemas required to make this testable.
1 parent 506ea6c commit f604b43

File tree

1 file changed

+23
-3
lines changed

1 file changed

+23
-3
lines changed

roundup/cgi/form_parser.py

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -594,11 +594,31 @@ def parse(self, create=0, num_re=re.compile('^\d+$')):
594594
# new item (any class) with no content - ignore
595595
del all_props[(cn, id)]
596596
elif isinstance(self.db.classes[cn], hyperdb.FileClass):
597-
# Avoid emptying the file
598-
if props.has_key('content') and not props['content']:
599-
del props ['content']
597+
# three cases:
598+
# id references existng file. If content is empty,
599+
# remove content from form so we don't wipe
600+
# existing file contents.
601+
# id is -1, -2 ... I.E. a new file.
602+
# if content is not defined remove all fields that
603+
# reference that file.
604+
# if content is defined, let it pass through even if
605+
# content is empty. Yes people can upload/create
606+
# empty files.
607+
print("props are %r",props)
608+
print("class is: %s",cn)
609+
if props.has_key('content'):
610+
if id is not None and \
611+
not id.startswith('-') and \
612+
not props['content']:
613+
# This is an existing file with emtpy content
614+
# value in the form.
615+
del props ['content']
616+
else:
617+
# this is a new file without any content property.
600618
if id is not None and id.startswith('-'):
601619
del all_props[(cn, id)]
620+
# if this is a new file with content (even 0 length content)
621+
# allow it through and create the zero length file.
602622
return all_props, all_links
603623

604624
def parse_file(self, fpropdef, fprops, v):

0 commit comments

Comments
 (0)