Media Upload mimeType validation bug

[vivekbhusal]

I am working on plugin that lets user upload/select doc or docx file and followed by running task to parse those file and create post.

Problem: I am using media uploader (wp.media.frame) to select or upload files of only mimetype
["application/vnd.openxmlformats-officedocument.wordprocessingml.document", "application/msword"]

user can choose and upload the file, but after the file is uploaded, the mediaframe should be updated to display the newly uploaded file asynchronously, but it doesn't. However it does if I am only one mimeType.
Here is the Code I use for media upload

 // If the media frame already exists, reopen it.
        var alex_doc_uploader = $thisButton.data('file_frame');
        if (alex_doc_uploader) {
            alex_doc_uploader.open();
            return;
        }

        // Create the media frame.
        alex_doc_uploader = wp.media.frames.file_frame = wp.media({
            title: "Select Documents",
            button: {
                text: "Select Documents"
            },
            library : {
              type:   ["application/vnd.openxmlformats-officedocument.wordprocessingml.document", "application/msword"]
            },
            multiple: true  // Set to true to allow multiple files to be selected
        });



        $thisButton.data('file_frame', alex_doc_uploader);


        /** NOT very relevant after this but still adding it here**/
        alex_doc_uploader.on('select', function(){
            var selection = alex_doc_uploader.state().get('selection');

            selection.map( function(attachment) {
                if(attachment.attributes.filename)
                    attachments.push({'id':attachment.id, 'attachment':attachment.attributes.filename});
            });

            if(attachments.length > 0) {
                $("#alex-doc-import-selector").hide();
                $('#alex-doc-file-insert').show();

                $.each(attachments, function(index, value) {
                    $("#alex-import-selected-file-type").append("<li data-id='"+index+"'>"+value.attachment+"</li>");
                });
                $("#alex-doc-popup-container").dialog('open');
            }
        });

Issue explained here aswell: ​StackOverfflow

So on tracking the problem I found, after upload it verifies the validity of newly added content on media-models.js .
In the process calls filter.type function at line 897 media-models.js

/**
		 * @static
		 * @param {wp.media.model.Attachment} attachment
		 *
		 * @this wp.media.model.Attachments
		 *
		 * @returns {Boolean}
		 */
		type: function( attachment ) {
			var type = this.props.get('type');
			return ! type || -1 !== type.indexOf( attachment.get('type') );
		},

Here it get attachment.get(type) as "application" so it works when using one mimeType, because it uses index of to compare in string, but won't work for mimeType array.

Here is the my log,
var type = ["application/vnd.openxmlformats-officedocument.wordprocessingml.document", "application/msword"]
attachment.get("type") gives "application"

and attachment object is

Solution:

                 /**
                 * @since 4.2.3
		 * @static
		 * @param {wp.media.model.Attachment} attachment
		 *
		 * @this wp.media.model.Attachments
		 *
		 * @returns {Boolean}
		 */
		type: function( attachment ) {
			var type = this.props.get('type');
			return ! type || -1 !== type.indexOf( attachment.get('mime') );
		},

[vivekbhusal]

Screenshot of attachment object on console

[wonderboymusic]

Shouldn't it be:

var type = this.props.get('type');
return ! type || -1 !== attachment.get('mime').indexOf( type );

Can you try that and see if it works?

[wonderboymusic]

The reason being: type can be abbreviated - image/jpeg or just image

[wonderboymusic]

Actually, indexOf is polymorphic, so the original issue is that an array of full mime-types was passed to type, it was looking for an abbreviated type in a list of full types.

[wonderboymusic]

In 32915:

When filtering media by type in wp.media.model.Attachments.filters.type(), account for the library's type being an array of full mime-types.

Fixes #32746.

[wonderboymusic]

In 32916:

Because programming is fun. After [32915], _.find() will return undefined if an empty array is passed - in that case, the function should return true.

See #32746.

[vivekbhusal]

Replying to wonderboymusic:

Shouldn't it be:

var type = this.props.get('type');
return ! type || -1 !== attachment.get('mime').indexOf( type );

Can you try that and see if it works?

It should be type.indexOf(attachment.get("mime");
"type" being the array of all supported mime types and attachment.get("mime") being the mime type of current file.

[vivekbhusal]

Replying to wonderboymusic:

In 32916:

Because programming is fun. After [32915], _.find() will return undefined if an empty array is passed - in that case, the function should return true.

See #32746.

Agree with your patch but there are some fundamental issues.
First thing, we don't need to convert attachment to JSON everytime, its just added overhead,
Second if we are gonna compare our new file type to provided mime type and also it should support abbreviated file type, we don't require mimetype of file.
Third, if above everything the type doesn't exist you have nothing to worry about and can return at the beginning.

Its just added minor condition check, whether the type is array or not. The fundamental logic would be still same. This will work better

           type: function( attachment ) {

                if( ! this.props.get("type") ) {
                    return true;
                }

                var type = this.props.get('type'),
                    attachmentType = attachment.get("type"), found;

                if ( _.isArray( type ) ) {
                    found = _.find( type, function (t) {
                            return -1 !== t.indexOf( attachmentType );
                    } );
                } else {
                    found = -1 !== type.indexOf( attachmentType );
                }

                return found;

            },

[vivekbhusal]

When filtering media attachment by type in wp.media.model.Attachments.filters.type(), checks for type being array or string and validades accordingly

[obenland]

@wonderboymusic could you see if the patch lets you finish this ticket?

[wonderboymusic]

@vivekbhusal some thoughts:

• 32746.diff​ are the changes I think we should make. If you're running the SVN repo, you can run grunt precommit before making your patch and it will alert you to JS coding style changes that need to made
• I think calling .toJSON() is fine - internally, in Backbone, it is just a getter for model.attributes, which is more forward-compatible that directly accessing the property
• The abbreviated mime is supported - type is just the first segment of the mime-type. If your library declares type as an abbreviated version, it will match. The difference we made in [32915] is that the exact mime-type will also match. The type for these MS docs is actually application, which could match several false-positives. I believe the problem before was this: we were trying to match full mime-types against type, which would never return true. Also, because indexOf() is polymorphic (works on arrays and strings), and we weren't checking the type of type, we were getting inconsistent results.

[wonderboymusic]

In 33091:

In wp.media.model.Attachments.filters.type(), return true earlier if type isn't set.

Props vivekbhusal.
Fixes #32746.

[vivekbhusal]

@wonderboymusic: I will check that next time, thanks for that. Supporting both mimetype and then type makes more sense as it reduces false positive.