Proposal: skip all Stringprep.nodeprep occurences by configuration

Openfire Openfire Dev
1

Hi all,

During the profiling of openfire, I noticed that a lot of cpu time is spent on Stringprep.nodeprep method.

This method prevent unacceptables usernames, escaping special characters.

In the situations where username is already checked before openfire request, this control would be avoided.

In JID class there is also a constructor with boolean argument to skip this nodeprep check.

I extended this skip clause also in JID.init updating the method:

if(checkStringprep)

this.node = Stringprep.nodeprep(node);

else

this.node=node;

and

if(checkStringprep)

this.resource = resourceprep(resource);

else

this.resource = resource;

I also update UserManager.createUser enveloping the nodeprep with a if clause

checkStringPrep = JiveGlobals.getBooleanProperty(“check_stringprep”, true);

if(checkStringPrep) {

// Make sure that the username is valid.

try {

username = Stringprep.nodeprep(username);

}

catch (StringprepException se) {

throw new IllegalArgumentException("Invalid username: " + username, se);

}

}

In this way I improved a lot the performance according to the profiler results.

Do you think it will be acceptable to insert a JiveGlobals property in order to let this enhancement?

Thanks.

Giancarlo Frison

2

Hey Giancarlo,

I think that it is acceptable to do stringprep operations while creating new users. This should not be a frequent operation under heavy load so I don’t think that we need to skip a validation there. If we let users define invalid usernames then we will have other problems.

Could you tell me a little more about the change in the JID#init method? How is that being used and when are you disabling the validation in that case. That method is using caches to avoid doing stringprep operations since as you noted they are expensive. For normal usage the cache is pretty effective. However, if you are testing creating new users then you will miss the username strinprep cache since you are creating new things. But again, I’m not sure if that is a case we should optimize.

Regards,

– Gato

3

Hey Gato,

It is not totally unheard of (actually, I’ve ran into a few examples just reading the Ignite forum threads) of Openfire instances running in a very controlled environment. If someone is running an environment where controlled processes instead of humanoid users are the Users in Openfire User instances, certain JID formats can be guaranateed, and node-prepping is a complete waste of resources. In particular these kind of automated environments (which grow very heavy very fast) would benefit a lot of some kind of global switch that disables node-prepping.

4

Hey Guus,

I agree with you that in controlled environments the responsibility for validating new JIDs could be placed outside of the server and move it to the automated tool. However, I’m still curious if creating new users is a frequent task. I never heard of installations that need to create hundreds of users per second during a long time. Except when we were running our load test cases and we needed to create thousands of users. Anyway, let me know if that is a real use case that you have.

Thanks,

– Gato

5

Hi Gato,

A partial optimization already exist in JID constructor with skipStringprep argument, I just suggest to expand this idea in all occurence.

In JID#init replacing the

this.node = Stringprep.nodeprep(node);

with

if(checkStringprep)
                    this.node = Stringprep.nodeprep(node);
                else
                    this.node=node;

and the same with resource and domain.

Furthermore skip the nodeprep step also creating users. I believe that handle an application which create hundreds of new users per second isn’t the usual case, but it should be easy to manage with a so spread XMPP server which Openfire is.

I agree by default the system has to check for usernames resources and domains strings, but also let to skip this so expensive control.

It’s so frustrating to realize that much of the cpu resources are spent by Openfire on such methods, and you know how much we take care about it.
JID.java (21051 Bytes)
UserManager.java (16551 Bytes)

6

Hi Giancarlo,

I didn’t look at the main code, but I guess that Openfire inspects the JID in every packet. Unfortunately it does not use an XML schema to validate the whole packet, this would be much more save.

Even with Spark I can use the smack debug mode and send a random packet with random content. So I wonder if one should really allow an administrator to change this behavior.

Maybe one can add a “static final boolean checkNodeprep = true” (or false for the ones who want to change this and then compile their own code), so the java compiler can optimize this during compilation.

“an application which create hundreds of new users per second” is really rare and I wonder if Openfire should support this. It seems that the DefaultUserProvider is not synchronized … one may want to do this.

Throwing exceptions is nodeprep fails may be even more bad than executing nodeprep as this needs a lot of resources.

So there are a lot of places where one could do some optimizations, dropping noeprep is imho not a good one.

LG