Bug in UploadValidator


(Jens Maier) #1

Damn, I messed up with Git, so I’m taking this from Github to here.

There’s a tiny bug in the UploadValidator which will raise a NoMethodError when being fed certain invalid data: a filename without an extension. This usually doesn’t happen because the buggy codepath is not executed when such files are actually allowed and the frontend doesn’t pass uploads without an extension on to the backend when they aren’t allowed, but I ran into it while importing attachments and avatars from an old (SMF2) forum.

--- a/lib/validators/upload_validator.rb
+++ b/lib/validators/upload_validator.rb
@@ -5,7 +5,7 @@ module Validators; end
 class Validators::UploadValidator < ActiveModel::Validator

   def validate(upload)
-    extension = File.extname(upload.original_filename)[1..-1]
+    extension = File.extname(upload.original_filename)[1..-1] || ""

     if is_authorized?(upload, extension)
       if FileHelper.is_image?(upload.original_filename)

(Erick Guan) #2
  1. Fork Discourse, get your clone.
  2. git remote add upstream <protocol>://github.com/discourse/discourse this add a remote branch for your local repo.
  3. git fetch upstream & git merge upstream/master this downloads updates from official Discourse repo and merge into yours. Now your local repo is the newest.
  4. git checkout -b <branch-name> this creates a new branch and switch to it.
  5. make any changes and commit.
  6. push to github and make a pull request!

(Jens Maier) #3

No, really? I totally didn’t know that! :grin:

I use Git routinely for pretty much all my personal development, but I rarely get a chance to use it for its intended purpose, sharing code with other people. Hence I “accidentally” force-pushed a rebase into my forked repo and killed my PR. At that point I just didn’t feel like opening yet another PR for a five character change.

The intention is appreciated though. :slight_smile:


(Régis Hanol) #4

You know you can easily do that via GitHub, right? :wink:


(Jens Maier) #5

Yeah, and I did, the first time around. Then an advanced form of laziness set in. :stuck_out_tongue:

https://github.com/discourse/discourse/pull/2554


(Régis Hanol) #6