Skip to content

Commit

Permalink
Validates room name node part chars on conference join request.
Browse files Browse the repository at this point in the history
  • Loading branch information
damencho committed Oct 23, 2017
1 parent 23379d5 commit d26d8b3
Showing 1 changed file with 33 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@

import org.jxmpp.jid.*;
import org.jxmpp.jid.impl.*;
import org.jxmpp.jid.parts.*;
import org.jxmpp.stringprep.*;

import org.xmlpull.v1.*;

/**
Expand Down Expand Up @@ -67,9 +70,8 @@ public ConferenceIq parse(XmlPullParser parser, int initialDepth)
{
iq = new ConferenceIq();
EntityBareJid room
= JidCreate.entityBareFrom(
parser.getAttributeValue("",
ConferenceIq.ROOM_ATTR_NAME));
= getRoomJid(
parser.getAttributeValue("", ConferenceIq.ROOM_ATTR_NAME));

iq.setRoom(room);

Expand Down Expand Up @@ -172,4 +174,32 @@ else if (ConferenceIq.Property.ELEMENT_NAME.equals(name))

return iq;
}

/**
* Constructs the jid for the room by taking the last '@' part as domain
* and everything before it as the node part. Doing validation on the node
* part for allowed chars.
*
* @param unescapedValue the unescaped jid as received in the iq
* @return a bare JID constructed from the given parts.
* @throws XmppStringprepException if an error occurs.
*/
private EntityBareJid getRoomJid(String unescapedValue)
throws XmppStringprepException
{
// the node part of the jid may contain '@' which is not allowed
// and passing the correct node value to Localpart.from will check
// for all not allowed jid characters
int ix = unescapedValue.lastIndexOf("@");

This comment has been minimized.

Copy link
@ibauersachs

ibauersachs Oct 24, 2017

Member

Are you saying that Localpart.from() rejects an @ while entityBareFrom accepts it (in the localpart)?

This comment has been minimized.

Copy link
@ibauersachs

ibauersachs Oct 24, 2017

Member

Or should this simply be JidCreate.entityBareFromUnescaped?

This comment has been minimized.

Copy link
@damencho

damencho Oct 24, 2017

Author Member

Yes, using both entityBareFromUnescaped and entityBareFrom will accept a jid as damencho@[email protected]. The problem is that all the logic inside smack uses indexOf and extracts as node damencho which is correct and basically all the validation that is done in Localpart.from is skipped. Also tried asking on the issue tracker igniterealtime/jxmpp#14

This comment has been minimized.

Copy link
@ibauersachs

ibauersachs Oct 24, 2017

Member

See my comment in the jxmpp issue. This parsing here can only be a temporary workaround until jxmpp is fixed.

This comment has been minimized.

Copy link
@damencho

damencho Oct 24, 2017

Author Member

Yep, I agree, there are several other places using these jxmpp methods. This is temporary, I had to state it somewhere :)

if (ix == -1)
throw new XmppStringprepException(unescapedValue,
"wrong room name jid format");

String domainPart = unescapedValue.substring(ix + 1);
String localPart = unescapedValue.substring(0, ix);

return JidCreate.entityBareFrom(
Localpart.from(localPart), Domainpart.from(domainPart));
}
}

0 comments on commit d26d8b3

Please sign in to comment.