If the field "Permitted upload file extensions." is left empty, no files can be uploaded. I think a more reasonable behaviour would be to disable checking the extensions in such a case.

Comments

harking’s picture

harking CreditAttribution: harking commented

Sounds like the thing to do to me. +1

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard commented

This proposal goes counter to the principle of "least permissions" and to the general Drupal method for defining file filters.

traxer’s picture

traxer CreditAttribution: traxer commented

The concept of identifying the type of the file by (part of) it's name is at least doubtful. For a human this might be OK, computers do have more powerful methods.

Few people are really aware of what extensions they want to allow. See issue Field "Permitted upload file extensions." is too short.

I have often seen statements like "If you choose none, all will be available" in Drupal modules.

If you are still not convinced, here comes the compromise: Rename the field and allow regular expressions to be entered.

traxer’s picture

traxer CreditAttribution: traxer commented
Title: Allow any files to be uploaded » Allow any files to be uploaded or make "Permitted upload file extensions" a required field
Version: 5.x-1.x-dev » 5.x-2.2
Category: feature » bug

I turned the feature request into a bug report: the field "Permitted upload file extensions" is not marked as required. If the user leaves the field empty, uploads won't be possible and the user might not know why.

I updated the version this issue relates to. I did not test HEAD, but issue persists in 5.x-2.2.

jpetso’s picture

jpetso CreditAttribution: jpetso commented
Version: 5.x-2.2 » 6.x-3.x-dev
Category: bug » feature

Good point, I made the file extensions field required for now (committed to HEAD and DRUPAL-5, will be in the next release and the latest 5.x-2.x snapshots). Yes, the originally reported issue persists in CVS HEAD as well, and I'm really indifferent on this issue apart from the desire not to get critisized by dopry for causing security holes.

So, the definitive way to make me implement this is to get dopry's consent. I don't exactly remember how upload.module does this (and don't want to look it up right now), so it would also help to explain how it works there. Sorry for being a pain in the butt ;-)

bengtan’s picture

bengtan CreditAttribution: bengtan commented

Hi,

I came across this issue while evaluating filefield.module for a site I'm building. I think it is important that filefield.module can be configured to allow uploading of files of any (or no) extension. I can just see my client saying "but I want to be able to upload anything". Most people won't know in advance what extensions they want to allow. I certainly don't.

(Well, actually, I know what extensions I want to allow. I want to allow all of them :)

Furthermore, as it currently stands, I can't upload files with no extension. For example, I can't upload a file called "readme" without making .readme a permitted extension. The same would apply if I wanted to upload a file called ".htaccess" or "changelog".

I don't think allowing any extensions is necessarily a security hole, but if it has the potential to be one, let the site developer make that judgement call. Don't get in the way of developers trying to make our sites more usable for our users.

Finally, I don't really mind how this is implemented. Personally, I think we should disable extension checking if "Permitted upload file extensions" is empty, but we could also designate a special wildcard to mean any extension.

I'm prepared to submit a patch for this if there is agreement on what should be done, and if I think it will be followed through and checked in.

bengtan’s picture

bengtan CreditAttribution: bengtan commented

Oh, I just tried upload.module. It doesn't care about the file extension.

+1 to making filefield.module like upload.module.

jpetso’s picture

jpetso CreditAttribution: jpetso commented

Sounds reasonable imho, I think we should have this. I'll see what dopry thinks about this. (@dopry: You could comment here right away :D )

dopry’s picture

dopry CreditAttribution: dopry commented
Status: Active » Closed (fixed)

umm yeah... if valid extensions is empty future released will skip the extension check... just committed.