-
-
Notifications
You must be signed in to change notification settings - Fork 10.4k
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
https image urls if accessed over SSL #8373
Conversation
Tentative fix for #8372
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your PR!
Could you please add a test case to proof your case is not working?
Probably in core/test/unit/utils/url_spec.js
.
tests whether utils.url.urlFor returns https urls for images over https when blog is configured with a http baseUrl fixes #8372
Our blog had optional https but http by default, so it was configured with an http base url. The https version of the rss feed was returning http links for images as the current code only checks the image path for https, ignoring that the actual connection may be https. This PR fixes it. |
core/server/utils/url.js
Outdated
@@ -259,7 +259,7 @@ function urlFor(context, data, absolute) { | |||
urlPath = data.image; | |||
imagePathRe = new RegExp('^' + getSubdir() + '/' + STATIC_IMAGE_URL_PREFIX); | |||
absolute = imagePathRe.test(data.image) ? absolute : false; | |||
secure = data.image.secure; | |||
secure = data.image.secure || secure; |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Thank you!
closes TryGhost#8372 - https image urls if accessed over SSL (fix secure option for images)
Tentative fix for #8372. I'm not familiar with this codebase so I don't know the rationale behind the current code nor whether this change would be enough, but hopefully will point to the right direction.