Opened 5 years ago

Closed 2 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

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 sebzur 5 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 5 years ago by sebzur

comment:1 Changed 5 years ago by Alex

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

comment:2 Changed 5 years ago by Alex

  • milestone set to 1.2

comment:3 Changed 5 years ago by ubernostrum

  • milestone 1.2 deleted

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

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