Escaped help messages 'broken' on non-html chats #343

Closed
OleMchls opened this Issue Aug 27, 2012 · 10 comments

Comments

Projects
None yet
2 participants
@OleMchls
Contributor

OleMchls commented Aug 27, 2012

Hey,

As said in Topic, the "new" escaped help messages will break in any evniroment which is non-html based, like jabber or shell adapter.

hubot who is <user> - see what roles a user has

This is what you'll get on the shell adapter using hubot help. At the moment the escaping is done in the help command itself. So my guess is that it would make more sense to do any sanitizing in the adapter itself. But I'm not sure if this might have side effects to other features.'

Love,

@technicalpickles

This comment has been minimized.

Show comment
Hide comment
@technicalpickles

technicalpickles Aug 27, 2012

Contributor

I just ran into this today too actually. To work around, I removed the escaping, which appears to work both for shell & campfire. I can commit that fix to hubot proper unless there's a reason not to.

On Aug 27, 2012, at 5:00 PM, Ole Michaelis wrote:

Hey,

As said in Topic, the "new" escaped help messages will break in any evniroment which is non-html based, like jabber or shell adapter.

hubot who is <user> - see what roles a user has
This is what you'll get on the shell adapter using hubot help. At the moment the escaping is done in the help command itself. So my guess is that it would make more sense to do any sanitizing in the adapter itself. But I'm not sure if this might have side effects to other features.'

Love,


Reply to this email directly or view it on GitHub.

Contributor

technicalpickles commented Aug 27, 2012

I just ran into this today too actually. To work around, I removed the escaping, which appears to work both for shell & campfire. I can commit that fix to hubot proper unless there's a reason not to.

On Aug 27, 2012, at 5:00 PM, Ole Michaelis wrote:

Hey,

As said in Topic, the "new" escaped help messages will break in any evniroment which is non-html based, like jabber or shell adapter.

hubot who is <user> - see what roles a user has
This is what you'll get on the shell adapter using hubot help. At the moment the escaping is done in the help command itself. So my guess is that it would make more sense to do any sanitizing in the adapter itself. But I'm not sure if this might have side effects to other features.'

Love,


Reply to this email directly or view it on GitHub.

@OleMchls

This comment has been minimized.

Show comment
Hide comment
@OleMchls

OleMchls Aug 28, 2012

Contributor

Yeah but I would prefer a more general fix, b/c I guess almost all users who are using a non-html adapter will get this issue.

Contributor

OleMchls commented Aug 28, 2012

Yeah but I would prefer a more general fix, b/c I guess almost all users who are using a non-html adapter will get this issue.

@OleMchls

This comment has been minimized.

Show comment
Hide comment
@OleMchls

OleMchls Aug 28, 2012

Contributor

fixed with #345

Contributor

OleMchls commented Aug 28, 2012

fixed with #345

@technicalpickles

This comment has been minimized.

Show comment
Hide comment
@technicalpickles

technicalpickles Aug 28, 2012

Contributor

Your original idea of having adapters escape or not is probably a more general fix, rather than having it in help.coffee. The biggest downside of it being there is that the only time you definitely get this updated file is starting a new hubot, or if you know enough to get updates from hubot.

Contributor

technicalpickles commented Aug 28, 2012

Your original idea of having adapters escape or not is probably a more general fix, rather than having it in help.coffee. The biggest downside of it being there is that the only time you definitely get this updated file is starting a new hubot, or if you know enough to get updates from hubot.

@technicalpickles

This comment has been minimized.

Show comment
Hide comment
@technicalpickles

technicalpickles Aug 28, 2012

Contributor

Also, I may not have been clear, but campfire (ie a html based campfire) seems to be okay with unescaped output. I think it's because help does a paste, which is wrapped in <pre>

Contributor

technicalpickles commented Aug 28, 2012

Also, I may not have been clear, but campfire (ie a html based campfire) seems to be okay with unescaped output. I think it's because help does a paste, which is wrapped in <pre>

@OleMchls

This comment has been minimized.

Show comment
Hide comment
@OleMchls

OleMchls Aug 28, 2012

Contributor

So what was the reason to add this escaping.
Related question, why is it escaped this way DIY and not with a sanitizer?

Maybe we can just remove the escaping? It seems that it causes more confusion then it helps, or am I missing a big point?

Contributor

OleMchls commented Aug 28, 2012

So what was the reason to add this escaping.
Related question, why is it escaped this way DIY and not with a sanitizer?

Maybe we can just remove the escaping? It seems that it causes more confusion then it helps, or am I missing a big point?

@technicalpickles

This comment has been minimized.

Show comment
Hide comment
@technicalpickles

technicalpickles Sep 5, 2012

Contributor

Based on dc8ad3c, it looks like it was mistakenly added to the help command, rather than the HTTP endpoint.

Fixed this, but you'll need to copy the local copy down to get the update.

Contributor

technicalpickles commented Sep 5, 2012

Based on dc8ad3c, it looks like it was mistakenly added to the help command, rather than the HTTP endpoint.

Fixed this, but you'll need to copy the local copy down to get the update.

@OleMchls

This comment has been minimized.

Show comment
Hide comment
@OleMchls

OleMchls Sep 5, 2012

Contributor

👍

Contributor

OleMchls commented Sep 5, 2012

👍

@OleMchls

This comment has been minimized.

Show comment
Hide comment
@OleMchls

OleMchls Sep 5, 2012

Contributor

But why does an "create [--create]" hubot not use the scripts from node_modules/hubot/src/scripts/* path?

Contributor

OleMchls commented Sep 5, 2012

But why does an "create [--create]" hubot not use the scripts from node_modules/hubot/src/scripts/* path?

@technicalpickles

This comment has been minimized.

Show comment
Hide comment
@technicalpickles

technicalpickles Sep 5, 2012

Contributor

Not sure, separate issue though.

On Sep 5, 2012, at 10:04 AM, Ole Michaelis notifications@github.com wrote:

But why does an "create [--create]" hubot not use the scripts from node_modules/hubot/src/scripts/* path?


Reply to this email directly or view it on GitHub.

Contributor

technicalpickles commented Sep 5, 2012

Not sure, separate issue though.

On Sep 5, 2012, at 10:04 AM, Ole Michaelis notifications@github.com wrote:

But why does an "create [--create]" hubot not use the scripts from node_modules/hubot/src/scripts/* path?


Reply to this email directly or view it on GitHub.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment