« back

Pitfalls of Passing an InputStream to Multiple Buffers

18 Feb 2013

While integration testing my RequestParser for my java server, I was noticing some strange behavior.

In order to mock out the input stream with a fake request, I was abstracting up a class and relying on the java reader class. That way, I would use a StringReader in my test as "in" and when I was really using my RequestParser I could pass an InputStreamReader. This strategy was born of not having to mock a BufferedReader. Here is an example...

1 @Test public void itShouldBeAbleToReturnTheFirstCharacterAfterBlankLines() throws IOException {
2         StringReader in = new StringReader("\r\n\r\nmy = data value1 = hello\n");
3         RequestStore requestStore = new RequestStore();
4         RequestParser requestParser = new RequestParser(in, requestStore);
5         assertEquals("m",requestParser.checkForBlankLines(in));
6     }

Then, inside each method I would wrap my input in a BufferedReader. I had some good red, green, refactor cycles going on with my unit tests. I finished them up, did a commit and a push, and then realized that I was not testing the main(not main as in it was actually called main but main it that it was the only method that I would call from outside the class) It was a composed method, delegating all the work to the other methods. Everything else worked, so this should work, right? That's what I was thinking.

WRONG!!!!!!

I was surprised and went through my other code to make sure that my tests were good. I couldn't figure out what was going wrong. Then I did what every developer that feels like they may be going slightly insane trying to track something down might do. That's right, System.out.println(); , baby. I System.out.println(); the bejeezus out of that code. I finally reached the point that I was feeling 97% sure that the problem was not with my code but with the input. I wasn't sure if it was a limitation of the way I was using StringReader, or if it there was a problem with the BufferedReader. After this fairly confident conclusion, I wasn't sure how to test my hypotheses. It was Sunday and I decided that after a weekend of java server, I would take the rest of the afternoon off. I was eager to talk to Doug in the morning.

After showing Doug what I had found, he suggested that we test it out. He was able to show me how to write a test to figure out which one of the pieces was the culprit. It was a really simple and effective test to get to the heart of the matter. I had not thought of that and thought it was really cool.

It turns out that you should not wrap input with more than one buffer. There is a default size to the buffer(which is quite large (8192 bytes according to the JDK source)), and when you use it, the input will be read in and when you go to read again, depending on what is already in the buffer, your result could very well be in a different spot than what you were expecting. Since my data was quite short and all fit in the default buffer size, it would return me nothing.

The solution was to remove the responsibility of creating the BufferedReader from all the methods that were doing so and to actually pass the BufferedReader into the class when it is created. Then in my tests, I still use the StringReader but I wrap them there inside the test in the BufferedReader and pass that in. We can look at the above example again...

1 @Test public void itShouldBeAbleToReturnTheFirstCharacterAfterBlankLines() throws IOException {
2     StringReader request = new StringReader("\r\n\r\nmy = data value1 = hello\n");
3     BufferedReader in = new BufferedReader(request);
4     RequestStore requestStore = new RequestStore();
5     RequestParser requestParser = new RequestParser(in, requestStore);
6     assertEquals("m",requestParser.checkForBlankLines(in));
7 }

Just when you thought that was it, I'll give you a bit of bonus information as well. You're welcome. :)

It turns out that another reason you do not want to use multiple buffers is that if the the wrappers allow look-ahead, an attacker can exploit this. Someone could actually redirecting System.in (from a file) or by using the System.setIn() method to redirect System.in. Pretty much any input stream that supports nonblocking buffered I/O is susceptible to this form of misuse.

WOAH!

And that's why you don't use multiple buffers.

comments powered by Disqus