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

Follow redirects #789

Merged
merged 8 commits into from
Nov 26, 2019
Merged

Follow redirects #789

merged 8 commits into from
Nov 26, 2019

Conversation

SaWey
Copy link
Contributor

@SaWey SaWey commented Sep 10, 2019

Fixes #775

What's Changing and Why

Automatically follow http redirects instead of erroring out.

What else might be affected

Tasks

  • Add tests
  • Update Documentation
  • Update jimp.d.ts
  • Add SemVer Label

Published PR with canary version: 0.8.1-canary.789.460.0

@hipstersmoothie
Copy link
Collaborator

@SaWey please fix lint and add a test if you can

@hipstersmoothie hipstersmoothie added minor Increment the minor version when merged patch Increment the patch version when merged and removed minor Increment the minor version when merged labels Sep 12, 2019
Fixing lint error
Fix tests
@FridayJew
Copy link

I installed version with this changing and that solve my problem.
Byffer error happen only if I try to read a file that is only partially downloaded.

I think it's good results.

@hipstersmoothie
Copy link
Collaborator

I'd still like a test so we don't break it in the future. Good to know that it solved the problem though!

@SaWey
Copy link
Contributor Author

SaWey commented Sep 16, 2019

I can't seem to get the test environment setup on my windows machine.
Keep bothering mee about not finding '@jimp/test-utils' module when running the tests.

How would you see this test being setup? Start a custom server to send a 301 response when loading an image?
Because if we were to use an internet link, we're not sure it's going to stay online.

@andresmmujica
Copy link

Hi, I'm having an issue that probably will get fixed with this. My url redirects to the actual image but Jimp/phin is reading as XML so it fails. If I test with curl url it shows the xml if I test curl -L url it works like a charm, so please if this gets into Jimp would be awesome

@SaWey
Copy link
Contributor Author

SaWey commented Sep 20, 2019

I was able to add a test with a local server to mock a 301 redirect.

@SaWey
Copy link
Contributor Author

SaWey commented Sep 20, 2019

@hipstersmoothie any idea how I should tackle this testcase?
I don't know how to differentiate between browser and node running the test.

@SaWey
Copy link
Contributor Author

SaWey commented Oct 10, 2019

@hipstersmoothie
Can this patch be applied?

@hipstersmoothie hipstersmoothie merged commit c3835b2 into jimp-dev:master Nov 26, 2019
@hipstersmoothie
Copy link
Collaborator

🚀 PR was released in v0.9.3 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch Increment the patch version when merged released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error: Could not find MIME for Buffer <null>
4 participants