Skip to content

Conversation

@natocTo
Copy link

@natocTo natocTo commented Dec 26, 2017

It is necessary to have $ at the end of reqex (I do not understand reqex to much)? Images with src like "/images/for.png?1716226" (some kind of cache busting) are not recognized now.
Also I suggest add webp because is quite popular nowadays.

It is necessary to have $ at the end of reqex? Images with src like "/images/for.png?1716226" (some kind of cache busting) are not recognized now.
Also I suggest add webp because is quite popular nowadays.
@natocTo
Copy link
Author

natocTo commented Dec 27, 2017

Today I see I can override this setting. It is clever. But maybe this PR is still good to look.

@Tjatse
Copy link
Owner

Tjatse commented Jan 3, 2018

This sounds reasonable, especially for the webpack things (vue.js, react...) or some configured nginx (/images/for.png?1716226 style is very common now.)

/\.(gif|jpe?g|png|webp)/i is okay but not safe, some url , e.g. /path/to/a.gif.cropper/1202 will be considered as an image.

/\.(gif|jpe?g|png|webp)(\?[^?]+)?$/i is better, test to pass:
/path/to/some-image.png
/path/to/some-image.jpg
/path/to/some-image.png?1231312
/path/to/some-image.webp?a=c&d=1

I'll commit this after some necessary tests with my spiders, and release it ASAP!

Thanks a lot.

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