Relative protocol avatar URLs (Gravatar) cause issues with Slack webhooks.

Erisa commented 5 years ago

Issue Summary

If you have an author with an unset avatar that is handled by Gravatar, the sent webhook has no protocol for the author avatar URL, instead using //www.gravatar.com.

Looking into it, this appears to be a deliberate choice to use "relative protocol" links for Gravatar URLs. Slack handles this without error however the authors avatar is never displayed in the webhooks attachment. On Discord, this errors with 400 Bad Request and the webhook is never sent.

To Reproduce

For Slack:

Create a Slack webhook and set it up with Ghost.
Create and publish a new post with an author who has a Gravar (unset) avatar.
Notice the avatar isn't visible in Slack:

For Discord:

Create a Discord webhook, add /slack to it and set it up as a Slack webhook in Ghost.
Create and publish a new post with an author who has a Gravatar (unset) avatar
Notice that the webhook didn't send and observe the 400 error in ghost log.

Here's the error I get doing it for Discord, with the webhook token redacted:

[2019-09-09 08:25:51] INFO Trigger Webhook for  "post.added" with url "https://discordapp.com/api/webhooks/620081411432513546/WEBHOOK-TOKEN-REDACTED".
[2019-09-09 08:25:51] INFO "POST /ghost/api/canary/admin/posts/" 201 97ms
[2019-09-09 08:25:51] WARN Request to https://discordapp.com/api/webhooks/620081411432513546/WEBHOOK-TOKEN-REDACTEDfailed because of: .
[2019-09-09 08:25:55] INFO "PUT /ghost/api/canary/admin/posts/5d75fe7fba0017629b20e7e5/" 200 124ms
[2019-09-09 08:25:55] ERROR

NAME: InternalServerError
MESSAGE: The server has encountered an error.

level: normal

"The slack service was unable to send a ping request, your site will continue to function."
"If you get this error repeatedly, please seek help on https://ghost.org/docs/."
InternalServerError: The server has encountered an error.
    at new GhostError (/var/www/ghost/versions/2.30.2/core/server/lib/common/errors.js:10:26)
    at /var/www/ghost/versions/2.30.2/core/server/services/slack.js:117:34
    at tryCatcher (/var/www/ghost/versions/2.30.2/node_modules/bluebird/js/release/util.js:16:23)
    at Promise._settlePromiseFromHandler (/var/www/ghost/versions/2.30.2/node_modules/bluebird/js/release/promise.js:517:31)
    at Promise._settlePromise (/var/www/ghost/versions/2.30.2/node_modules/bluebird/js/release/promise.js:574:18)
    at Promise._settlePromise0 (/var/www/ghost/versions/2.30.2/node_modules/bluebird/js/release/promise.js:619:10)
    at Promise._settlePromises (/var/www/ghost/versions/2.30.2/node_modules/bluebird/js/release/promise.js:695:18)
    at _drainQueueStep (/var/www/ghost/versions/2.30.2/node_modules/bluebird/js/release/async.js:138:12)
    at _drainQueue (/var/www/ghost/versions/2.30.2/node_modules/bluebird/js/release/async.js:131:9)
    at Async._drainQueues (/var/www/ghost/versions/2.30.2/node_modules/bluebird/js/release/async.js:147:5)
    at Immediate.Async.drainQueues [as _onImmediate] (/var/www/ghost/versions/2.30.2/node_modules/bluebird/js/release/async.js:17:14)
    at runCallback (timers.js:705:18)
    at tryOnImmediate (timers.js:676:5)
    at processImmediate (timers.js:658:5)
    at process.topLevelDomainCallback (domain.js:126:23)

HTTPError: Response code 400 (BAD REQUEST)
    at EventEmitter.emitter.on (/var/www/ghost/versions/2.30.2/node_modules/got/source/as-promise.js:74:19)

It's interesting that despite the webhook url having /slack appended, the warn in that response doesn't show it. Doesn't appear related though.

The raw JSON it was trying to send:

{"text":"Notification from *Erisa\'s Corner of the Internet* :ghost:","unfurl_links":true,"icon_url":"https://erisa.dev/favicon.ico","username":"Ghost","attachments":[{"fallback":"Sorry, content cannot be shown.","title":"(Untitled)","title_link":"https://erisa.dev/untitled-8/","author_name":"Erisa\'s Corner of the Internet","image_url":"https://erisa.dev/","color":"#008952","fields":[{"title":"Description","value":"a.","short":false}]},{"fallback":"Sorry, content cannot be shown.","color":"#008952","thumb_url":"//www.gravatar.com/avatar/a78a3feecffd588d2d7382257de807bc?s=250&d=mm&r=x","fields":[{"title":"Author","value":"<https://erisa.dev/404/ | Erisa>","short":true}],"footer":"Erisa\'s Corner of the Internet","footer_icon":"https://erisa.dev/favicon.ico","ts":1568013955}]}

Technical details:

Ghost Version: 2.30.2
Node Version: v10.16.3
Browser/OS: Firefox Dev Edition 70.0b3 / Ghost hosted on Ubuntu 18.04.3 LTS
Database: MySQL Ver 14.14 Distrib 5.7.27

naz commented 5 years ago

@Erisa hey :wave: Can confirm that this is a bug but a very minor one :) Don't think it would make sense to change how we serve gravatar links across the system just for this case, but think it would be a plausible workaround to handle them locally in slack integration service and transform protocol-less gravatar urls to https ones.

@kevinansfield maybe you have an opinion on this since you've done the most with URL handling?

kevinansfield commented 5 years ago

@gargol protocol-relative URLs can be a bit of a pain because they are technically "invalid" when parsing without additional context and can fail when used in non-browser contexts such as webhooks.

I'd be in favour of using https for gravatar URLs everywhere, there's no downside as far as I'm aware 🤔 We originally switched to protocol-relative because using http resulted in mixed-content warnings but https is likely a better default.

naz commented 5 years ago

Awesome. Switching to https would be a perfect solution then :+1:

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

TryGhost / Ghost