Now the "from URL" tab is called "web" and it is a bit more pluggable. This will undergo revisions I'm sure, but the base architecture seems decent. Each "Provider" (media_youtube will be the first) will be able to claim an embed code or URL as their own, validate it, and convert it into a file to be saved to the DB.

The attached patch implements the current "from URL" feature using this new architecture.

In the future, media_internet providers will support a search method as well, which will make them awesome.

CommentFileSizeAuthor
#3 media.validators.patch741 byteseffulgentsia
media_internet.patch19.66 KBJacobSingh

Comments

JacobSingh’s picture

JacobSingh commented
Status: Needs review » Fixed

robeano__ is now known as robeano.
[2:53pm] james_elliott left the chat room. (Ping timeout: 265 seconds)
[2:55pm] JacobSingh: aaronwinborn: http://drupal.org/node/901728
[2:55pm] Druplicon: http://drupal.org/node/901728 => Move the from URL option to a separate module to start providing support for 3rd party providers (like youtube, etc) => Media, Code, normal, needs review, 0 comments, 1 IRC mention
[2:56pm] aaronwinborn: looks nice!
[2:56pm] james_elliott joined the chat room.
[3:00pm] JacobSingh: aaronwinborn: commit?
[3:02pm] aaronwinborn: go for it; looks like a good cleanup JacobSingh, and will be helpful to compartmentalize it all. thanks!

effulgentsia’s picture

effulgentsia commented
Assigned: Unassigned » effulgentsia
Status: Fixed » Needs work 
+++ media.pages.inc	3 Sep 2010 18:48:03 -0000
@@ -158,7 +158,7 @@
   // A blank set of allowed file extensions means no need to validate.
   if (!$validators['file_validate_extensions'][0]) {
-    unset($validators['file_validate_extensions']);
+    $validators['file_validate_extensions'] = array(0 => '');
   }

I'm concerned about this change. Not sure why exactly yet, but marking this issue "needs work" as a reminder to myself to look more closely at the security implications of this. It might be fine, in which case, I'll mark this issue "fixed" again, once I'm convinced of that.

Powered by Dreditor.

effulgentsia’s picture

effulgentsia commented
Status: Needs work » Needs review
StatusFileSize
new media.validators.patch741 bytes

I'm still not sure about this, but here's a patch to revert the hunk in #2.

JacobSingh’s picture

JacobSingh commented

Yeah, this kinda snuck in along with. Here's the problem:

Drupal has a hardcoded list of allowed extensions in file.inc (oddly, that list is different from the list in system.install for the filefield). If you don't provide a list of allowed extensions, it uses this list. That list doesn't include some very very commonly used extensions like "mp3". So because media is about supporting anything in your files table, this wasn't going to work. Of course, it still renames potentially executable files (like php, py, rb, etc) to $filename.txt. I'm not sure if that is 100% protection or not, but it might be.

There is no UI to change this default list, nor is it a variable :(

So I guess we can provide a UI to change the list for media itself, and I think we should and then default it to a really liberal list.

effulgentsia’s picture

effulgentsia commented
Status: Needs review » Fixed

Jacob committed #3 because of #902990: The specified file x could not be uploaded. Only files with the following extensions are allowed: .. Follow-ups to proper file extension validation while allowing mp3 and other useful media files can happen in #870728: Cannot upload mp3.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.