Global file-extensions limit Conversations global extensions #1660

Closed
Mnkras opened this Issue Dec 17, 2014 · 10 comments

Projects

None yet

2 participants

@Mnkras
Member
Mnkras commented Dec 17, 2014

Example:

Global list: png,jpg,gif
conversions list: png,jpg,gif,raw

If you try and upload a .raw file it will fail with invalid extension, I am not sure what the expected functionality is.

@aembler
Member
aembler commented Dec 18, 2014

Should be fixed now.

On Tue, Dec 16, 2014 at 6:15 PM, Michael Krasnow notifications@github.com
wrote:

Example:

Global list: png,jpg,gif
conversions list: png,jpg,gif,raw

If you try and upload a .raw file it will fail with invalid extension, I
am not sure what the expected functionality is.


Reply to this email directly or view it on GitHub
#1660.

@aembler aembler closed this Dec 18, 2014
@Mnkras
Member
Mnkras commented Dec 18, 2014

What is the expected functionality? I just pulled and if I try with the settings above and try to upload a .raw file the result is the same...

@aembler
Member
aembler commented Dec 19, 2014

If the conversation override checkbox is checked, whatever extensions are in there should be allowed. This is working in my local install.

@Mnkras
Member
Mnkras commented Dec 19, 2014

What about the ones set in the dashboard?

@aembler
Member
aembler commented Dec 19, 2014

Those should populate the list initially. They may not have the first time
due to a bug but if you reinstall and test with config/conversations.php it
should inherit the conversation attachment file types properly.

On Thu, Dec 18, 2014 at 4:14 PM, Michael Krasnow notifications@github.com
wrote:

What about the ones set in the dashboard?


Reply to this email directly or view it on GitHub
#1660 (comment)
.

@Mnkras
Member
Mnkras commented Dec 19, 2014

Just did a complete re-install,

at /index.php/dashboard/system/conversations/settings I added ,raw

Added a Conversations block and saved it without editing anything.

Tried to upload a .raw, was unable to with the message invalid extension.

I also tried with checking the box to "override" the defaults and it still gives me that error. Basically when it calls ->import() the import method checks against the global file allowed types and not the conversations list.

If that is the intended functionality, then the list should not be a comma separated list, but using something like select2 to only allow extensions from the global list. (and not new ones)

@aembler
Member
aembler commented Dec 19, 2014

Can anyone else reproduce this? I reproduced your steps exactly with mp3
and it uploaded fine.

On Thu, Dec 18, 2014 at 5:28 PM, Michael Krasnow notifications@github.com
wrote:

Just did a complete re-install,

at /index.php/dashboard/system/conversations/settings I added ,raw

Added a Conversations block and saved it without editing anything.

Tried to upload a .raw, was unable to with the message invalid extension.

I also tried with checking the box to "override" the defaults and it still
gives me that error. Basically when it calls ->import() the import method
checks against the global file allowed types and not the conversations
list.

If that is the intended functionality, then the list should not be a comma
separated list, but using something like select2 to only allow extensions
from the global list. (and not new ones)


Reply to this email directly or view it on GitHub
#1660 (comment)
.

@aembler
Member
aembler commented Dec 19, 2014

Actually n/m I'm wrong.

@aembler
Member
aembler commented Dec 19, 2014

Actually yes. Heh. Now that I have a chance to step through it logically
and think you're right – this is the intended behavior (although it is
weird.) The conversations attachments extensions allow a sub-set of allow
file type extensions – but they don't fully override. That's because the
Importer class checks the global file extensions. And the more I think
about this the more it makes sense. Because we'll have some hosted
solutions where we want to lock down the uploadable file types. We wouldn't
want to allow conversation administrators to complete sidestep that upload
protection by defining a conversation with their own extensions.

we should probably add some messaging that makes this clearer.

On Fri, Dec 19, 2014 at 9:28 AM, Andrew Embler andrew@concrete5.org wrote:

Actually n/m I'm wrong.

@Mnkras
Member
Mnkras commented Dec 19, 2014

What If we used the chosen/select2 plugin that we use elsewhere so that the
only allowable types to be entered are from the global file list?

On Fri, Dec 19, 2014, 12:31 PM Andrew Embler notifications@github.com
wrote:

Actually yes. Heh. Now that I have a chance to step through it logically
and think you're right - this is the intended behavior (although it is
weird.) The conversations attachments extensions allow a sub-set of allow
file type extensions - but they don't fully override. That's because the
Importer class checks the global file extensions. And the more I think
about this the more it makes sense. Because we'll have some hosted
solutions where we want to lock down the uploadable file types. We
wouldn't
want to allow conversation administrators to complete sidestep that upload
protection by defining a conversation with their own extensions.

we should probably add some messaging that makes this clearer.

On Fri, Dec 19, 2014 at 9:28 AM, Andrew Embler andrew@concrete5.org
wrote:

Actually n/m I'm wrong.

Reply to this email directly or view it on GitHub
#1660 (comment)
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment