Tokartronica: The Life and Learnings of Tokes

Beware of Hash Cookies

2008-05-30T18:19:00.003+10:00

As posted on the Atlassian Developer Blog:

During my latest stint as the JIRA Developer on Support, I came across an issue where a customer reported that the Remember Me functionality of JIRA wasn't working - but only for some users. In his instance, he had two users: wmlalai and lagonil - for now, let's call them B1 and B2. For B1, the Remember Me functionality worked as expected; for B2, not so.

Curious to the cause of this problem, I loaded up the customer's data in a local instance, and sure enough I was able to reproduce it. Monitoring the cookies within the browser to make sure they were being saved correctly, I noticed that upon the first login, the cookie was present in the browser for both users. When I closed the browser and restarted it, the cookie again was still there. But, when I navigated to JIRA, B2's cookie was being deleted. This was accompanied by an exception in JIRA's logs:

2008-03-07 15:46:32,470 http-8090-Processor2 DEBUG [atlassian.seraph.cookie.EncryptedCookieEncoder] Invalid password cookie submitted, trying insecure
java.lang.RuntimeException: javax.crypto.IllegalBlockSizeException: Input length must be multiple of 8 when decrypting with padded cipher
 at com.atlassian.seraph.util.EncryptionUtils.decrypt(EncryptionUtils.java:77)
 at com.atlassian.seraph.cookie.EncryptedCookieEncoder.decodePasswordCookie(EncryptedCookieEncoder.java:43)
 at com.atlassian.seraph.auth.DefaultAuthenticator.decodeCookie(DefaultAuthenticator.java:393)
 at com.atlassian.seraph.auth.DefaultAuthenticator.getUserFromCookie(DefaultAuthenticator.java:245)
 at com.atlassian.seraph.auth.DefaultAuthenticator.getUser(DefaultAuthenticator.java:221)

Why would the cookie be the wrong length? I set a breakpoint in the DefaultAuthenticator.getUserFromCookie() method in Seraph, where the cookie is retrieved and decoded. When attempting to log in with B2, I observed the following value for the seraph.os.cookie:

lLaNPRBsPajwpMtCaoXPpImn

as compared to the actual value stored in the cookie in the browser:

lLaNPRBsPajwpMtCaoXPpImn>Ca7ic

i.e., when retrieving the cookie value from the request, it was being truncated at the '>' character.

Who stole (part of) the cookie from the cookie jar? I sniffed the network traffic to ensure the browser was sending the full value, and it was, so this led me to believe that the application server was the fiend. After a bit of searching, I found that others had been the victim of Tomcat's cruel appetite. What confused me more was Apache's response:

That is because those characters are illegal in version 0 cookies (as per the spec). You need to use version 1 cookies.

I had read the specs they were referring to: "Version 0" and "Version 1", and neither made mention of illegal characters in cookie values other than semi-colon, comma and whitespace. Regardless, I tried their suggested solution of switching to "Version 1" cookies (by using the setVersion() method on the Cookie object when constructing it), and it did work. Note however that, when I tested the issue on Orion and Resin, they did not experience this problem, even though the cookies did have the "illegal" characters in them, AND the Cookie objects were "Version 0". Go figure.

Why do we use these characters in our cookies anyway? Interestingly, this problem only appeared after the release of JIRA 3.12, when updates were made to Seraph code to increase the security of the seraph.os.cookie, by taking the hash of the username and password and encrypting it a little. And sometimes, this hash happens to contain those characters. What's cool is that it seems to be reproducible, so if you fire up your own instance of JIRA running on Tomcat, and create a user named lagonil, you should get the same problem. We're hoping to resolve this issue very soon; you can track it here.

So let that be a lesson to you, kids - if you're ever going to use hash cookies, make sure you know what's in them!

Know your true @@IDENTITY

2007-08-12T23:44:00.000+10:00

When developing a data access layer for persisting information to a database, one of the commonly used techniques is to implement CRUD operations (Create, Read, Update, Delete) by using stored procedures. When writing the "create" stored procedure, I tend to use the following very simple pattern:

CREATE PROCEDURE Employee_Insert (
@EmployeeId int OUTPUT
, @FirstName varchar(50)
, @LastName varchar(50)
, @DateOfBirth datetime
) AS
BEGIN
INSERT INTO Employee (
FirstName
, LastName
, DateOfBirth
)
VALUES (
@FirstName
, @LastName
, @DateOfBirth
)

SELECT @EmployeeId = @@IDENTITY
END

In this example, the Employee table's primary key is the EmployeeId, and it is also an identity column (meaning key values are generated by the database). After inserting the new Employee record into the table using the first statement, the second statement of the stored procedure saves the identity value that was used for the new record into the @EmployeeId output parameter, so that the data access layer can use it in some meaningful way. The way to find out the identity value that was used in the latest INSERT statement is simple - it's stored in the special @@IDENTITY variable, which is maintained by the database engine. This is very convenient, as it saves you from having to query the table again. But - is it really this simple? Does the @@IDENTITY variable always return the identity value from your last INSERT statement?

The answer is "not necessarily", in SQL Server 2005 at least. From the @@IDENTITY page in SQL Server 2005 Books Online:

"If the statement fires one or more triggers that perform inserts that generate identity values, calling @@IDENTITY immediately after the statement returns the last identity value generated by the triggers."

This means that if are using triggers on your Employee table which fire on insertion, and if those triggers in turn insert records into another table which also has an identity column, the result of the @@IDENTITY variable is the identity value used in the second insertion, and not the first, which is in most cases undesirable.

I came across this rather fun issue in an application I've been working on. From my data access layer, I was inserting several different records in a transaction, where one record had a foreign key relationship with another record's identity value. The way to save these new records correctly is to save the "parent" record first, then read the identity value returned from the stored procedure, update the corresponding field in the "child" record, and then insert the child record. I was getting unexpected results however, where my child records were not being created because of a violation of the foreign key constraint. After debugging, I found that following the insertion of the parent record, I was getting back identity values that didn't make sense - they were not even close to the previous identity values stored in the table.

On closer inspection of the database schema, I realised that the table I was inserting the parent record into had a trigger on it, who's purpose was to insert new records into a logging table which, sure enough, had an identity column in it. So when running the SELECT statement, the stored procedure was actually returning the identity value used in the logging table, which explains the violation of the foreign key constraint!

So now that we know what the issue is, how do we fix it? Once again, trusty SQL Server Books Online has the answer - use SCOPE_IDENTITY() instead of @@IDENTITY.

@@IDENTITY and SCOPE_IDENTITY return the last identity value generated in any table in the current session. However, SCOPE_IDENTITY returns the value only within the current scope; @@IDENTITY is not limited to a specific scope.

What this means essentially is that SCOPE_IDENTITY will always return the last identity value used in an INSERT statement that you called directly; INSERT statements nested inside triggers, or even other stored procedures which are called from the currently executing stored procedure, are considered to be in a different scope. Because of this, my advice would be to always use SCOPE_IDENTITY() when writing the "create" stored procedures for your CRUD operations, because 99% of the time, when you want the last used identity value, you're usually referring to the INSERT statement that you can actually see right in front of you.

So remember - Know your true @@IDENTITY! Hope this post saves someone out there a little pain. :)

- Tokes

LogScrobbler 0.15 released!

2007-07-23T23:19:00.001+10:00

I have recently spent some time on my first true open source project - LogScrobbler. LogScrobbler is a little tool written by Tim Geiges that allows you to sync music tracks scrobbled from Rockbox to your Last.fm account. The app is written in C# and standard Winforms.

Last week, Tim and I released version 0.15 - the first version since I began working on the project. In this version, I focused mainly on cleaning up the code which was still very prototype-y, and removing some redundant features. I also gave the front end a bit of a face lift by simplifying the interface and standardising the look and feel of the controls. I don't get to do a lot of programming using Winforms in .NET, so this was a cool project to work on. I think in an upcoming release I will refactor the code a bit further and write a command-line interface to the app, just to add a bit more value.

Working on this project has also got me wanting to work on more open source projects in the future. I might check out the Rockbox code next and see if I can make some slight improvements to the scrobbling/Last.fm module. Rockbox also seems to make my iRiver consume more power than I remember when compared to the original firmware - so that could be another thing to look into.

If you're interested in checking out the LogScrobbler code or giving the app a try, just head over to the project homepage hosted on Google Code, or you can click here to download the latest version directly. Of course, if you come across any bugs, you should let me know by creating an issue in the project issue tracker (quite a handy feature of Google Code!)

Small victories - how I saved myself a few hours of work

2007-06-24T21:55:00.000+10:00

Over the past few weeks at work, I've been working on an internal tool for doing a little financial data entering and reporting. For reasons I won't go into, this tool needed to be developed rapidly, but given that this company's main development platform is ASP.NET, rapid development would not be a problem. The thing about rapid development though, is that requirements often "pop up" at odd stages in your development cycle (someone forgets to mention that this page is meant to do that, or whatever), so it's important that your design decisions are sensible without going overboard (due to time constraints, complexity of the problem, etc).

One such design decision, which I think is very important in creating ASP.NET applications, is creating a base class (or several base classes) for all the pages in your application to inherit from which captures at the very least a high-level purpose of the page. I think for people who are new to ASP.NET, this might get overlooked from time-to-time, because when you add a new page to your project, it automatically creates the class files for you (which inherit directly from System.Web.UI.Page), and also because application development in ASP.NET tends to be "designer-driven" - a lot of tutorials on the web and a lot of the improvements made to the ASP.NET framework seem to focus on "Look Ma, I didn't need to write any lines of code!". The fact that people tend to miss this design decision is also evidenced by the fact that in the last couple of ASP.NET projects I worked on, none had implemented this design. But the benefits can be really crucial, especially when your project is on a fixed schedule, and above all - it's just good OO practise to do it this way.

I'll give you an example of this design in practise, and how it helped me save a few hours of work. You might find this really trivial, but I was just happy at the simplicity of it all, and if I can save a few people a few hours of work, all the better. So in this application, there are several pages (somewhere between 10 and 20) of data entry, which all follow a similar kind of design, but the specifics of the data entry are distinct between each page. All pages use a GridView control to display currently entered records and to allow the entry of new records, but the columns of data vary between each page. The "action" columns (containing Add, Edit, Update, Delete buttons) are consistent across all pages. So far so good. Now for the conflict - a change request comes along stating that when a certain condition is met, the user is to be denied access to data entry. This can be implemented in several ways, but the one that makes the most sense is to hide the action columns of the GridView control in each data entry page. To do this for one page, you would have to do something like this in your Load or PreRender event:

bool dataEntryCondition = ...;
// assuming that action columns are at the 7th and 8th indexes
myGridView.Columns(7).Visible = dataEntryCondition;
myGridView.Columns(8).Visible = dataEntryCondition;
...

I hate having to write code like this, because as soon as you re-order the columns in your GridView, it breaks. What's scary is, I've seen this kind of thing done a lot. A better solution would be something like this:

bool dataEntryCondition = ...;
for each (CommandField actionColumn in myGridView.Columns)
{
actionColumn.Visible = dataEntryCondition;
}

This is definitely better, but you can't just assume all CommandFields in your GridView are going to have this logic applied to them (well, you could assume that if you were implementing this for a single page, but the whole point is that we're thinking generally here... it will all come together soon). Also, in my case, the columns in question weren't actually instances of CommandField; because they required special formatting, I had to use TemplateFields instead. So, as you probably figured out by now, what I did was create my own class of column which inherits from TemplateField, then create a very lightweight interface for "toggle-able" fields. The reason for creating a custom field class and an interface was because if I ever needed another kind of custom field that extended from CommandField, I would still be able to get my toggle-able functionality. The declarations looked something like this:

interface IToggleField
{
public void toggleVisibility(bool visibleCondition);
}

class MyTemplateField extends TemplateField implements IToggleField
{
public void toggleVisibility(bool visibleCondition) { this.Visible = visibleCondition; }
}

You would then need to modify your GridView in the design view to use this new type of column[1]. And to toggle, just slightly modify the previous attempt to use this new class:

bool dataEntryCondition = ...;
// all columns in a GridView inherit from DataControlField
for each (DataControlField actionColumn in myGridView.Columns)
{
if (actionColumn.GetType() is IToggleField)
((IToggleField) actionColumn).toggleVisibility(dataEntryCondition);
}

Much better, yes? You're probably thinking "What's the big deal?". Well the big deal is, I have to make this change across 10-20 pages. Changing the GridView columns definitions to use this new class has to be done "manually" (because I shamefully did not use a custom GridView class for my data entry grids, but let's just ignore that little fact for now), but this is easy enough because the columns in question have the exact same definition across all pages (they were copy-pasted :P) which makes it easy to do a search-replace. But, (this is where it all comes together), because I made the easy design decision of all my data entry pages inheriting a DataEntryPage class, adding in the logic to toggle the columns visibility all of those pages, which could have been painful, is really simple! All I had to do was create a single function to do the above procedure, and call it in the page event of my choice from within the DataEntryPage class, which effectively registers the call in all implementers' events. And the result is clean, simple code, the way it should be in OO programming.

It's because of situations like this, that if I continue doing ASP.NET projects, I will make sure that whoever my development team is knows about these fundamental design choices. If my current experiences are anything to go by, I would suggest that a fair portion of the ASP.NET developers out there probably would have missed this solution. Obviously, it would had been harder to implement had the project been larger and further down the track in development (requiring more refactoring time), but as they always say - you'll never know whether someone's going to have to look at your code in the future, so in my opinion if you can afford to spend the time to refactor the code, it's definitely worth it.

I hope that this hasn't been too boring! It feels good to write about this though, even if it was a trivial problem. Hopefully, the problems I encounter in the future will be more difficult ;)

- Tokes

[1] - for details on how to use custom fields in your pages, and for a more in-depth look on how to make custom fields that actually do complex things, check out this article at MSDN Magazine: Custom Data Control Fields.

First Post

2007-06-21T15:26:00.000+10:00

So, I have finally created a blog. I'm not quite sure what is going to go on here yet. My initial motivation was to have a space where I could talk about the technical challenges I come across at work or through my own exploration, but I guess every now and then I might post some personal stuff too.

The name of the blog (Tokartronica) came to me randomly... I quite like it, it kinda sounds like a technical, robotic, musical instrument.