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

Questions about html escape #43

Closed
dstrelau opened this issue Mar 15, 2014 · 12 comments
Closed

Questions about html escape #43

dstrelau opened this issue Mar 15, 2014 · 12 comments
Labels
bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented

Comments

@dstrelau
Copy link
Contributor

Hi folks. I'm getting unexpected behavior when trying to send formatted links.

I'm attempting to send a response from hubot with a link label, like so:

msg.send "<https://github.com/link/to/a/PR|myrepo #42> fix some broken"

What I'd expect to see in Slack is this, with {{ … }} representing hyperlinked text with the URL I passed.

{{myrepo #42}} fix some broken

Instead, I see this, with literal brackets and the link in the wrong place:

<{{myrepo}} #42> fix some broken

Looking around, it's pretty evident that this is due to the escapeHTML call that occurs before the message is sent. In fact, removing that call gives me the result I'd expect, with the properly parsed link.

I'm trying to understand how https://api.slack.com/docs/formatting works in the context of hubot, but I'm a bit confused. What is the best way to allow for what I want to do without screwing up everyone else's hubot? Should I create msg.sendRaw? Is it actually safe to remove escapeHTML since slack handles most of it server-side?

@odensc
Copy link

odensc commented Sep 15, 2014

👍, having this problem too.

@wyefei
Copy link

wyefei commented Oct 16, 2014

👍 having this problem too! Please fix

@nanonanomachine
Copy link

👍 , but I'm a little confused. In Slack API Message Formatting, we need to escape when we want to use &, <, >. Do Slack API clients have to distinguish following two cases?

  1. Using these characters with hyperlink, user link(<@user>), channel link( <#channel>), special command(<!foo>)
  2. Using these characters without them

It seems there's a way to fix escaping function, but I don't know it is possible.

@dstrelau
Copy link
Contributor Author

Just as a note, we just upgraded our hubot adapter to the new hubot-slack that uses the bot API (awesome btw!!).

Unfortunately, this is still an issue :( I'll see if I can track down what's going on in the new code and post a patch, but I'm still a bit unclear how this is supposed to work. Any input from SlackHQ?

@paulhammond
Copy link
Contributor

Hi! Sorry for the problems, and sorry for not replying here sooner!

As you might have noticed the new hubot adapter is built on an API that does link formatting slightly differently, so sending marked up links is now broken in a slightly different way. I think this is the same bug we're tracking in #114.

To fix this we're going to need to make a few changes on the server so the API can accept sending pre-marked-up links. We're working actively on that now, and hope to have an update soon.

@paulhammond paulhammond added the bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented label Dec 18, 2014
@dstrelau
Copy link
Contributor Author

Thanks for the update @paulhammond! Happy to help if possible, but sounds like we're waiting on server-side support at this point. Let me know if I can help test anything.

@dstrelau
Copy link
Contributor Author

@paulhammond Could you provide an update here? I'd be happy to work on the adapter code here if the API fixes you mentioned have taken place.

@lukerollans
Copy link

Also experiencing this issue! When our hubot sends pull request related notices to Slack, the message should partly appear as repo#pr_number, eg my-repo#35

This means that the msg is sent to Slack partly like this
https://github.com/organization/repo/pull/25|repo#25

However, the pipe character is being url encoded so the entire message appears broken.

Watching this issue closely!

@technicalpickles
Copy link
Contributor

I was able to reproduce this pretty easily. I'm not sure why it's being escaped yet, as there doesn't to seem to be any obvious escaping code in the adapter. It might be in the node-slack, or maybe it's a parameter to the API that needs to be specified to parse it as mrkdwn.

I found a workaround though. It's not obvious, but you can use slack attachments to do this:

robot.emit 'slack.attachment', {text: "<https://github.com/link/to/a/PR|myrepo #42> fix some broken">}

@technicalpickles
Copy link
Contributor

I did find a difference between adapter.send and adapter.customMessage (which is used by slack.attachment) event: send uses channel.send which calls channel.sendMessage which calls client._send, compared to customMessage which calls channel.postMessage which calls the API.

So the difference comes down to adapter.send uses the RTM to send messages back, where attachments use the web API. Over on https://api.slack.com/rtm#sending_messages it says:

The RTM API only supports posting simple messages formatted using our default message formatting mode. It does not support attachments or other message formatting modes. To post a more complex message as a user clients can call the chat.postMessage Web API method with as_user set to true.

So it seems like this should work as is, which makes me think there is a bug in the RTM. I'm not sure if there's any advantage to using RTM for responses, or if we can always just use the web API instead.

@technicalpickles
Copy link
Contributor

I got confirmation from Slack that the RTM API doesn't support link formatting:

Your conclusion is correct, the RTM API currently doesn't support link formatting. Using the chat.postMessage method is what we would recommend in cases where you need more than just bare bones formatting, and you shouldn't notice any downsides to using it instead of the RTM API.

So, sounds like we update to always use the chat.postMessage API.

@UncannyBingo
Copy link

Closing this as fixed in 4.0. Please reopen if I am mistaken!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented
Projects
None yet
Development

No branches or pull requests

8 participants