Summer of Code: Checkstyle to the rescue!


Today I added some checkstyle rules to PGPainless.Checkstyle is a gradle plugin, which checks the source code for style violations.

Some say, strict checkstyle rules are unnecessary and that it is annoying to be held back from pushing a commit to the master branch only to fix “style issues” for half an hour. I must say, in the beginning I thought the same way. I was annoyed thinking “why does it matter, if a line comment ends with a period or not?” But after being forced to give it a try when I first became a contributor to the Smack project, I became a fan of it. In the beginning I had to often recommit my changes because they broke the checkstyle rules. For example I often forgot to leave spaces between mathematical operators. I would write “i = 5+5;” instead of “i = 5 + 5;”. But after some amount of time, I got less and less warnings.

I adopted most of the (honestly *very* strict) rules in Smacks rule set to my own coding style. I like how it automatically leads to cleaner, more uniform code (not that it is impossible to write garbage with it of course). For that reason, I decided to put those rules into place in PGPainless today (I only left one rule out, because who the hell cares about the alphabetical sorting of imports???).

At some point, PGPainless will be released as a maven artifact. In preparation for this historical event, I bought the domain pgpainless.org. For now it is just a forwarding to the PGPainless git repository, but I will likely setup a small website with documentation etc. at some point.

During my testing of Smacks OX implementation, I came across an interesting problem. When a user queries a PubSub node in Smack, Smack first does a disco#info query on that node to determine, whether it is a LeafNode or a CollectionNode. This normally works fine. However, it becomes more and more popular to make use of the PubSub access model ‘open’. The open access model makes a PubSub node accessible to entities (like other users) which are not in the contact list of the user. This enables the use of OMEMO in group chats, where not every participant is in your contact list for example.

The problem is that a server which allows access to open PubSub nodes, does not necessarily allow the disco#info query. The question is: Should disco#info queries on open PubSub nodes be allowed or not? An argument against it is, that it might allow “jid-harvesting”. An attacker might use disco#info queries on open PubSub nodes in order to determine, whether the user exists or not. This is a bad thing, because it allows spammers to collect the Jabber IDs of potential victims. On the other hand however, the attacker could simply do a direct PubSub query on the open node and the result would be the same. The benefit of allowing disco#info queries would be, that you can in fact determine the node type.
For now my mail to the standards mailing list remained unanswered, but I think that there should be a well defined expected behavior for this edge case.

For now I worked around the issue by using Javas reflections to access the LeafNode constructor directly, avoiding the disco#info query.

Other than that, I didn’t get a whole lot done this week. Unlike the demotivating week though, this time the reason was primarily exciting new hardware 😀

Happy Hacking!

, ,

Leave a Reply

Your email address will not be published. Required fields are marked *