-
Notifications
You must be signed in to change notification settings - Fork 171
Add support for XEP-0461: Message Replies #226
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,7 +30,6 @@ import ( | |
| "fmt" | ||
| "hash" | ||
| "io" | ||
| "log" | ||
| "math/big" | ||
| "net" | ||
| "net/http" | ||
|
|
@@ -95,11 +94,7 @@ func getCookie() Cookie { | |
| func getUUID() string { | ||
| // Use github.com/google/uuid as XEP-0359 requires an UUID according to | ||
| // RFC 4122. | ||
| id, err := uuid.NewV7() | ||
| if err != nil { | ||
| log.Fatal(err) | ||
| } | ||
| return id.String() | ||
| return uuid.Must(uuid.NewV7()).String() | ||
| } | ||
|
|
||
| // Fast holds the XEP-0484 fast token, mechanism and expiry date | ||
|
|
@@ -1451,6 +1446,7 @@ func (c *Client) IsEncrypted() bool { | |
|
|
||
| // Chat is an incoming or outgoing XMPP chat message. | ||
| type Chat struct { | ||
| ID string // if unset, will be generated on send | ||
| Remote string | ||
| Type string | ||
| Text string | ||
|
|
@@ -1462,10 +1458,11 @@ type Chat struct { | |
| Ooburl string | ||
| Oobdesc string | ||
| Lang string | ||
| // Only for incoming messages, ID for outgoing messages will be generated. | ||
| OriginID string | ||
| // Only for incoming messages, ID for outgoing messages will be generated. | ||
| StanzaID StanzaID | ||
| // XEP-0359 | ||
| StanzaID StanzaID // only for incoming messages | ||
| OriginID string // if unset, will be generated on send | ||
| // XEP-0461 | ||
| Reply *Reply | ||
| Roster Roster | ||
| Other []string | ||
| OtherElem []XMLElement | ||
|
|
@@ -1550,6 +1547,7 @@ func (c *Client) Recv() (stanza interface{}, err error) { | |
| v.Delay.Stamp, | ||
| ) | ||
| chat := Chat{ | ||
| ID: v.ID, | ||
| Remote: v.From, | ||
| Type: v.Type, | ||
| Text: v.Body, | ||
|
|
@@ -1561,6 +1559,7 @@ func (c *Client) Recv() (stanza interface{}, err error) { | |
| Lang: v.Lang, | ||
| OriginID: v.OriginID.ID, | ||
| StanzaID: v.StanzaID, | ||
| Reply: v.Reply, | ||
| Oob: v.Oob, | ||
| } | ||
| return chat, nil | ||
|
|
@@ -1852,12 +1851,28 @@ func (c *Client) Send(chat Chat) (n int, err error) { | |
| oobtext += `</x>` | ||
| } | ||
|
|
||
| var replytext string | ||
| if chat.Reply != nil { | ||
| replytext = `<reply id='` + xmlEscape(chat.Reply.ID) + `'` | ||
| if chat.Reply.To != `` { | ||
| replytext += ` to='` + xmlEscape(chat.Reply.To) + `'` | ||
| } | ||
| replytext += ` xmlns='urn:xmpp:reply:0'/>` | ||
| } | ||
|
|
||
| chat.Text = validUTF8(chat.Text) | ||
| id := getUUID() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a reason why you removed this part where go-xmpp generates a new ID automatically and ignores any previously set ID on the message? I think both are reasonable behavior, but if you have an argument for this, it would probably be better in a separate PR (personal opinion).
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is that because in order to handle replies properly the client should keep track of sent origin-ids? And since the
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, exactly. The client needs an ID that is referenced later on reply, when it turns into ...it would be nice to just use the UUID that go-xmpp generates, but yeah it's not already returned in Send() so it felt out of reach. This seemed like the cleanest way. open to suggestions! |
||
|
|
||
| if chat.OriginID == "" { | ||
| chat.OriginID = getUUID() | ||
| } | ||
| if chat.ID == "" { | ||
| chat.ID = chat.OriginID | ||
| } | ||
|
|
||
| stanza := fmt.Sprintf("<message to='%s' type='%s' id='%s' xml:lang='en'>%s<body>%s</body>"+ | ||
| "<origin-id xmlns='%s' id='%s'/>%s%s</message>\n", | ||
| xmlEscape(chat.Remote), xmlEscape(chat.Type), id, subtext, xmlEscape(chat.Text), | ||
| XMPPNS_SID_0, id, oobtext, thdtext) | ||
| "%s<origin-id xmlns='%s' id='%s'/>%s%s</message>\n", | ||
| xmlEscape(chat.Remote), xmlEscape(chat.Type), chat.ID, subtext, xmlEscape(chat.Text), | ||
| replytext, XMPPNS_SID_0, chat.OriginID, oobtext, thdtext) | ||
| if c.LimitMaxBytes != 0 && len(stanza) > c.LimitMaxBytes { | ||
| return 0, fmt.Errorf("stanza size (%v bytes) exceeds server limit (%v bytes)", | ||
| len(stanza), c.LimitMaxBytes) | ||
|
|
@@ -2166,6 +2181,13 @@ type StanzaID struct { | |
| By string `xml:"by,attr"` | ||
| } | ||
|
|
||
| // XEP-0461 Message Replies | ||
| type Reply struct { | ||
| XMLName xml.Name `xml:"urn:xmpp:reply:0 reply"` | ||
| ID string `xml:"id,attr"` | ||
| To string `xml:"to,attr"` | ||
| } | ||
|
|
||
| // RFC 3921 B.1 jabber:client | ||
| type clientMessage struct { | ||
| XMLName xml.Name `xml:"jabber:client message"` | ||
|
|
@@ -2184,6 +2206,9 @@ type clientMessage struct { | |
| OriginID originID `xml:"origin-id"` | ||
| StanzaID StanzaID `xml:"stanza-id"` | ||
|
|
||
| // XEP-0461 | ||
| Reply *Reply `xml:"reply"` | ||
|
|
||
| // Pubsub | ||
| Event clientPubsubEvent `xml:"event"` | ||
|
|
||
|
|
||
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.
The spec says there
SHOULDbe atofield. I think we don't have a reason to escape this and we should produce an error whenidortois an empty string. Do you agree?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.
oh yeah, nice! Makes sense to me, will fix
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.
The incoming side has to allow for missing
tos because SHOULD means optional; I don't think it hurts anything to be symmetric on the sending side.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.
It doesn't hurt. But a SHOULD has a clear meaning in RFCs which means unless you have a very strong reason not to do it, you have to do it (a MUST accepts no exception). It's not a MAY so my opinion is we should not make it optional on the sending side.
In the end the maintainers of the library will decide whatever fits more the library as a whole. That was just my personal opinion :)