~samwhited/xmpp

all: change handler to provide a decoder

This patch changes the definition of handlers from taking an
xmlstream.TokenReadEncoder to the new xmlstream.TokenDecodeEncoder.
This is a breaking API change.

I've changed the definition of Handlers several times in this past, and
was loath to change it again becaues it's a major breaking change that
will affect almost all users of this module, but I decided it was
necessary once again.

While working on new packages, I was writing something like the
following as the first line of every handler:

    d := xml.NewTokenDecoder(tokenReader)

I had also introduced bugs that broke this several times (eg. because
the first start token had already been consumed and the new Decoder
would encounter the end token and break). This led me to think that we
needed to provide a decoder internally so that we could already use it
to pop tokens and make sure its internal state tracking was up to date
with the underlying stream, and to reduce the amount of boilerplate code
users of the Handler types have to write.

A simpler argument for the change might just be one of consistency: we
were previously returning an Encoder for writing tokens, but only a
TokenReader for reading them. Why would we provide more functionality
for writing tokens than we would for reading them? The answer is likely
that we can construct a Decoder from the TokenReader but we can't
construct an Encoder from a TokenWriter with the current functionality
of encoding/xml, but it's still nice for us to just provide a symetric
API to the users.

This feels like the logical next evolution of the API. I am not sure why
it wasn't done the previous time when we changed to an
xmlstream.TokenReadEncoder, but it is done now and I don't see anywhere
else we could really go with this particular parameter so hopefully
things will be more stable as we continue to progress towards 1.0.

Signed-off-by: Sam Whited <sam@samwhited.com>
internal/marshal: add encoder wrapper

Adds the ability to wrap an xml.TokenWriter into an xmlstream.Encoder
that uses the other encoding functions from the Marshal package under
the hood.

It would be better if we had something like xml.NewTokenEncoder similar
to the NewTokenDecoder changes we made to the standard library to
facilitate this module, but changes to encoding/xml are hard to get
reviwed and this would be a much bigger change than the equivalent token
decoder changes, so for now we will continue to use the inefficient
workarounds from the marshal package.

Signed-off-by: Sam Whited <sam@samwhited.com>
receipts: fix broken doc comment

Fix some copy/pasta from the xtime package that had leaked into
receipts.

Signed-off-by: Sam Whited <sam@samwhited.com>
all: prepare release v0.16.0

Signed-off-by: Sam Whited <sam@samwhited.com>
receipts: limit the amount of locking required

Limiting the amount of output stream locking required to just the actual
sending of the element means we don't have to block the entire output
stream while we wait for a response, which will likely lead to deadlock.

Signed-off-by: Sam Whited <sam@samwhited.com>
xmpp: use entire payload in SendElement

Previously Session.SendElement would only wrap the first element
encountered in the token stream and transmit it. However, you would
expect it to wrap the entire stream (even if it were multiple elements)
in the provided start element.

Signed-off-by: Sam Whited <sam@samwhited.com>
receipts: fix resource leak

Previously messages weren't removed from being tracked by the handler
after they were acked or after the context was canceled. This patch
makes sure they are removed, and adds a regression test to double check
that they're removed on context cancelation.

Signed-off-by: Sam Whited <sam@samwhited.com>
.builds: change CI to use OpenBSD temporarily

The FreeBSD image on SourceHut is broken and they have indicated that
they intend to wait for FreeBSD to issue a release instead of working
around the issue themselves. For now, change the builder to OpenBSD.

Signed-off-by: Sam Whited <sam@samwhited.com>
roster: fix iter bug in Go 1.13

Sometimes Go 1.13 leaks an io.EOF out of xml.Decoder's even when things
worked successfully. Temporarily ignoring this error will allow tests to
pass in Go 1.13 and the workaround can be removed when Go 1.15 comes
out. For the removal timeline and any updates see:
https://mellium.im/issue/29

Signed-off-by: Sam Whited <sam@samwhited.com>
receipts: new Message Delivery Receipts package

This patch imlements XEP-0184: Message Delivery Receipts
(https://xmpp.org/extensions/xep-0184.html).

Signed-off-by: Sam Whited <sam@samwhited.com>
mux: close mux iterator

An element iterator used in the muxer was previously unclosed. In this
case, the close method will likely always be a noop, but just in case
close it so that if we make changes later we don't introduce a lock
contention or resource freeing issue.

Signed-off-by: Sam Whited <sam@samwhited.com>
stanza: create stanzas from any start element

Previously stanzas were unmarshaled from the start element using an
xml.Decoder. This is slow and inefficient, and if the name of the stanza
differs an error will be returned. Implementing manual decoding of the
start element should make things faster (though this has not been
verified) and allows the name of the initial start element to be
disregarded. This will not change the name when the stanza typed value
is marshaled back into an XML stream.

Signed-off-by: Sam Whited <sam@samwhited.com>
.builds: let the DCO check work on master branch

Previously we compared the branch against whatever the current master
branch was to find new commits, so if you push to your own master branch
or we test a merged master there would never be any commits to check.
Instead, check against origin/master so that we're always comparing with
whatever we pulled down from upstream.

Signed-off-by: Sam Whited <sam@samwhited.com>
mux: fix a possible infinite loop

Previously a bug in EOF error handling could have lead to the last token
of a message stream being cut off. This could lead to Decode entering an
infinite loop of nil tokens and nil errors being returned. This is
likely a bug in the encoding/xml package which should be fixed as well,
but for now we can just make sure that we don't trigger it.

Signed-off-by: Sam Whited <sam@samwhited.com>
roster: support handling roster pushes

Add a handler to support handling roster pushes as defined in RFC 6121
and an option for easily registering the handler on a mux.ServeMux.

Signed-off-by: Sam Whited <sam@samwhited.com>
xtime: minor fix to test failure comment

Previously a test error was copied from the ping package and still
mentioned handling pings.

Signed-off-by: Sam Whited <sam@samwhited.com>
all: include close element in handler stream

Previously the close element was not included in the limited XML stream
passed to handlers. This made it possible to enter an infinite loop or
deadlock when trying to use the DecodeElement method of xml.Decoder's.
A regression test has also been added to make sure that we can't read
beyond the end of the element (which was previously the case but there
was no test for this, which was dangerous) and to make sure that the new
behavior (with the end element) is enforced.

Signed-off-by: Sam Whited <sam@samwhited.com>
mux: use xml.CopyToken instead of manual copying

Previously we were manually copying the token for token types that have
bufferes that get reused. Instead, use a function out of the
encoding/xml package that can copy any valid token type.

Signed-off-by: Sam Whited <sam@samwhited.com>
mux: use existing child element iterator

An iterator already exists in the mellium.im/xmpp/internal/iter package
that can be used to iterate over child elements. Reusing this keeps
things simple.

Signed-off-by: Sam Whited <sam@samwhited.com>
mux: simplify buffer state tracking

Previously we kept separate state for any buffered token readers and
jumped back and forth between the start of the stream (when handing them
off to handlers) and the last read start element. This patch simplifies
the code by creating a new buffered reader sharing the underlying buffer
for handlers so that the offset never has to be changed in the original
buffered reader.

Signed-off-by: Sam Whited <sam@samwhited.com>
docs: add rules for creating extensions

Signed-off-by: Sam Whited <sam@samwhited.com>
Next