MiniDnsResolver returning neither failed nor resolved addresses

Smack Smack Support
1

Two findings from the “another day, another yak shaved” department:

1. MiniDNS can’t resolve CNAMEs

I have a test domain for IDNA and some other things, and apparently the network I’m in fails at SRV, so Smack4 is falling back to the direct hostname resolution instead:

_xmpp-client._tcp.ツ.op-co.de.	3585 IN	SRV	5 0 5222 xmpp.op-co.de.
_xmpp-server._tcp.ツ.op-co.de.	3585 IN	SRV	5 0 5269 xmpp.op-co.de.
ツ.op-co.de.		3585	IN	CNAME	op-co.de.
op-co.de.		2165	IN	A	83.223.75.24

The resolver returns the last two lines (CNAME and IN/A), then Record.isAnswer() is called on each of those. First it checks the CNAME response, which is not accepted as an answer because CNAME is neither of A, AAAA or ANY. Then it checks the A response, which is not an accepted answer because the name doesn’t match:

    public boolean isAnswer(Question q) {
        return ((q.type == type) || (q.type == TYPE.ANY)) &&
               ((q.clazz == clazz) || (q.clazz == CLASS.ANY)) &&
               (q.name.equals(name));
    }

This leads MiniDnsResolver.lookupHostAddress0() to return an empty list of resolved addresses, without populating the failedAddresses with anything.

2. stop using assert already!

Luckily, there is an assert check for that, doing some serious double-negation:

assert-emptiness
assert-emptiness993×306 48.5 KB

Wait, how could it ever pass through the assert? Both lists are empty! That can’t ever happen! Except, you need to perform some badly documented black magic to make assert work (adb shell setprop debug.assert 1, but then the assert still will pass!?).

Please pretty please just replace it with a checked(!) exception already. Just because this is not allowed to happen doesn’t mean you can either silently ignore the condition or crash my whole application.

1 Like
2

(1) A SRV RR pointing to a CNAME is invalid as per RFC 2782

Target

The domain name of the target host. There MUST be one or more address records for this name, the name MUST NOT be an alias (in the sense of RFC 1034 or RFC 2181).

(2) The asserts in this case check if the used components obey the API contract. In this case they apparently do not. I avoid performing those checks unconditionally.

3

If you have another look at my DNS paste, the SRV is not pointing to a CNAME. Those are different records. The CNAME is merely a fallback for clients that can’t do SRV lookup, for whatever reason. You can consider the SRV records as non-existent for the reproduction of this ticket. If you wish, I can even remove them for you to be able to reproduce the setup. Otherwise, feel free to manually set ツ.op-co.de:5222 as the host:port for a Smack connection.

The asserts in this case check if the used components obey the API contract. In this case they apparently do not. I avoid performing those checks unconditionally.

We’ve had this discussion back in 2014. Asserts haven’t become less harmfull since then. You are prematurely optimizing two integer comparisons at a huge cost to the debug-ability of your library.

The fact that the assert is silently passed despite being incorrent (and that I sunk another hour of my time into single-stepping this problem) is probably not enough to convince you anyway. Of the arguments that I made back in 2014, only #2 applies in this specific situation:

  1. assert is the wrong tool for checking the validity of external inputs.
  2. you will have inconsistent library behavior that is dependant on JRE runtime configuration
  3. somebody will be able to profanity masked KILL YOUR APPLICATION by sending invalid XML
  4. even by using assert() in a place where it does no harm you will create a bad example ready to be copied into places where it can do actual damage.

However, just a short grep for assert in the current code base made my eyes bleed (in XMPPTCPConnection):

if ("jabber:client".equals(parser.getNamespace(null))) {
	streamId = parser.getAttributeValue("", "id");
	String reportedServerDomain = parser.getAttributeValue("", "from");
	assert (config.getXMPPServiceDomain().equals(reportedServerDomain));
}

Please pretty please just stop using asserts in your library code. I’ve got enough grey hair already.

4

Ahh right sorry. As you didn’t mention what the input for the resolve function is, I assumed it to be op-co.de. Yes CNAME chasing is not yet implemented in MiniDNS. See

If you know someone suitable and interested, then this could be a project for this years GSoC.

5

This topic was automatically closed 100 days after the last reply. New replies are no longer allowed.