Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Image regex is case sensitive #9

Closed
walter opened this issue Aug 17, 2016 · 5 comments
Closed

Image regex is case sensitive #9

walter opened this issue Aug 17, 2016 · 5 comments

Comments

@walter
Copy link

walter commented Aug 17, 2016

If I'm not mistaken https://github.com/smeevil/cloudex/blob/master/lib/cloudex.ex#L6 will not match JPG, GIF, etc.

I'm also curious what the reasoning is for checking that the file has an extension. I'm going to have workaround this in when using Phoenix upload mechanism where the temp file doesn't have an extension.

@smeevil
Copy link
Owner

smeevil commented Aug 17, 2016

Thank you for bringing the case insensitivity to the light. I've updated it to be case insensitive now.

Checking the file extension was more to make sure we are uploading something useful to Cloudinary, this should be replaced with a mime type check to make it less fickle and to remove the dependency of an extension. @manukall maybe you have some input as well about this ?

@manukall
Copy link
Contributor

to be honest i'm directly using the functions in Cloudex.CloudinaryApi.Live, mainly because of this check. imho it could completely go and maybe be offered as an opt-in helper function, like image_list |> Cloudex.filter_image_urls |> Cloudex.upload.

@walter
Copy link
Author

walter commented Aug 18, 2016

I'm also of the opinion that the check should be an optional helper. Feels like Phoenix uploads are a common case. The submitted filename is available in the Plug.Upload struct and that is would be the easier thing to check against rather than path.

Alternatively Cloudex.upload could provide an option to skip the check.

@smeevil
Copy link
Owner

smeevil commented Aug 19, 2016

I've updated the hex and removed both the file and url extension checks.
Thank you all for your input on this 👍

@smeevil smeevil closed this as completed Aug 19, 2016
@walter
Copy link
Author

walter commented Aug 21, 2016

Thanks @smeevil!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants