Export to GitHub

mockwebserver - issue #9

Accepting any request with content length header set


Posted on May 21, 2013 by Happy Dog

spring android (and I believe spring rest template not only for android ) adds a content length header to ANY request. Even GET methods will have a body set.

And this is actually not compatible with mock web server. I really agree that this is a bad practice they adopted but, that the way their code is :

http://grepcode.com/file/repo1.maven.org/maven2/org.springframework.android/spring-android-rest-template/1.0.0.RELEASE/org/springframework/http/client/SimpleBufferingClientHttpRequest.java#SimpleBufferingClientHttpRequest

Their code is really complex (bloated ?) and will be hard to change. On Android, requests can be of 2 kinds : one for pre-FROYO and one for post-FROYO. Moreover the maintainer of the lib doesn't seem to go deep in the core of the lib anymore.

In our current app, this makes our tests fail. It simply crashes the mock server we use and all subsequent tests will fail. We tried hard to hack spring android to change this, but it's not possible.

Would you mind to add a parameter so that mockwebserver is less conservative and accepts this kind of requests...

I am not used with SVN at all, I use git/github usually. And I don't know how to submit a patch to you about this. So, I hope you will forgive this but I will submit a working patch and 2 tests for it attached to this issue.

** Actually, if you could release this to maven central, it would tremendously help our project. **

Let me know if there is anything I coud do to help with it.

Thanks in advance, and really, mock web server is a great piece of software, damn useful for testing real apps. Thx for writing and maintaining it. :)

Stéphane PS : I am sorry, I lost your formatting when I wrote the patch, but there are around 100 characters that changed + tests.

Attachments

Comment #1

Posted on May 22, 2013 by Massive Bear

I'll take a look.

Comment #2

Posted on May 22, 2013 by Happy Dog

Thank you very much. We are really eager to clean our code as our workaround was really ugly.. but with the patch, it oughta be ok.

-- St�phane NICOLAS, OCTO Technology D�veloppeur & Consultant Android / Java .......................................................... 50, Avenue des Champs-Elys�es 75008 Paris +33 (0)6.26.32.34.09 www.octo.com - blog.octo.com www.usievents.com ...........................................................

Comment #3

Posted on May 25, 2013 by Massive Bear

I finally took a look at this code, and it's more change than I'd expected.

Do you need to support GETS with a content length other than 0 ? It's trivial to support that, we just change this line:

    if (contentLength != -1) {

to this:

    if (contentLength != -1 && contentLength != 0) {

Will that work for you?

Comment #4

Posted on May 25, 2013 by Happy Dog

I can't say. But it could work.

Would you mind to release a snapshot so that I can try it out ?

S Le 25 mai 2013 21:26, a �crit :

Comment #5

Posted on Jul 11, 2013 by Grumpy Ox

I just ran into this when trying to test a Retrofit client, which also sets contentLength == 0 on GET requests. Has this patch been made?

Comment #6

Posted on Jul 11, 2013 by Happy Dog

I am sorry not to have returned to you jessie. But the issue is also important for Spring Android client. I can't tell if your solution is the right one, but I provided two tests, if the test pass, then the feature is in :)

St�phane

-- St�phane NICOLAS, OCTO Technology D�veloppeur & Consultant Android / Java .......................................................... 50, Avenue des Champs-Elys�es 75008 Paris +33 (0)6.26.32.34.09 www.octo.com - blog.octo.com www.usievents.com ...........................................................

Comment #7

Posted on Jul 12, 2013 by Massive Bear

neal, snicolas: it's a bug in the HTTP client to set a Content-Length on GET requests.

neal: I can't reproduce the problem with retrofit. Are you using the latest retrofit? snicolas: can you report this as a bug to Spring Android?

Comment #8

Posted on Jul 12, 2013 by Grumpy Ox

Using UrlConnectionClient with Retrofit works. I was using the AndroidHttpClient which exhibits the bug. I agree, it is not a bug in MockHttpServer.

Comment #9

Posted on Jul 13, 2013 by Happy Dog

I agree with you that it's a bug in spring android, and that mock web server is doing things right.

Nevertheless Spring Android is a large program, less maintained than mock web server.

This change would impact SpringAndroid at a very low level and I doubt it will ever be changed.

Nevertheless, a nice solution could be to patch mock webserver and make this feature optional. The normal behavior would be not to accept length headers in get requests and a boolean could allow it.

S. Le 12 juil. 2013 14:28, a �crit :

Comment #10

Posted on Jul 13, 2013 by Massive Bear

@neal can you provide me with a test case that includes "Content-Length: 0" ? If its Retrofit or Android proper I can try to get it fixed.

@snicolas: just tell the SpringAndroid folks. I bet they'll fix it. And if they don't, you should quit SpringAndroid altogether. You shouldn't depend on projects that don't fix bugs.

I'm not going to fix mock web server to malformed HTTP requests. If you want to fork it to do so, it's open source! But I like that it's strict.

Comment #11

Posted on Jul 14, 2013 by Quick Giraffe

Hi @limpbizkit (jessie ?) ,

I agree with your point of view about introducing a patch to fix a bug in some other library. Nevertheless, as I told you, Spring Android is not so actively maintained and the bug has to be fixed deeply in Spring Android, maybe even in harmony library.... I really doubt that it will be fixed.

But let see : https://jira.springsource.org/browse/ANDROID-136

Comment #12

Posted on Jul 15, 2013 by Massive Bear

Thanks @steff.

Status: WontFix

Labels:
Type-Defect Priority-Medium