New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is POST really require body? #751

Closed
pepyakin opened this Issue Apr 22, 2014 · 22 comments

Comments

@pepyakin

pepyakin commented Apr 22, 2014

Hi, i'm encountering same problem as with #745, but at this time with POST method.
I don't really care what specs saying about whether POST must have a body or not, but I definitely know, that some server APIs don't expect any body at all with POST method.

Let's not blame creators of such API's, but just admit it.
So, when dealing with such APIs I should use clumsy code like following one:

new Request.Builder()
  .url(requestUrl)
  .method("POST", null);
  .header("Content-Length", "0");

Maybe we should add "Content-Length: 0" header automatically if request's body is null?

@pepyakin pepyakin changed the title Is POST a really require body? Is POST really require body? Apr 22, 2014

@swankjesse swankjesse added the bug label May 4, 2014

@swankjesse swankjesse added this to the 2.0 milestone May 4, 2014

@swankjesse

This comment has been minimized.

Member

swankjesse commented May 4, 2014

We might fix this with code or by making it foolishly easy to get an empty post body.

@adriancole

This comment has been minimized.

Collaborator

adriancole commented May 5, 2014

Incidentally CallTest.postZeroLength would fail, if it didn't send a Content-Length: 0 header.. Can you verify that this is an issue on latest snapshot?

  @Test public void postZeroLength() throws Exception {
    server.enqueue(new MockResponse().setBody("abc"));
    server.play();

    Request request = new Request.Builder()
        .url(server.getUrl("/"))
        .method("POST", null)
        .build();

    executeSynchronously(request)
        .assertCode(200)
        .assertBody("abc");

    RecordedRequest recordedRequest = server.takeRequest();
    assertEquals("POST", recordedRequest.getMethod());
    assertEquals(0, recordedRequest.getBody().length);
    assertEquals("0", recordedRequest.getHeader("Content-Length"));
    assertEquals(null, recordedRequest.getHeader("Content-Type"));
  }
@adriancole

This comment has been minimized.

Collaborator

adriancole commented May 5, 2014

closing for now, as I think this is no longer current.

@adriancole adriancole closed this May 5, 2014

gaurav-fueled referenced this issue Apr 21, 2015

Never convert null into an empty request body.
This is a behavior change.
@1zaman

This comment has been minimized.

1zaman commented May 22, 2015

This is still an issue in 2.4.0 RC.

@swankjesse

This comment has been minimized.

Member

swankjesse commented May 22, 2015

@1zaman can you provide a 0-length body? I don't want to conflate null with empty.

@1zaman

This comment has been minimized.

1zaman commented May 22, 2015

@swankjesse: This update breaks my existing application which uses Volley and OkHttp, so I'm just reverting it for now. If you mean to enforce a body in POST requests, then this should be documented as a breaking change.

@SleeplessByte

This comment has been minimized.

SleeplessByte commented May 27, 2015

It is trivial to provide a zero length body with Retrofit;

For anyone getting here with retrofit, you can use the @Body annotation.

public interface MyService {
    @POST( "yourpath") R call( @Body String body )
}

myServiceInstance.call( "" );
@JakeWharton

This comment has been minimized.

Collaborator

JakeWharton commented May 27, 2015

I would call that pretty trivial actually.

On Tue, May 26, 2015 at 5:32 PM Derk-Jan Karrenbeld <
notifications@github.com> wrote:

It is not trivial to provide a zero length body with Retrofit;

For anyone getting here with retrofit, you can use the @Body annotation.

public interface MyService {
@post( "yourpath") R call( @Body String body )
}

myServiceInstance.call( "" );


Reply to this email directly or view it on GitHub
#751 (comment).

@SleeplessByte

This comment has been minimized.

SleeplessByte commented May 27, 2015

Yeah; I wrote not hard and forgot to replace the not.

@beshkenadze

This comment has been minimized.

beshkenadze commented May 27, 2015

@JakeWharton This is a crutch. If the server does not expect a parameter, it should not be.

@JakeWharton

This comment has been minimized.

Collaborator

JakeWharton commented May 27, 2015

The RFC for HTTP requires it and you are using an OkHttp that is newer by
numerous months than Retrofit. It's unsurprising that it enforces stronger
semantics.

On Wed, May 27, 2015, 1:14 AM Aleksandr Beshkenadze <
notifications@github.com> wrote:

@JakeWharton https://github.com/JakeWharton This is a crutch. If the
server does not expect a parameter, it should not be.


Reply to this email directly or view it on GitHub
#751 (comment).

@SleeplessByte

This comment has been minimized.

SleeplessByte commented May 27, 2015

The fact that servers accept non RFC requests is nothing new; I think explicitly sending an empty body isn't bad.

@JakeWharton

This comment has been minimized.

Collaborator

JakeWharton commented May 27, 2015

I agree but empty is not the same as absent. Empty is allowed and not the
problem faced here. You have absent bodies that are no longer being allowed
because they need coerced to empty in Retrofit.

On Wed, May 27, 2015, 1:44 AM Derk-Jan Karrenbeld notifications@github.com
wrote:

The fact that servers accept non RFC requests is nothing new; I think
explicitly sending an empty body isn't bad.


Reply to this email directly or view it on GitHub
#751 (comment).

@SleeplessByte

This comment has been minimized.

SleeplessByte commented May 27, 2015

Can the default behaviour not be to empty an absent POST body? Convention over configuration wise?

@JakeWharton

This comment has been minimized.

Collaborator

JakeWharton commented May 28, 2015

That's not what that phrase means. A POST per the HTTP spec needs the body.

On Wed, May 27, 2015, 6:58 PM Derk-Jan Karrenbeld notifications@github.com
wrote:

Can the default behaviour not be to empty an absent POST body? Convention
over configuration wise?


Reply to this email directly or view it on GitHub
#751 (comment).

@SleeplessByte

This comment has been minimized.

SleeplessByte commented May 31, 2015

Yes, I understood that. So give it (in okhttp) an empty one if none was given, instead of raising an exception?

I think it's good to require the body and raise the exception, but since it is a breaking change now, one could make it deprecated yadayadayada --

@slomo

This comment has been minimized.

slomo commented Nov 13, 2015

Where from the HTTP standard is it obvious that POST requires a body? As I see it RFC7320 says on page 27:

The presence of a message body in a request is signaled by a
Content-Length or Transfer-Encoding header field. Request message
framing is independent of method semantics, even if the method does
not define any use for a message body.

And later on on page 32 it specifies:

If this is a request message and none of the above are true, then the message body length is zero (no message body is present).

I couldn't find an restrictions for any request type/verb.

@ExplodingCabbage

This comment has been minimized.

ExplodingCabbage commented Dec 13, 2015

@slomo The best argument I can see for POSTs requiring a body comes not from RFC 7320 but from RFC 7231, where the POST method is defined. From the start of section 4.3.3:

The POST method requests that the target resource process _the
representation enclosed in the request
_ according to the resource's
own specific semantics.

(Emphasis is mine.)

From all the prior discussion of "representations" and representations being "enclosed" in request payloads earlier in the RFC, it is reasonable to infer that "the representation enclosed in the request" refers to its body.

So we have language in the definition of POST that seems to take for granted (but doesn't explicitly state) that a POST request must contain an "enclosed representation", and a whole bunch of earlier remarks in the document that seem to take for granted (but don't explicitly state) that "representations" are always things that exist in the body of a request or response.

Pretty unsatisfying!

Meanwhile, back in RFC 7230, there's this snippet:

A user agent SHOULD send a Content-Length in a request message when
no Transfer-Encoding is sent and the request method defines a meaning
for an enclosed payload body. For example, a Content-Length header
field is normally sent in a POST request even when the value is 0
(indicating an empty payload body). A user agent SHOULD NOT send a
Content-Length header field when the request message does not contain
a payload body and the method semantics do not anticipate such a
body.

Here there's an explicit distinction being made between methods like POST that define a meaning for the payload body, and others, like GET, that do not. In the former case, we're invited to think of sending request with no body as sending an "empty" body, and in the latter case we're invited to think of it as sending "no" body.

Of course, as far as bytes over the wire goes an "empty" body and "no" body are the same thing anyway (unless I'm mistaken about something?), so I don't think I'd've made the decision that the square chaps have made to enforce explicitly passing an empty body in the cases where a body is semantically required. (Shrugs.)

@robfletcher

This comment has been minimized.

robfletcher commented Jan 15, 2016

It's "trivially easy" to add a request body provided you're not using OkHttp under Retrofit, for example. I don't want to pollute my Retrofit API interface with a body parameter that's just going to confuse people using it. I tried to use an Interceptor on the OkHttpClient but apparently the check is done before the interceptor is invoked. That in itself seems odd. There seems to be a perfectly reasonable solution to what is a breaking change.

@dagronnell

This comment has been minimized.

dagronnell commented Feb 10, 2016

What about PUT? Right now it requires a request body. Should it?

@swankjesse

This comment has been minimized.

Member

swankjesse commented Feb 10, 2016

You can add an empty body.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment