Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

jpg mime type on IE6 #1

Closed
tomasz-tomczyk opened this Issue · 12 comments

3 participants

@tomasz-tomczyk

Hey, awesome bundle!

Turns out, IE6 uploads .jpg files with mime type of 'image/pjpeg' (notice the p), so resizer.php line 173 switch() will never return true, as File::mime() function from Laravel returns the mime type as 'image/jpeg' for jpg files. Now this could be their mistake, because it should return all mime types allowed for that extension or intended - at the moment anyway it returns the first item of the array (even though default config file includes both mime types).

As a temporary fix, adding case 'image/pjpeg': above the case File::mime('jpg'): does the trick, I'll see what Laravel says about this :)

Thanks

@maikeldaloo
Owner

Hello Tomasz,

Thanks for letting me know, I'll make that change, it's a small and easy fix :)

@tomasz-tomczyk

Thanks for responding so quickly!

Let's keep this issue open, while my fix works, it's not very clean - I'll discuss it with laravel devs, if it was up to me, output of File::mime() would provide you with all the possibilities, or there should be a helper function File::is_mime($input, 'jpg') to compare with the possible mimes of a file type.

@maikeldaloo maikeldaloo reopened this
@maikeldaloo
Owner

Oops, I accidentally closed the issue, I re-opened it.

BTW.. it actually does return an array..

In application/config/mimes.php, lines 65-67, you'll find:

'jpeg'  => array('image/jpeg', 'image/pjpeg'),
'jpg'   => array('image/jpeg', 'image/pjpeg'),
'jpe'   => array('image/jpeg', 'image/pjpeg'),

But in laravel/file.php, starting at line154, you'll find

public static function mime($extension, $default = 'application/octet-stream')
{
    $mimes = Config::get('mimes');
    if ( ! array_key_exists($extension, $mimes)) return $default;
    return (is_array($mimes[$extension])) ? $mimes[$extension][0] : $mimes[$extension];
}

And it looks like it returns the first element of the array, meaning the 'image/pjpeg' is skipped.

@maikeldaloo maikeldaloo was assigned
@tomasz-tomczyk

Oh yeah I absolutely get that - it is set up with array but never actually looks past the first element - which makes me think it's an oversight from Laravel devs :)

@maikeldaloo
Owner

Yeah, I'll forward this URL to Taylor, see what he says about it :)
Thanks for mentioning it.

@daylerees

Hmm, what about an alternate File::mimes() which would bring back the array, rather than a string?

@maikeldaloo
Owner

That would be good :)

@daylerees

My thoughts being.. forcing users to use is_array() in their own code is normally the kinda thing Taylor keeps away from, so it might be best to keep the mime method returning a string. Mimes could use array_merge() or whatever other neat trick to force return of an array. I dunno, I'm sure Taylor could pimp this.

@maikeldaloo
Owner

Wouldn't the following do the trick?

public static function mimes($extension)
{
    $mimes = Config::get('mimes');

    // extension doesn't exist in mimes array.
    if ( ! array_key_exists($extension, $mimes)) return null;

    return $mimes[$extension];
}
@daylerees

The trick is to have the method return a single type, so the user always knows what they are getting, so in this case :

public static function mimes($extension, $default = array('default/mime'))
{
    $mimes = Config::get('mimes');

    // extension doesn't exist in mimes array.
    if ( ! array_key_exists($extension, $mimes)) return $default;

    return array_merge(array(), $mimes[$extension]);
}

Now the user will always get an array back.

@maikeldaloo
Owner

Ahh ok, I see. Forcing an array to be returned always.
Very good.

@tomasz-tomczyk

daylerees: I like that a lot - this would be a good addition I think. Having said that, I'm not sure what's the logic behind returning the first item of the array for File::mime() in the first place? How is audio/mpeg better than audio/mp3 for mp3 extension?

@maikeldaloo maikeldaloo closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.