Opened 7 years ago

Closed 4 years ago

#12742 closed New feature (wontfix)

File upload handler in Comments framework

Reported by: Sebastian Żurek Owned by: nobody
Component: contrib.comments Version: master
Severity: Normal Keywords: files
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I've recently find out some annoying django.contrib.comments limitations-chain. It came out after unsuccessful custom comment form (that included FileField) submission.

This is what I mean by limitations-chain:


A)
I could not use comment model having file field (CommentSeciurityForm does not use *args, kwargs when supering forms.Form and thus files=request.FILES was not valid init kword argument in forms that base on CommentSeciurityForm)

B)
In consequence comment form post handler (view function named 'post_comment') is not using request.FILES

C)
Which is related with form template issue: check for form.is_multipart is missing (hence multipart/form-data enctype is not set)


Are those file-handling limitation intentionally created (some security reasons?)? I'm attaching a .diff with some cosmetic changes, that allow for files upload.

While browsing django.contrib.comments code and using the framework in my project I can see there's still some work to be done (there's plenty of room for some generalizations - I found API being nice, however we could make it more flexible with e.g. allowing more than one comment form definition).

However, to stay focused on one issue: could You please comment this problem (or not) with files?

Attachments (2)

file_upload.diff (2.3 KB) - added by Sebastian Żurek 7 years ago.
12742.diff (1.8 KB) - added by Thejaswi Puthraya 5 years ago.
generic patch that will also handle both #12742 and #10838

Download all attachments as: .zip

Change History (10)

Changed 7 years ago by Sebastian Żurek

Attachment: file_upload.diff added

comment:1 Changed 7 years ago by Alex Gaynor

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: UnreviewedAccepted

comment:2 Changed 7 years ago by Alex Gaynor

milestone: 1.2

comment:3 Changed 7 years ago by James Bennett

milestone: 1.2

1.2 is feature-frozen, moving this feature request off the milestone.

comment:4 Changed 6 years ago by Matt McClanahan

Severity: Normal
Type: Bug

comment:5 Changed 5 years ago by Julien Phalip

Needs documentation: set
Needs tests: set
Type: BugNew feature

comment:6 Changed 5 years ago by Thejaswi Puthraya

Easy pickings: unset
UI/UX: unset

Related (slightly duplicate) to #10838.

Changed 5 years ago by Thejaswi Puthraya

Attachment: 12742.diff added

generic patch that will also handle both #12742 and #10838

comment:7 Changed 5 years ago by Thejaswi Puthraya

The reason for adding args and kwargs to the post_comment and then passing these to the form would be to allow users to send additional data and (say) from there on to the model (could be a custom comment).

For example, with the deprecation of the RemoteAddrMiddleware, there is no standard way to fetch the IP address. The IP address could be passed as a kwarg to the form. This could prevent hacks like https://github.com/theju/django-comments-apps/blob/master/recaptcha_comments/views.py

Another case, could be where the form's rendering depends on the request (or the session) and these could be passed through the args and/or kwargs (they can't be handled in the current case).

For this ticket, the post_comment could be passed the request from which the request.FILES can be accessed.

This patch will also solve quite a lot of #10838 and which requires the user to be passed.

Just for the record, Alex (from a conversation on IRC) is not happy with passing args and kwargs to the post_comment view function. I have placed my arguments for the reason to include the args and kwargs and now the decision is left to you :)

comment:8 Changed 4 years ago by Jacob

Resolution: wontfix
Status: newclosed

django.contrib.comments has been deprecated and is no longer supported, so I'm closing this ticket. We're encouraging users to transition to a custom solution, or to a hosted solution like Disqus.

The code itself has moved to https://github.com/django/django-contrib-comments; if you want to keep using it, you could move this bug over there.

Note: See TracTickets for help on using tickets.
Back to Top