First 2 bytes of http headers getting dropped on some http client

[jkyamog]

On some http client such as MS.net http client the headers does not start with CRLF. So the first 2 bytes of the headers get dropped always, which mangles the first header key.

Unsure how to create a unit test for it. However tested to work and does not break Chrome 31, Firefox 25 and IE 11. Removing the drop(2) fixes the issue with MS.net httpclient

[lightbend-cla-validator]

Hi @jkyamog,

Thank you for your contribution! We really value the time you've taken to put this together.

We see that you have signed the Typesafe Contributors License Agreement before, however, the CLA has changed since you last signed it.
Please review the new CLA and sign it before we proceed with reviewing this pull request:

http://typesafe.com/contribute/cla

[huntc]

Thanks for this. It should be relatively straightforward to create a unit test for the specific method you have changed. Would you be able to give that a go?

[jkyamog]

Yes I will try. On the app we use play, we have a helper that uploads a file. I would think that is too high level.

https://gist.github.com/jkyamog/8546327

A more appropriate test is a test that runs Multipart.multipartParser, pass a partHandler to it. I am unsure how I can put a fake request to it.

I will give it a try, any suggestion on how to do it would be great. Or an existing test/spec to give me hints would be great too.

[huntc]

There's a play/src/test/scala/play/api/mvc/ContentTypesSpec that can be extended. There's probably something quite low level to be tested on the multipartparser function. Thanks.

[richdougherty]

@jkyamog, take a look at some of the tests in https://github.com/playframework/playframework/tree/master/framework/src/play-integration-test/src/test/scala/play/it/http, especially the tests that use BasicHttpClient.

[jkyamog]

@richdougherty Sorry got side tracked in making a successful test. Thanks for the pointers. Furthest I got is here:

https://gist.github.com/jkyamog/8945955

So I am getting the expected result of the Either. Its a Right but the data parts are empty

Right(MultipartFormData(Map(),List(),List(),List()))

Would it be alright if I visit you sometime this week? I am just 2 streets away anyway.

[jkyamog]

@richdougherty ok done the test. Did not use the BasicHttpClient as it was too low level. I was actually close on the gist I have above, only problem was my test payload was incorrect. So I had the correct Right(MultipartFormData), but simply would not parse as I have doubled the headers, was not using CRLF delimiters on the headers. After installing IDEA today, I finally got a debugger to attach.

Sorry for the long wait, been very busy. I will still come by this Wed.

[jroper]

LGTM. Would you mind squashing your commits, this will help us if we ever decide to backport this fix, as it means we can't forget to include tests with the fix.

[jkyamog]

Squashed 2 commits into 1. Thanks.

[huntc]

Thanks - I wonder why we ever removed the first two bytes...