| Age | Commit message (Collapse) | Author |
|---|
|
Corresponds to the Swiften change of the same name, d949d1638c
Test-information:
Unit tests pass. Verified that the new code works as expected in
a test application that previously would never see timeouts.
Change-Id: I95cc73a81e42d6ac00c79f74531e8dd6c67882f3
|
|
The Connector class had "_xmpp-client._tcp." hard-coded in it, which
meant that it was not suitable for non XMPP clients.
This change means that Connector could now be used by clients who are
interested in arbitrary SRV records; the CoreClient class is updated
accordingly.
Test-information:
Built and tested using MLC.
Also tested with a client that is interested in IMAP SRV records
Change-Id: Ia23c148fd8afdd7b3271c47b1c96d086d57a44bd
|
|
A recent change made Stroke use the dnsjava library instead of JNDI for
domain service queries, because JNDI had problems with IPv6 addresses.
The change also replaced the use of Java's standard InetAddress class for name
resolution with the Address class inside dnsjava - this change was not neccessary, and
is problematic because although the documentation for "Address" says that it "Includes functions
similar to those in the java.net.InetAddress class", it does not provide equivalent functionality.
Specifically, whereas InetAddress.getAllByName() will use the local system's
"host" file when attempting to resolve hostnames, the corresponding Address.getAllByName() method
in dnsjava does not do this.
This means that if a user inserts values into /etc/hosts, they will be ignored by
the Address.getAllByName().
As a result, users who had expected stroke to honour values in /etc/hosts
(which is something you might want to do just for testing purposes) will be surprised
when it stops doing this.
So this patch reverts the code in question to use InetAddress instead of dnsjava's Address class.
Test Information
I added the following lines to my /etc/hosts file.
127.0.0.1 alexmac.com
127.0.0.1 alexmac.clayton.com
At time of the testing there already existed an external domain with name alexmac.com
but none corresponding to alexmac.clayton.com.
I then ran the 'Check DNS for a domain...' dialog in the MLC Help Menu.
Before the patch this would give me the the details for the external domain
for 'alexmac.com' and say no DNS could be found for 'alexmac.clayton.com'.
After the patch the correct details (i.e 127.0.0.1) were returned for both domains.
Also, before the patch I could not connect to the local xmpp server 'alexmac.com'.
After the patch I connected correctly.
Change-Id: If7f15b8aa98313278a1892eb27a5f73aaea8802b
|
|
There are limitations when using JNDI for DNS lookups, including that
it does not properly handle the situation when resolv.conf contains
IPv6 addresses (Isode bug #44832) - see e.g.
http://java.net/jira/browse/JITSI-295
JNDI is also not readily available on Android, which makes it slightly
more awkward to use Stroke on that platform.
This patch changes the PlatformDomainName classes so that they use
classes from dnsjava rather than JNDI.
The patch also updates the build scripts so that dnsjava.jar is
fetched (if necessary) and included in the build.
Indentation in build.xml has been tidied up
Test-information:
Ran unit tests - ok
Ran MLC - works OK and no longer throws NumberFormatExceptions
when resolve.conf contains "nameserver 2001:470:f052::2"
Change-Id: Iacf1105c52c281f9e59b60ea6caa011914b588dc
|
|
Some discussion followed the "Fix synchronization problem in
ByteArray" patch, and that led us to believe that it would be better
to change the JavaConnection class so that it does not rely on being
able to pass ByteArrays around in a way that makes them vulnerable to
the problems that had been seen.
The JavaConnection class accepts a ByteArray in its "write()" method,
and emits a ByteArray when it has read data.
ByteArrays are not the ideal way for the JavaConnection class to
manipulate data and so this patch changes the implementation so that:
a) the "write()" method extracts the byte[] from the supplied
ByteArray and uses these objects, rather than keeping
references to the ByteArray objects (which might lead to
synchronisation issues).
b) the "doRead()" method uses a ByteArrayOutputStream to hold incoming
data, and only constructs a ByteArray out of it when it is ready to
return the data to the application.
These changes make the class more efficient, since in the case of (a),
the need to create temporary ByteArrays is removed, and in (b) the
code no longer creates ByteArrays by iterating through the network
data one byte at a time and appending it to a ByteArray.
It also means that the "synchronized" patch (which would fix the
problem) is no longer necessary, and so that code is reverted.
Test-information:
I patched the code to emulate the situation that would occur when a
buffer is only partially written, and verified that in this case it
correctly re-inserted the unwritten portion of the buffer at the
front of the pending queue.
Ran MLC to various servers, all seems to work OK.
Tested in Harrier, seems to work OK, and does not exhibit problems
that we had seen previously which led us to investigate this issue.
Change-Id: Ifcda547402430c87da45ba7d692518b5af285763
Signed-off-by: Nick Hudson <nick.hudson@isode.com>
|
|
In the PlatformDomainNameResolver class there is a DomainNameAddressQuery
class (accesible via DomainNameResolver->createAddressQuery()) for
performing a DNS lookup on a given domainname. This should have been
returing the set of all HostAddress associated with a given domain, but
instead was only returning a singleton set (or empty if there was no dns).
This patch fixes this by changing the method call from
InetAddress.getByName() to InetAddress.getAllByName().
Test-information:
Tested on top of my MLC Diagnose SRV patch. For 'google.com' we now see a
full list of ip addresses associated with it, rather then just the one.
Change-Id: I6e57c16bb64f76048f16bcff8ee9c1924049a051
|
|
This change moves responsibility for creating the TLSContextFactory
from CoreClient into NetworkFactories, which is in line with the
Swiften implementation.
This means that a caller may now provide his own concrete
TLSContextFactory using code of the form:
NetworkFactories myNetworkFactories;
.
.
myNetworkFactories = new JavaNetworkFactories(eventLoop()) {
@Override
public TLSContextFactory getTLSContextFactory() {
return new MyTLSContextFactory();
}
};
Test-information:
I implemented separate TLSContextFactory and TLSContext classes that
used OpenSSL via JNI) to provide SSL functionality. I was able to
switch to using these with the mechanism that this patch provides.
I also verified that existing code which doesn't try to provide its
own NetworkFactories subclass still works as before (i.e. this patch
doesn't break existing applications).
Change-Id: Ibf07ddbbb4a4d39e4bb30a28be9aa0c43afe005f
Signed-off-by: Nick Hudson <nick.hudson@isode.com>
|
|
It is possible to have a null selector if socketchannel open failed so adding a
null check in this patch.
Test-information:
sanity tested on linux by connecting/reconnecting on an xmpp service on linux
Change-Id: Idee180ca4aefd1f743705da674b486dd8acc4922
Reviewer: Nick Hudson <nick.hudson@isode.com>
Reviewer: Kevin Smith <kevin.smith@isode.com>
|
|
I left MLC which is an XMPP Client running overnight and noticed "Too many open files"
error when trying to stop/start xmpp server. On doing an "lsof | grep java", I noticed
a large number of open sockets which was presumably the cause of this error.
After this patch, the lsof command shows a constant number of open sockets.
Test-information:
tested on centos vm by doing "lsof | grep java " wc-l"-open sockets do not increase.
Change-Id: I7ddff78a1efb005177427fda21f1d0b92d8ed7cc
Reviewer: Kevin Smith <kevin.smith@isode.com>
|
|
When investigating problems on Solaris, attention focused on the
JavaConnection class, whose implementation appeared to be non-optimal.
The original implementation had a loop which operated on a
non-blocking socket, and looked something like this:
while (!disconnecting) {
while (something to write) {
write data to socket;
if write failed {
sleep(100); // and try again
}
}
try reading data from socket
if (any data was read) {
process data from socket;
}
sleep(100);
}
Because the socket is non-blocking, the reads/writes return straight
away. This means that even when no data is being transferred, the
loop is executing around ten times a second checking for any data to
read/write.
In one case (Solaris client talking to Solaris server on the same VM)
we were consistently able to get into a state where a write fails to
write any data, so that the "something to write" subloop never exits.
This in turn means that the "try reading data" section of the main
loop is never reached.
Investigation failed to uncover why this problem occurs. The
underlying socket appears to be returning EAGAIN (equivalent to
EWOULDBLOCK), suggesting that the write fails because the client's
local buffer is full. This in turn implies that the server isn't
reading data quickly enough, leading to the buffers on the client side
being full up. But this doesn't explain why, once things have got
into this state, they never free up.
At any rate, it was felt that the implementation above is not ideal
because it is relying on a polling mechanism that is not efficient,
rather than being event driven.
So this change re-implements JavaConnection to use a Selector, which
means that the main loop is event-driven. The new implementation
looks like this
while (!disconnected) {
wait for selector
if (disconnected) {
break;
}
if something to write {
try to write data;
}
if something to read {
try to read data;
}
if still something to write {
sleep(100);
post wake event; // so that next wait completes straight away
}
}
Test-information:
Testing appears to show that the problems we saw on Solaris are no
longer seen with this patch (Solaris tests still fail, but later on,
which appears to be due to a separate problem).
Testing shows that this leads to the thread spending much more time
idle, and only being active when data is being read/written (unlike
the original implementation which was looping ten times a second
regardless of whether any data was being read/written).
Testing using MLC seems to show the new implementation works OK.
I was unable to provoke the "write buffer not completely written"
case, so faked it by making the doWrite() method constrain its maximum
write size to 200 bytes. By doing this I verified that the "leftOver"
section of code was working properly (and incidentally fixed a problem
with the the initial implementation of the patch that had been passing
the wrong parameter to System.arrayCopy).
Change-Id: I5a6191567ba7e9afdb9a26febf00eae72b00f6eb
Signed-off-by: Nick Hudson <nick.hudson@isode.com>
|
|
|
|
Makes ClientOptions do more.
|
|
In one of my testing scenario, socket was not getting closed. This was happening when
an XMPP client was connecting to a domain which was different from the domain in the
jabber ID of the connection user. Moving the close call to a finally block ensures that
socket gets closed in all scenarios.
Test-information:
I created an IM domain j.com on my XMPP Server and then added a user with that
domain (user@j.com). Then I tried connecting to my Primary domain using this
new user. After removing j.com, I could an increase in number of sockets after
every poll (coreclient.connect()) but not after this patch.
|
|
If I leave an Application using Stroke running for a few hours(making periodic
connection attempts), the JVM would throw an Exception saying "Too Many Open Files".
On doing an "lsof -p <pid_of_jvm>", I noticed that there were number of open
sockets in CLOSE_WAIT state and these went up after every attempt to do a
connect on CoreClient object.
Closing of DirContext object fixes this bug and the number of open sockets
does not increase.
Test-information:
Ran MLC and kept on monitoring the result of "lsof -p <pid>". It would not
increase after this patch.
|
|
|
|
|
|
I broke the "getLocalAddress()" method. This fixes it.
Test-information:
By debugging and looking at "JavaConnection.toString()" output
|
|
The initial implementation of JavaConnection used the "Socket" class,
from which it derived InputStream and OutputStream objects to read/write data.
In order to avoid blocking, the "read" loop would only attempt to read
data if the InputStream.available() method indicated that there was
data ready to be read. However, in order to determine that an
InputStream has been closed, you have to read from it and have it
return -1 to indicate "end-of-stream".
There was no explicit test for a -1 return, but even if there had
been, it wouldn't have tripped, because of the "available()" test.
So this change makes JavaConnection use "SocketChannel" rather than
Socket, which allows (when you configure the SocketChannel to be
non-blocking) you to issue non-blocking reads which *can* return -1
when the input stream has finished.
Test-information:
Tested with test program and MLC, using both TLS and non-TLS
connections. Applications still work as expected.
Observed that when I deliberately stop the server, or break the socket
connection, the client almost immediately gets a "disconnected"
signal.
Prior to this change, stopping the server results in the client
getting a disconnected signal only when it next tries to write
something.
|
|
|
|
Hopefully the changes speak for themselves.
Some feedback relating to JavaCertificate has not been addressed; that
will be done in a separate patch
Test-information:
Can still establish sessions with / without TLS
|
|
Note that TLS won't be enabled with this patch unless you uncomment the
change in PlatformTLSFactories. With that comment removed, then a new
CoreClient session will attempt to negotiate TLS if the server supports
it.
Further changes are required to support this properly, as there
appears not to be comprehensive support in the CoreClient class for
dealing with situations when the server's certificate is not acceptable.
There's also no support yet for setting up client certificates.
Further changes will also be needed (see below) to support full
parsing of subjectAltNames from server certificates.
Significant changes are as follows
- TLSProceed - FIXME comments removed
- JavaConnection - changed so that it reads bytes from the socket's
InputStream, rather than reading chars and then constructing a
String out of them from which a byte array is then extracted.
While this seemed to work for non-binary data (e.g. non-encrypted
XMPP sessions), it breaks when you start sending binary (i.e. TLS)
data.
- JavaTLSConnectionFactory - implemented
- PlatformTLSFactories - By having this return a JSSEContextFactory, then
this will cause the client to try TLS if possible. But because other
changes are needed to make this work properly, the current code still
returns null.
- JSSEContext - new class which uses an SSLEngine to handle TLS handshake
and subsequent encryption/decryption. This is the main substance of
the SSL implementation
Note the "hack" in here to cope with SSLEngine requiring that some data
be sent from the application before it will do a TLS handshake
- JSSEContextFactory - just creates JSSEContexts
- JavaCertificate - this wraps an X509Certificate and does *some* of the
parsing of a certificate to look for stuff that is expected when
verifying an XMPP server certificate (RFC 6120 and RFC 6125). Note that
the JDK classes for parsing certificates don't provide an easy way
to decode "OTHER" subjectAltNames, and so this implementation does
not find XMPP or SRV subjectaltnames from the server certificate. This
will need extra work.
- JavaTrustManager - obtains the server certificate from the TLS handshake
and verifies it. Currently the only verification done is to check that
it's in date. More work will be needed to perform proper validation
- Where necessary, Remko's copyright comments were changed from GNU to
"All rights reserved". Isode copyright notices updated to "2012"
Test-information:
Set up XMPP server with its own certificate, and checked that TLS gets
negotiated and starts OK (provided the server cert contains e.g. a DNS
subjectAltName matching its own name). Subsequent operation appears
to be as expected.
|
|
Also made "Stanza" be an abstract class and had its ".toString()" include
the name of the subclass which is involved, so that the subclasses don't
have to do that themselves.
Also added null check to existing HostAddress.toString() method
Also fixed Remko copyright in Connector class
Test-information:
Stuff is displayed as expected in debugger.
|
|
The JavaTimer class uses Thead.sleep() to wait a specified number of
milliseconds. Thread.sleep() requires the caller catch
InterruptedException, but in the original implementation, such an
exception would result in the code assuming that the specified time
had been reached.
So as things stood, if you e.g. set a timer for 60 seconds, then the
timer might generate its "onTick" signal before that 60 seconds had
elapsed.
This patch changes the code so that the method will wait until the
specified time has been reached.
The "milliseconds" parameters are also changed to "long", which is the
type used by the rest of the java library for millisecond values.
Added a bit of javadoc and a toString() method as well.
Note there is still a "FIXME" in the code which I've not addressed.
Test-information:
Tested in debugging setup; things seem to be working as expected.
|
|
|
|
|
Swift