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

Amperize issues #9022

Closed
4 tasks done
aileen opened this issue Sep 19, 2017 · 1 comment · Fixed by #9367
Closed
4 tasks done

Amperize issues #9022

aileen opened this issue Sep 19, 2017 · 1 comment · Fixed by #9367
Assignees
Labels
bug [triage] something behaving unexpectedly server / core Issues relating to the server or core of Ghost

Comments

@aileen
Copy link
Member

aileen commented Sep 19, 2017

Issue Summary

We detected three major problems within the Amperize repository:

  1. Images without extension
    Images like this https://www.zomato.com/logo/18163505/minilogo work, but don't have an extension and will therefore cause an error in the underlying image-size library.

  2. One image fails all
    If one image fails to load, none are loaded. This is simple to test by adding an image (open amp page) then add another image with a made up name and view the amp page again. Both images are missing.

  3. Redirects are not followed
    Redirected image URLs are not followed. Within Ghost (where we use a similar logic to fetch image dimension), we fixed this by changing to got for our requests. See 🙈 Used got to handle requests for image-size #8953 and the follow up PR ✨ Request util to wrap got library  #8980.

Todos/Sub-issues

Note: I created a new repository which is fixed the above mentioned issues. It is possible to either port those changes to the Amperize repository, my fork of it, or using the new repository. All dependent on how the repository owner plans to keep maintaining it or not.

@aileen aileen self-assigned this Sep 19, 2017
@aileen aileen added bug [triage] something behaving unexpectedly and removed optimisation labels Sep 19, 2017
@kirrg001 kirrg001 added the server / core Issues relating to the server or core of Ghost label Sep 19, 2017
@kirrg001
Copy link
Contributor

kirrg001 commented Jan 2, 2018

@AileenCGN

Images without extension: jbhannah/amperize#97

Is that a hard thing to fix? Otherwise, assign a PR to me in amperize and then we can close this issue 👻

aileen added a commit to aileen/Ghost that referenced this issue Jan 4, 2018
closes TryGhost#9022

Images without extensions don't need to be manipulated, as we're now reading the bytes and pass those to the `image-size` lib.

This PR adds another `user-agent` to emulate multiple browser requests, as I stumbled over an example where the image without extension is protected otherwise.

Added a test, that works with above mentioned image, but is currently mocked. Nevertheless, the image worked as a PoC, that we're able to read the bytes of an image without its extension and still return the dimensions of the image.
kirrg001 pushed a commit that referenced this issue Jan 4, 2018
closes #9022

Images without extensions don't need to be manipulated, as we're now reading the bytes and pass those to the `image-size` lib.

This PR adds another `user-agent` to emulate multiple browser requests, as I stumbled over an example where the image without extension is protected otherwise.

Added a test, that works with above mentioned image, but is currently mocked. Nevertheless, the image worked as a PoC, that we're able to read the bytes of an image without its extension and still return the dimensions of the image.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug [triage] something behaving unexpectedly server / core Issues relating to the server or core of Ghost
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants