Skip to content
Snippets Groups Projects

http: resource: drain reply payload on redirection/error

Closed François Cartegnie requested to merge fcartegnie/vlc:mr070901 into master
2 unresolved threads

Payload is not read for non 200 replies to disconnect faster.

This prevents proper reuse of connections, and also creates bogus replies on 401 Authentification where previous payload is merged with auth reply headers.

[00007f6e98182488] http generic debug: incoming response:
<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN">
<html><head>
<title>401 Unauthorized</title>
</head><body>
<h1>Unauthorized</h1>
<p>This server could not verify that you
are authorized to access the document
requested.  Either you supplied the wrong
credentials (e.g., bad password), or your
browser doesn't understand how to supply
the credentials required.</p>
</body></html>
HTTP/1.1 206 Partial Content
Date: Fri, 03 Sep 2021 14:14:04 GMT
Server: Apache/2.4.48 (Fedora) OpenSSL/1.1.1k
Last-Modified: Wed, 19 Jul 2017 15:46:20 GMT
ETag: "df-554ad88844b00"
Accept-Ranges: bytes
Content-Length: 223
Vary: Accept-Encoding
Content-Range: bytes 0-222/223
Content-Type: application/vnd.apple.mpegurl

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
296 296 return vlc_http_stream_read(m->payload);
297 297 }
298 298
299 void vlc_http_msg_drain(struct vlc_http_msg *m)
  • 122 122 if (res->cbs->response_validate(res, resp, opaque))
    123 123 goto fail;
    124 124
    125 if(status >= 300 && vlc_http_msg_get_size(resp) != 0)
    126 vlc_http_msg_drain(resp); /* will not use data, discard human response payload */
    • This is terrible. We want to reset the stream (at least unless it has been entirely received already), not wait however long for the useless and potentially large error response to be received.

    • We have latency issues due to TLS sessions. Not reusing connection is likely not better than reading error pages.

    • With TLS, the current code normally just send two packets, with zero waiting. Worst case, the ancient server lacks HTTP/2 and we have one RTT to make a new TLS connection with TCP fast open and TLS false start.

      With the patch, we have to wait arbitrarily long for the current stream to end.

      So this patch is much worse in both optimistic and pessimistic cases.

    • Please register or sign in to reply
  • mentioned in issue #26084 (closed)

  • Johannes Kauffmann mentioned in merge request !591 (closed)

    mentioned in merge request !591 (closed)

  • Please register or sign in to reply
    Loading