With bb8dcc9874 the concept if IQ request
handlers was introduced in Smack. This doesn't allow packet/stanza
collectors/listeners to filter for incoming IQ requests. Unfortunately
the file transfer code relied on this being able, so it broke with the
change.
There were two places where the file transfer code was listening for
incoming IQ requests:
- InitationListener(s)
- Negotiator(s)
With this change, we let the InitiationListener signal the existence of
an incoming initation request, send by an IQ of type 'set', using the
newly created EventManager utility.
The negotiator waits for those events to arrive and proceedes as it would
have done when the packet collector was used.
the combination with concurrencyLevel and LinkedBlockingQueue never
worked as intented. The idea was that the cachedExecutorService would
spawn new threads until maximumPoolSize (=concurrencyLevel) is reached,
and then start queing the Runnables.
But this was not the case, since ThreadPoolExecutor does not take into
consideration if the worker threads is busy, i.e. executing a Runnable,
or idle, i.e. waiting for a Runnable.
This means that if a busy Worker would execute a Runnable, which would
block, because it's waiting for an event (e.g. an incoming IQ
request), then the handling of those incoming IQ request would be
queued by ThreadPoolExecutor, because no fewer threads then corePoolSize
are running and the task can be queued (since the LinkedBlockingQueue is
unbounded).
Using the term 'enabled' was a terriable choice from a security
perspective, as it gives the user the impression that the security is
"enabled". In fact this setting is only slightly better then
"disabled".
Make that fact clear in the javadoc too.
The XMPPConnection interface does not define methods to manipulate the
connection state (e.g. connect(), disconnect()). The example should use
the connection type as declared type.
Since e6045c6593 the cached executor
services maximum pool size is limited to concurrencyLevel (was
previously Integer.MAX_VALUE). In order to prevent
RejectExecutionException we need to use an queue which max size is
greater than 1 (i.e. nont an SynchronousQueue).
Connection closed with error java.util.concurrent.RejectedExecutionException: Task org.jivesoftware.smack.tcp.XMPPTCPConnection$2@41dce200 rejected from java.util.concurrent.ThreadPoolExecutor@41d59150[Running, pool size = 3, active threads = 3, queued tasks = 0, completed tasks = 4]
at java.util.concurrent.ThreadPoolExecutor$AbortPolicy.rejectedExecution(ThreadPoolExecutor.java:2011)
at java.util.concurrent.ThreadPoolExecutor.reject(ThreadPoolExecutor.java:793)
at java.util.concurrent.ThreadPoolExecutor.execute(ThreadPoolExecutor.java:1339)
at org.jivesoftware.smack.AbstractXMPPConnection.asyncGo(AbstractXMPPConnection.java:1583)
at org.jivesoftware.smack.tcp.XMPPTCPConnection.processHandledCount(XMPPTCPConnection.java:1655)
at org.jivesoftware.smack.tcp.XMPPTCPConnection.access$2300(XMPPTCPConnection.java:137)
at org.jivesoftware.smack.tcp.XMPPTCPConnection$PacketReader.parsePackets(XMPPTCPConnection.java:1083)
at org.jivesoftware.smack.tcp.XMPPTCPConnection$PacketReader.access$200(XMPPTCPConnection.java:896)
at org.jivesoftware.smack.tcp.XMPPTCPConnection$PacketReader$1.run(XMPPTCPConnection.java:911)
at java.lang.Thread.run(Thread.java:841)
and remove getConnectionID().
Also make streamId a field of AbstractXMPPConnection. Most XMPP
connection types have a streamId, it appears to be optional when BOSH
is used though.
RFC 6121 § 5.2.2:
"""
If an application receives a message with no 'type' attribute or the
application does not understand the value of the 'type' attribute
provided, it MUST consider the message to be of type "normal" (i.e.,
"normal" is the default).
"""
Otherwise we may get an NPE if the key was cached:
Connection closed with error java.lang.NullPointerException: Attempt to get length of null array
at org.jivesoftware.smack.util.stringencoder.Base64.encodeToString(Base64.java:43)
at org.jivesoftware.smack.sasl.core.SCRAMSHA1Mechanism.evaluateChallenge(SCRAMSHA1Mechanism.java:201)
at org.jivesoftware.smack.sasl.SASLMechanism.challengeReceived(SASLMechanism.java:230)
at org.jivesoftware.smack.SASLAuthentication.challengeReceived(SASLAuthentication.java:328)
at org.jivesoftware.smack.SASLAuthentication.authenticated(SASLAuthentication.java:347)
at org.jivesoftware.smack.tcp.XMPPTCPConnection$PacketReader.access$200(XMPPTCPConnection.java:894)
at org.jivesoftware.smack.tcp.XMPPTCPConnection$PacketReader$1.run(XMPPTCPConnection.java:909)
at java.lang.Thread.run(Thread.java:818)
instead of just logging a warning if the XMPP domain has no DNS SRV
lookups, create the failedAddresses list now within DNSUtil and add the
information that the SRV lookup failed.
In initConnection, only initReaderAndWriter() throws IOException.
connectUsingConfiguration doesn't need to take an argument.
PacketReader.init does not throw a SmackException.
Use Async.go() in PacketWriter, just like it's already done in PacketReader.
Also limit the max pool size of the remove callbacks service and use the
same keep alive time for idle threads as the cached executor service
uses.
Note that it would be possible to merge those two. But this could lead
to tasks from the cached executor service blocking the removal of
callbacks, which we don't want.
Make 'order' an long
Parse fall-through case's child elements (message, iq, presence-in,
presence-out)
Remove
privacy.addExtension(new DefaultPacketExtension(parser.getName(), parser.getNamespace()));
at the beginning of PrivacyProvider. Was there since day one for an
unknown reason.
since this method is not only used to determine the key for the
presence map, but also for the 'entries' map.
The logic is in both cases the same: If somehow an entry with an
resourcepart went into the roster (RFC 6121 does not explicity forbid
JIDs with an resourcepart in the roster), then use the full JID as
key, otherwise use the bare JID.
See also SMACK-19.
add a new AutoReceiptMode enum that specifies how delivery receipt
requests are handled. Default is to send receipts if the requstor is
subscribed to the user's presence.
Also make sure that messages contain an id if a receipt request is
added to it.
instead of getMessage(), because some Exceptions, e.g. Android's
NetworkOnMainThreadException, will return null on
getMessage(). Exception.toString() does what we want, i.e. returns
- the exception class name
- and the return value of getLocalizedMessage()
from o.jsmack.tcp.sm, as XEP-198 Stream Management is not an "XMPP over
TCP" exclusive feature. It could also be use together with the Websocket
binding of XMPP, so we may have a smack-streammangement in the
future. This change prepares for that by moving the SM code out of the
XMPP TCP package namespace.
if connect() was not previously called. Previously calling login() with
arguments would not check for the preconditions.
The check to throw needs to be performed in AbstractXMPPConnection
before every 'abstract login(Non)Anonymously()' call. That's the two
lines that check the preconditions are duplicated.
Also fix NPE in
XMPPTCPConnection.throwNotConnectedExceptionIfAppropriate() when
packetWrite is null (i.e. if the connection was never connected before).
(yeah, I know)
Sometimes "a friend" has setup an XMPP service which uses a self-signed
cert. While we can get a decent amount of security by using techniques
like e.g. the MemorizingTrustManager, there's still a pitfall. If the
service's TLS certificates contains no or the wrong service/hostname
information, Smack will throw an CertificateException. Therefore provide
an API call to disable hostname verification.
This is useful for cases where a result set is requested, as it's the
case in XEP-13 and XEP-313.
Also adds
XMPPConnection.createPacketCollector(PacketCollector.Configuration).
InterruptedExceptions should be treated as the users intention to
'cancel' the current thread's task. There is no such thing as a
spurious interrupt (not to be confused with "spurious wakeups").
after at most 12 hours.
Also set a keep alive time for the removeCallbacksService to 30 seconds
and add AbstractXMPPConnection.schedule(Runnable, long, TimeUnit).
isAnonymous() is now a bit weakened since it also considers
allowNullorEmptyUsername.
SaslExternalMechanism of smack-sasl-provided now also doesn't throw an
UnsupportedOperationException.
Follow up on 7e4e3699a1
to avoid confusion between the IQ element 'iq' and the IQs child
element. ELEMENT defined in an IQ sublcass should contain the *child*
element.
Add element to StreamInitation and fix FileTransferManager which still
used a packet listener instead of an IQ request handler to handle
incoming stream initiation requests.
to ConnectionConfiguration.Builder().
And prepare SASL EXTERNAL for empty or null usernames.
Also clarify some parts regarding the user field.
Fixes SMACK-627
This also moves the logic to send error IQ replies from "when there is
no IQ provider registerd" to "when there is no IQ request handler
registered". Which has for example the advantage that IQ parsing no
longer asks for a connection instance.
It's important to know if the stream was resumed. authenticated() is the
ideal callback for Managers to reset their state (e.g. cached values of
the connection state). But if the stream was resumed, the cached values
don't have to be reset.
notably add a cache for the active and default privacy list to avoid
IQ get/response round-trips.
Also add a few methods to PrivacyListManager to get the privacy list
names. The already existing methods always returned the whole list
together with the name, which caused two round-trips.
Simplified some code.
Properly escape Privacy XML.
Differentiate between asynchronous and synchronous ones. Asynchronous
are the ones where the invocation order may not be the same as the order
in which the stanzas arrived.
Since it's no longer guaranteed that when a unit test calls
processPacket(stanza)
the stanza will be completely processed when the call returns, it was
necessary to extend the unit tests (mostly Roster and ChatManager) with
a packet listener that waits for his invocation. Since we now also use
LinkedHashMaps as Map for the packet listeners (SMACK-531, SMACK-424),
adding a packet listeners as last also means that it will be called as
last. We exploit this behavior change now in the unit tests.
Rename 'recvListeners' to 'syncRecvListeners' in AbstractXMPPConnection.
Rename 'rosterInitialized' to 'loaded' in Roster.
Add Roster.isLoaded().
Reset 'loaded' to false in
Roster.setOfflinePresencesAndResetLoaded() (was setOfflinePresences()).
Fixes SMACK-583, SMACK-532, SMACK-424
on package layer instead of Declarative Service (DS) approach.
Restructuring and cleanup of initialization process to ensure that all
internal config files are found by the corresponding bundle
classloaders.
SMACK-343
- HeadersExtension.getHeaders() now returns a List instead of a
Collection
- Use XmlStringBuilder in Header and HeadersExtension toXML()
- Add HeadersProviderTest
- Use Smack formatting
Also remove duplicate parsing code regarding SHIM from HOXT
implementation.
- Change method modifiers keyword order to JLS
- Use attributecount when creating the HashMap
- Rename 'tag' to event'
- Use diamond operator
- Use Smack formatting style
Data Forms Validation are a part of Data Fields and implemented as
extensions, added to a Datafield.
Data validation extensions are validated before adding to the message,
using the consistency rules as described in the XEP.
Fixes SMACK-621.
Minor modifications done by Florian Schmaus <flo@geekplace.eu>
since most JUL implementations do not print log messages of level
FINE (or lower) per default. Therefore the debugging output is not seen
by the user who wants to debug and a second step is required to
configure JUL to also log FINE or lower log messages to the console.
This fixes e.g. IllegalArgumentException "'conflict' can not contain a
condition text", when receiving elements like
<stream:error>
<conflict xmlns='urn:ietf:params:xml:ns:xmpp-streams'></conflict>
<text xml:lang='' xmlns='urn:ietf:params:xml:ns:xmpp-streams'>
Replaced by new connection
</text>
</sream:error>
(ejabberd does this)
This simplifies code as there is no longer a distinction between
"normal" providers and introspection providers in ProviderManager
necessary.
It's also easier to get an idea where introspection is used for parsing.
Introduce AbstractError, change 'Conditions' to enums. Because of
AbstractError, it was necessary that PlainStreamElement and
TopLevelStreamElement becomes an interface. Thus the implementation of
TopLevelStreamElement.toString() had to be removed.
This adds
- policy-violation
- unexpected-request
to XMPPError.Condition, and removes the
- payment-required
- remote-server-error
- unexpected-condition
- request-timeout
Conditions
The file transfer code does now no longer throw XMPPErrorExceptions, but
SmackExceptions.
Fixes SMACK-608. Makes it possible to resolves SMACK-386.
there was a lot of duplicate code in ConsoleDebugger and
AndroidDebugger, which resides now in AbstractDebugger. Those two and
the new JulDebugger subclass AbstractDebugger.
Introducing a clean split between the constant connection configuration
parameters, which are now all in ConnectionConfiguration and the dynamic
connection state (e.g. hostAddresses) which are now in
AbstractXMPPConnection.
Also removed all arguments of login() since the username, password,
resource and callback handler need now to be configured via
ConnectionConfiguration.
Also remove documentation/extensions/messageevents.md, as it's already
in documentation/legacy
This is actually only part one, i.e. with this commit if the user adds a
PacketExtension to an IQ it will be included in IQ.toXml(). Which was
previously only the case if the IQ subclass explicitly included packet
extensions.
The second part of the change is to change the IQ provider, so that
packet extensions are automatically parsed.
Cases where PacketExtensions are used for Message and IQ are slightly
changed. The IQ sublcass now only has a field with this
PacketExtension (see for example
bytestreams.ibb.packet.DataPacketExtension).
Also changed hoxt API: Removed unnecessary indirection and made the
API more Smack idiomatic.
Remove the synchronized, as getExtensions() is properly synchronized and will
return a copy.
Return XmlStringBuilder instead of CharSequence to take advantage of
fast XmlStringBuilder.append(XmlStringBuilder).
Mark the method as final, as it should not be overwritten.
to keep the order in which the elements are added.
Some XEPs define a Schema using 'xs:sequence' (see for example XEP-60 §
17.1), so the order the PacketExtensions are added should be the same
they are transformed to a XML String.
In order to prevent
<iq to='...' from='...' type='result' id='1'>
<mynode xmlns='my:namespace' myattr='...'>
<somenode\>
</mynode>
</iq>
showing up as UnparsedResultIQ like this:
<iq to='...' from='...' type='result' id='1'>
<somenode\>
</iq>
Also make executorService a non-ScheduledExecutorService. This was an
artifact from times where executorService as used to schedule
Runnables. But now it's just used to queue the receive packets in
Runnables and call the packet collectors and listeners.
Those where never correctly implemented and are not really used. If you
want to compare Packets, compare their toXML() result.
N.B. that we keep hashCode/equals for Message.(Subject|Body) because
those are correct and are in-use ('bodies' and 'subjects' are Sets in
Message).
allowing O(1) lookups for PacketExtensions
The one EntityCapsManagerTest becomes obsolete with this change, as
duplicate extension elements (RFC 6120 § 8.4) are now no longer possible
after the stanza has been parsed (they still may be received on the
wire, but only the last duplicate will be added).
sendListeners are now invoked *after* the packet has been put on the
wire.
Also sending listener exceptions are not catched and not only
NotConnectedExceptions. And a exception does not cause a 'break' but a
'continue' now. Log level is WARNING now.
Thanks to Stefan Karlsson for helping with the implementation.
Also add SASLMechanism.checkIfSuccessfulOrThrow(), to increase the
security by verifying the mechanisms state at the end of SASL
authentication.
SASLMechanism now has a SASLPrep StringTransformer.
Refactor SHA1 functions out of StringUtils into SHA1 utility class.
Add MAC utility class.
Make DummyConnection getSentpacket() methods use generics to make unit
testing SCRAM-SHA1 easier.
Fixes SMACK-398
If a user enabled Smack debug via the property 'smack.debugEnabled', a
ConnectionConfiguration could be created where debuggerEnabled is
'false', because Smack is not yet initialized.
Also make sure that if the property is not set, it won't overwrite
DEBUG_ENABLED = true.
Thanks to William Murphy for providing a detailed issue description and
supposing a fix.
As otherwise there would be multiple log statements (which are of level
fine but nevertheless).
PacketParserUtils: XmlPullParser does not support XML_ROUNDTRIP org.xmlpull.v1.XmlPullParserException: unsupported feature: http://xmlpull.org/v1/doc/features.html#xml-roundtrip (position:START_DOCUMENT null@1:1)
PacketParserUtils: at org.kxml2.io.KXmlParser.setFeature(KXmlParser.java:2091)
PacketParserUtils: at org.jivesoftware.smack.util.PacketParserUtils.newXmppParser(PacketParserUtils.java:150)
PacketParserUtils: at org.jivesoftware.smack.util.PacketParserUtils.newXmppParser(PacketParserUtils.java:172)
PacketParserUtils: at org.jivesoftware.smack.tcp.XMPPTCPConnection.openStream(XMPPTCPConnection.java:963)
PacketParserUtils: at org.jivesoftware.smack.tcp.XMPPTCPConnection$PacketWriter.access$2600(XMPPTCPConnection.java:1224)
PacketParserUtils: at org.jivesoftware.smack.tcp.XMPPTCPConnection$PacketWriter$1.run(XMPPTCPConnection.java:1263)
PacketParserUtils: XmlPullParser does not support XML_ROUNDTRIP org.xmlpull.v1.XmlPullParserException: unsupported feature: http://xmlpull.org/v1/doc/features.html#xml-roundtrip (position:START_DOCUMENT null@1:1)
PacketParserUtils: at org.kxml2.io.KXmlParser.setFeature(KXmlParser.java:2091)
PacketParserUtils: at org.jivesoftware.smack.util.PacketParserUtils.newXmppParser(PacketParserUtils.java:150)
PacketParserUtils: at org.jivesoftware.smack.util.PacketParserUtils.newXmppParser(PacketParserUtils.java:172)
PacketParserUtils: at org.jivesoftware.smack.tcp.XMPPTCPConnection.openStream(XMPPTCPConnection.java:963)
PacketParserUtils: at org.jivesoftware.smack.tcp.XMPPTCPConnection$PacketReader.access$200(XMPPTCPConnection.java:970)
PacketParserUtils: at org.jivesoftware.smack.tcp.XMPPTCPConnection$PacketReader$1.run(XMPPTCPConnection.java:989)
PacketParserUtils: XmlPullParser does not support XML_ROUNDTRIP org.xmlpull.v1.XmlPullParserException: unsupported feature: http://xmlpull.org/v1/doc/features.html#xml-roundtrip (position:START_DOCUMENT null@1:1)
PacketParserUtils: at org.kxml2.io.KXmlParser.setFeature(KXmlParser.java:2091)
PacketParserUtils: at org.jivesoftware.smack.util.PacketParserUtils.newXmppParser(PacketParserUtils.java:150)
PacketParserUtils: at org.jivesoftware.smack.util.PacketParserUtils.newXmppParser(PacketParserUtils.java:172)
PacketParserUtils: at org.jivesoftware.smack.tcp.XMPPTCPConnection.openStream(XMPPTCPConnection.java:963)
PacketParserUtils: at org.jivesoftware.smack.tcp.XMPPTCPConnection$PacketReader.access$200(XMPPTCPConnection.java:970)
PacketParserUtils: at org.jivesoftware.smack.tcp.XMPPTCPConnection$PacketReader$1.run(XMPPTCPConnection.java:989)
PacketParserUtils: XmlPullParser does not support XML_ROUNDTRIP org.xmlpull.v1.XmlPullParserException: unsupported feature: http://xmlpull.org/v1/doc/features.html#xml-roundtrip (position:START_DOCUMENT null@1:1)
PacketParserUtils: at org.kxml2.io.KXmlParser.setFeature(KXmlParser.java:2091)
PacketParserUtils: at org.jivesoftware.smack.util.PacketParserUtils.newXmppParser(PacketParserUtils.java:150)
PacketParserUtils: at org.jivesoftware.smack.util.PacketParserUtils.newXmppParser(PacketParserUtils.java:172)
PacketParserUtils: at org.jivesoftware.smack.tcp.XMPPTCPConnection.openStream(XMPPTCPConnection.java:963)
PacketParserUtils: at org.jivesoftware.smack.tcp.XMPPTCPConnection$PacketReader.access$200(XMPPTCPConnection.java:970)
PacketParserUtils: at org.jivesoftware.smack.tcp.XMPPTCPConnection$PacketReader$1.run(XMPPTCPConnection.java:989)
The approach to avoid XmlPullParser's XML_ROUNDTRIP feature prevents
Smack from handling XML namespaces on e.g. PubSub payloads on platforms
where support for XML_ROUNDTRIP would be available.
We now check for this XmlPullParser feature and use it if it's
available.
Using
createPacketCollector(filter);
sendPacket(packet);
was error prone, i.e. the PacketCollector could leak if sendPacket()
would throw an exception and the user forgot to call
PacketCollector.cancel(). For cases where
createPacketCollectorAndSend(IQ) is not sufficient (because we don't
send IQs), createPacketCollectorAndSend(PacketFilter, Packet) is now
used, which does take care that the PacketCollector does not leak if
sendPacket() throws an Exception.
Use PresenceListener as intercepting callback interface.
Remove the for-each loops in MultiUserChat by using the interception
facilities of the XMPPConnection.
Add ToFilter.
both serve the same purpose: As callback for Packets. There is no need
to have both, so remace PacketInterceptor and let PacketListener take
its place. Some classes like ChatStateManager can now use
MessageListener as interceptor callback, which is more convenient.
instead of using a PacketListener, which means that the user has to
downcast the Packet to Message, we now use a Listener which callback
parameter is already Message/Presence.
It is necessary to introduce MessageListener and PresenceListener, which
are interfaces that have a callback for Message/Presence instead of
Packet. The 'old' MessageListener is renamed to ChatMessageListener.
Use Generics in ConnectionDetachedPacketCollector.
- Make MultipleAddress.Type a enum
- Change the signature of the methods to use Collection instead of List
- Use for-each loops instead of iterators
- Switch Provider to new provider pattern (using switch-case)
- Use XmlStringBuilder (extend the API by two new methods)
Allows interceptor to remove themselves in the interceptPacket() method.
Same change for Interceptors as was done in
15d59299a2 for (send|recv)Listeners.
instead of rely on ListenerWrapper checking the PacketFilter *and*
invoking the PacketListener we now use two for-each loops, where the
first filters the PacketListeners that should get invoked and use the
second for-each loop to actually invoke the PacketListener.
Before, the code was not thread safe if a PacketListener would remove
itself from the (send|recv)Listeners.
Also make packet(Listener|Filter) in ListenerWrapper final.
this is the first stop towards fixing "SMACK-65: parsing should look for
depth", by providing the initial parsing depth to the provider. Some
methods (.e.g parseMessage) now use the depth as abort condition,
instead of a unclean String equals check.
parseIQ() and parseExtension() where both renamed to parse.
This also restricts the Exceptions thrown by the parse method, to just
XmlPullParserException, IOException and SmackException (not really a big
victory, but nevertheless a slight improvement).
StreamFeatureProvider is now gone, we simply use PacketExtensionProvider
for stream features.
- De-duplicate code by moving it into AbstractXMPPConnection
- Introduce TopLevelStreamElement as superclass for all XMPP stream elements.
- Add SynchronizationPoint, ParserUtils
- Add ParserUtils
Fixes SMACK-333 and SMACK-521
The minimal condition in order to check equality in XmlStringBuilder is
that 'other' implements the CharSequence interface. Not as it was
previously, that it's also a XmlStringBuilder.
This allows junit's assertEquals() to be used (to a certain extend).
Setting `jid` to `null` was probably not the intended behaviour of this constructor and means that the `IQReplyFilter` will later reject responses that it shouldn't, because the `AbstractXmppConnection` will automatically construct a JID based on the authenticated username.
In particular, this breaks attempting to connect to the PVP.net XMPP server.
inverse boolean condition, correct is: "if packetListener has been
removed, then we received no response" and add check for
exceptionCallback being null.
also remove faulty PongFilter from PingManager. It never matched any
stanzas, since a Pong is just a plain result IQ that is not qualified by
any XMPP Ping namespace.
Fixes SMACK-597
You can set your custom debugger class as before, by using clear api
method ReflectionDebuggerFactory.setDebuggerClass, or you can set custom
debugger factory using SmackConfiguration.setDebuggerFactory if it's not
enough flexible for your needs
