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

Error with using $url addMedia #87

Closed
adamroyle opened this issue Feb 20, 2017 · 6 comments · Fixed by #96
Closed

Error with using $url addMedia #87

adamroyle opened this issue Feb 20, 2017 · 6 comments · Fixed by #96

Comments

@adamroyle
Copy link

This change ea119a3 has caused issues on php 5.6 in my app. I get the following error:

Warning (2): mime_content_type(): Failed identify data 0:no magic files loaded in ...

I'm guessing because I'm passing a url instead of a file location, and mime_content_type() doesn't automatically download the file?

@claytoncollie
Copy link

for a quick fix change line 218 to $mimetype = 'image/jpeg'; if you are adding a jpg of course.

@devnix
Copy link

devnix commented Mar 8, 2017

@adamroyle
Copy link
Author

The problem is not in detecting the mime type - the problem is the regression in functionality. The previous code would detect the mime type from a base64'd value of the data, whereas the new code uses mime_content_type() of the URL, which doesn't work because it's a url and not a file location.

I'm not sure why this code was changed, but I see the following options:

  1. Revert this change OR
  2. Save the file contents to a temporary file before using mime_content_type()

@ApeWare
Copy link

ApeWare commented Apr 4, 2017

Doing some updates and ran into this exact issue. We store our images on s3 and pass the url to addPhoto($url) and now we are getting the mime_content_type() error.

Is there a reason for this breaking change other than to reduce the library by 3 lines of code? Debating on forking and rolling back this change, but would prefer to understand why the change was important.

@ifunk
Copy link

ifunk commented Apr 4, 2017

Instead of forking we have just fixed our version at 1.2.2. At this time there's no need for us to upgrade to a newer version as we've been running it in production for over a year and have had no issues.

@axelitus
Copy link

axelitus commented Apr 15, 2017

I have the same issue. mime_content_type() does not work with urls, just local filenames. If the media contents has been already downloaded why not use that to verify the mime type and do another request just to figure it out? I don't understand this change...

Reverting this change works fine. Hopefully @jeroendesloovere shares his thoughts on this.

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

Successfully merging a pull request may close this issue.

6 participants