If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Send raw msgs through the conversation service

RESOLVED FIXED in 1.6

Status

Instantbird
Conversation
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: arlolra, Assigned: arlolra)

Tracking

(Blocks: 1 bug)

Details

Attachments

(1 attachment, 1 obsolete attachment)

1.07 KB, patch
florian
: review+
Details | Diff | Splinter Review
(Assignee)

Description

3 years ago
Created attachment 8503663 [details] [diff] [review]
raw.patch

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_9_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/38.0.2125.101 Safari/537.36
Attachment #8503663 - Attachment is patch: true
Attachment #8503663 - Attachment mime type: text/x-patch → text/plain
Attachment #8503663 - Flags: review+
Is there a particular situation this occurs in? (Where the conversation disappears.)
(Assignee)

Comment 2

3 years ago
If it doesn't go through the conversation service, it bypasses the observers we added for encryption.
(Assignee)

Comment 3

3 years ago
On second thought, maybe it's better to add another notifier in the command service before executing them that the otr extension can observe and modify ... hmmm. That seem like an ok place to solve the /me /msg command issues as well.
(Assignee)

Comment 4

3 years ago
Created attachment 8503691 [details] [diff] [review]
command.patch

Something like this patch.

Pidgin seems to have decided on encrypting "/me ..." as a string and sending a PRIVMSG instead of an action.

https://lists.cypherpunks.ca/pipermail/otr-dev/2013-September/001871.html
https://lists.cypherpunks.ca/pipermail/otr-dev/2013-September/001886.html
https://hg.pidgin.im/pidgin/main/rev/3edc70bf4e09

That's the "command may have changed" in the patch.

Pidgin currently doesn't handle encryption for notify|msg|query. I opened https://developer.pidgin.im/ticket/16397 for them, so I suppose we can't look there to see how to do it.
I find the description is this bug to be lacking. Context is available at [1].

Overall it's to solve how commands should be handled with OTR. /raw can be used on all protocols. IRC has /me, /msg, /notice.

(In reply to arlolra from comment #4)
> Pidgin seems to have decided on encrypting "/me ..." as a string and sending
> a PRIVMSG instead of an action.

This is a *bad* idea. Encrypting the full CTCP string should be done so you can encrypt any arbitrary CTCP command (e.g. DCC connects). This also makes it wayyyy Pidgin specific (since they parse "/me " at the front of messages to determine whether it's an action or not). I dislike this decision.

[1] http://log.bezut.info/instantbird/141011/#m170
Status: UNCONFIRMED → NEW
Ever confirmed: true
Blocks: 954310
So we talked about this briefly on IRC, but wanted to put it in the bug too. I think both Florian and I find this bug pretty confusing. The /raw/ patch seems reasonable, can we split the other commands to separate bugs?
(Assignee)

Comment 7

3 years ago
Sure, it's already r+'d so feel free to merge.
Attachment #8503691 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/332650fa4e8d
Assignee: nobody → arlolra
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 1.6
You need to log in before you can comment on or make changes to this bug.