zestedesavoir / zmarkdown

Live demo: https://zestedesavoir.github.io/zmarkdown/
MIT License
226 stars 52 forks source link

[remark-images-download] Do not throw error when HTTP status code is 301 #437

Closed Situphen closed 2 years ago

StaloneLab commented 3 years ago

Thanks for this PR, unfortunately I do not think it can be merged as is. When a 301 or 302 code is returned to the Node http agent, it doesn't follow the redirection. Hence, with your changes, no error is thrown but the image inserted in the resulting content is invalid (with an empty file downloaded I think). In fact, it seems the default http module doesn't support following redirects by default, so if you want to fix it, you'll likely need to:

Hope I'm not forgetting anything, you'll likely encounter problems I haven't even thought of anyway. Feel free if you have any questions, you're not restricted to empty messages :upside_down_face: .

artragis commented 3 years ago

Warning, any "follow-redirect" implementation MUST define a max-hop attribute to avoid redirection bombing. For us it should be set to 1 as we mainly want to follow http -> https.

StaloneLab commented 3 years ago

Yes, I think what you are referring to was what I meant by "potential infinite redirects problem" ;)

Situphen commented 3 years ago

I first thought this would be an easy fix but it clearly isn't. I will see if I can work to fix this or if it's easier to fix the content themselves.

StaloneLab commented 2 years ago

Closing for inactivity. Do not hesitate to reopen either this PR or a new issue for the problem.