okhttp icon indicating copy to clipboard operation
okhttp copied to clipboard

Support HTTP status 102 Processing

Open rfc2822 opened this issue 8 years ago • 11 comments

There's a HTTP interim status code 102 Processing. It's defined in RFC 2518 and can be found in the HTTP Status Code Registry.

Funnily, it has been dropped in the revised WebDAV RFC 4918, so I don't know whether it's worth dealing with it (RFC 4918 is from 2007 and 2518 from 1999!). However, it's still in the status code registry, and I have just encountered a quite large WebDAV service which uses this status code (original case).

It could be exactly treated as status code 100, so I guess adding it in Http1xStream would be enough?

// HTTP_PROCESSING: 102

 if (statusLine.code != HTTP_CONTINUE || statusLine.code != HTTP_PROCESSING)
// instead of
 if (statusLine.code != HTTP_CONTINUE)

What do you think about that?

rfc2822 avatar Jan 17 '16 15:01 rfc2822

Works for me.

swankjesse avatar Jan 17 '16 17:01 swankjesse

Cool. :sunglasses: RFC 2616 even says:

A client MUST be prepared to accept one or more 1xx status responses prior to a regular response, even if the client does not expect a 100 (Continue) status message. Unexpected 1xx status responses MAY be ignored by a user agent.

So maybe not only 102 should be added, but all 10x status codes. But as RFC 2616 is only a rough recommendation, I don't know whether this would be wise.

rfc2822 avatar Jan 17 '16 17:01 rfc2822

The spec we care about is here: http://tools.ietf.org/html/rfc7231#section-6.2

I think it’s a good idea to generalize our current 100 behavior to handle all 1xx codes.

swankjesse avatar Jan 17 '16 17:01 swankjesse

There's an issue tracking 100's as #1337.

JakeWharton avatar Jan 18 '16 03:01 JakeWharton

Closing as a dupe of #1337 and renaming it to say "1XX".

JakeWharton avatar Feb 03 '16 23:02 JakeWharton

Just came across this. See my comment in issue #1337. It was closed as a dupe of #675, which only added support for response code 100, but not for 102 (which was the original topic for this one).

guillerodriguez avatar Jun 27 '18 09:06 guillerodriguez

Agreed!

swankjesse avatar Jun 30 '18 11:06 swankjesse

Hi, I would like to tackle this issue. I think it would be nice to do it in a few steps:

  • document (in tests) current behavior of MockWebServer while using EXPECT_CONTINUE and CONTINUE_ALWAYS Socket Policies. Currently, there is one test based on HttpUrlConnection mockwebserver3.MockWebServerTest#http100Continue, which I think is not enough because it's not able to check that MockWebServer returned 100 (actually in this test it doesn't because it has no SocketPolicy set). I could check it by using java.net.Socket with thin wrapper and make String assertions on result
  • Add tests for what impact have 1xx responses on readTimeout. For example by allowing MockResponse to have follow-up server.enqueue(new MockResponse().setBody("response").withFollowingResponseForSameRequest(new MockResponse(), Duration.ofSeconds(1)))

  • Implement 102 Processing support and test it by using above method

What do you think about that?

nowkarol avatar Oct 25 '21 19:10 nowkarol

@nowkarol You seem to have put a lot of thought into how to make the change. Please submit a PR.

We normally discourage external contributions because we try to keep the scope of the library tight and have very opinionated ideas about the APIs and so on. It can be frustrating for new contributors.

So please take any feedback with this in mind :) And thanks for offering and putting so much thought into how to do it correctly.

yschimke avatar Nov 27 '21 11:11 yschimke

@yschimke Sure, will keep that in mind. So will try with very small chunks. First one is about testing current MockWebServer behavior while using EXPECT_CONTINUE. Probably some of behaviors should be changed to comply with RFC. After that I will be able to test and implement enqueuing of follow-up responses needed to test 102 Processing support.

nowkarol avatar Dec 15 '21 22:12 nowkarol

@yschimke - sorry for not checking before and submitting a dupe.

That said, this really needs to be treated as "bug", not an "enhancement".

reschke avatar Sep 08 '22 10:09 reschke