[4.x] Don't hang on a 103 response #7938
Conversation
yschimke
changed the title
| @@ -283,7 +285,10 @@ class Http2Stream internal constructor( | |||
| val open: Boolean | |||
| synchronized(this) { | |||
| if (!hasResponseHeaders || !inFinished) { | |||
| hasResponseHeaders = true | |||
| val status = headers[Header.RESPONSE_STATUS_UTF8]?.toIntOrNull() | |||
There was a problem hiding this comment.
Do you mind retargeting master, then cherry-picking this back to okhttp_4x ?
I’m a little surprised to see status codes handled at this layer
There was a problem hiding this comment.
I think it went here so I could make the minimal change.
I can try again to apply some of the changes directly, it's been a while, so it's a bit vague why it broken down.
There was a problem hiding this comment.
I’m still not a fan of handling status codes at the protocol layer. Revert this part and keep the fix for the infinite wait?
There was a problem hiding this comment.
This feels unfortunate. I really would like to handle this properly in 4.12.
| private var client = clientTestRule.newClient() | ||
| private var client = clientTestRule.newClientBuilder() | ||
| .followRedirects(false) | ||
| .build() | ||
|
|
||
| @Test | ||
| fun test103() { |
There was a problem hiding this comment.
Does master have this test?
There was a problem hiding this comment.
No, this is a manual test, see line 24. The problem is mockwebserver in 4.x doesn't have support for informational responses. So tests on master are better.
|
I'll check and update this weekend. |
Race condition means that a 103 might be considered to complete a request, such that the the actual headers are considered trailers.
#fixes #7936