Opened 6 years ago

Closed 3 years ago

#12742 closed New feature (wontfix)

File upload handler in Comments framework

Reported by: sebzur 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


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:

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)

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

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 sebzur 6 years ago.
12742.diff (1.8 KB) - added by thejaswi_puthraya 4 years ago.
generic patch that will also handle both #12742 and #10838

Download all attachments as: .zip

Change History (10)

Changed 6 years ago by sebzur

comment:1 Changed 6 years ago by Alex

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 6 years ago by Alex

  • milestone set to 1.2

comment:3 Changed 6 years ago by ubernostrum

  • milestone 1.2 deleted

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

comment:4 Changed 5 years ago by mattmcc

  • Severity set to Normal
  • Type set to Bug

comment:5 Changed 4 years ago by julien

  • Needs documentation set
  • Needs tests set
  • Type changed from Bug to New feature

comment:6 Changed 4 years ago by thejaswi_puthraya

  • Easy pickings unset
  • UI/UX unset

Related (slightly duplicate) to #10838.

Changed 4 years ago by thejaswi_puthraya

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

comment:7 Changed 4 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

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 3 years ago by jacob

  • Resolution set to wontfix
  • Status changed from new to closed

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; 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