Skip to content

handle guzzle5 exceptions #78

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

Closed
wants to merge 1 commit into from
Closed

handle guzzle5 exceptions #78

wants to merge 1 commit into from

Conversation

younes0
Copy link
Contributor

@younes0 younes0 commented Oct 20, 2015

This will prevent Guzzle to fetch data-based image urls:

  [InvalidArgumentException]
  Unable to parse malformed url: data:///image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==

This will prevent Guzzle to fetch data-based image urls:
```shell
  [InvalidArgumentException]
  Unable to parse malformed url: data:///image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==
```
@oscarotero
Copy link
Collaborator

Hi.
I'm not sure about this. Embedded images still being images, so they should be taken into account, like any other url. In fact, I just made a commit to fix this. You can check it here: https://github.com/oscarotero/Embed/blob/master/src/ImageInfo/Curl.php#L173
If the image is just downloaded (because it's embedded in the html), why not use it?

@younes0
Copy link
Contributor Author

younes0 commented Oct 21, 2015

Hello! You're right about the embedded images, will check your commit today and fix my PR

@younes0
Copy link
Contributor Author

younes0 commented Oct 21, 2015

toi avoid duplication, maybe should we place getEmbeddedImageInfo() in a ImageInfoTrait.php or elsewhere (and add getEmbeddedImageInfo() to ImageInfoInterface) ?

@oscarotero
Copy link
Collaborator

Yes, you're right. I just moved the function to a Trait, so you can reuse it in the Guzzle5 implementation.
I don't think this should be added to ImageInfoInterface, because it's internal stuff, used by getImagesInfo method.

@younes0
Copy link
Contributor Author

younes0 commented Oct 21, 2015

see #79

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 this pull request may close these issues.

2 participants