dart-lang / sdk

The Dart SDK, including the VM, dart2js, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
9.97k stars 1.54k forks source link

Add support for the Trailer HTTP header to the Dart HttpClient #54162

Open TijnvandenEijnde opened 7 months ago

TijnvandenEijnde commented 7 months ago

Problem

I am trying to make a connection with the following URL: https://www.popularmechanics.com/rss/science/?src=rss. In my browser, everything seems fine. However, when I try to retrieve the URL using .get() function from the http package. I get the following exception: ClientException: Failed to parse HTTP, 115 does not match 13, uri=https://popularmechanics.com/rss/science/?src=rss.

Here is a simplified version of my code:

import 'package:http/http.dart' as http;

void main() {
  final client = http.Client();

  final Uri url = Uri.https('popularmechanics.com', '/rss/science/', {
    'src': 'rss'
  });

  try {
    client.get(url);
  } catch (e) {
    print(e);
  }
}

Running the code will give the following:

Unhandled exception:
ClientException: Failed to parse HTTP, 115 does not match 13, uri=https://popularmechanics.com/rss/science/?src=rss
#0      IOClient.send (package:http/src/io_client.dart:121:7)
<asynchronous suspension>
#1      BaseClient._sendUnstreamed (package:http/src/base_client.dart:93:32)
<asynchronous suspension>

Process finished with exit code 255

I thought that it might have something to do with the http package so I tried the dio package as well. However, the same problem still persists:

import 'package:dio/dio.dart';

void main() {
  final client = Dio();

  try {
    client.get('https://popularmechanics.com/rss/science/', queryParameters: {
      'src': 'rss'
    });
  } catch (e) {
    print(e);
  }
}

Running the code will give the following:

Unhandled exception:
DioException [unknown]: null
Error: HttpException: Failed to parse HTTP, 115 does not match 13, uri = https://popularmechanics.com/rss/science/?src=rss
#0      DioMixin.fetch.<anonymous closure> (package:dio/src/dio_mixin.dart:507:7)
#1      _FutureListener.handleError (dart:async/future_impl.dart:180:22)
#2      Future._propagateToListeners.handleError (dart:async/future_impl.dart:858:47)
#3      Future._propagateToListeners (dart:async/future_impl.dart:879:13)
#4      Future._completeError (dart:async/future_impl.dart:655:5)
#5      _SyncCompleter._completeError (dart:async/future_impl.dart:63:12)
#6      _Completer.completeError (dart:async/future_impl.dart:27:5)
#7      Future.any.onError (dart:async/future.dart:618:45)
#8      _RootZone.runBinary (dart:async/zone.dart:1666:54)
#9      _FutureListener.handleError (dart:async/future_impl.dart:177:22)
#10     Future._propagateToListeners.handleError (dart:async/future_impl.dart:858:47)
#11     Future._propagateToListeners (dart:async/future_impl.dart:879:13)
#12     Future._completeError (dart:async/future_impl.dart:655:5)
#13     Future._asyncCompleteError.<anonymous closure> (dart:async/future_impl.dart:745:7)
#14     _microtaskLoop (dart:async/schedule_microtask.dart:40:21)
#15     _startMicrotaskLoop (dart:async/schedule_microtask.dart:49:5)
#16     _runPendingImmediateCallback (dart:isolate-patch/isolate_patch.dart:118:13)
#17     _Timer._runTimers (dart:isolate-patch/timer_impl.dart:405:11)
#18     _Timer._handleMessage (dart:isolate-patch/timer_impl.dart:429:5)
#19     _RawReceivePort._handleMessage (dart:isolate-patch/isolate_patch.dart:184:12)

Process finished with exit code 255

Cause

The server is sending a rarely used HTTP header called Trailer.

Connection: keep-alive
Content-Type: text/xml; charset=UTF-8
Expires: Sun, 26 Nov 2023 23:05:57 GMT
Pragma: public
Referrer-Policy: no-referrer-when-downgrade
Accept-Ranges: bytes
Date: Sun, 26 Nov 2023 23:08:39 GMT
Age: 462
X-Cache: HIT, HIT
Server-Timing: time-start-msec;dur=1701040119607,time-elapsed;dur=14,fastly-pop;desc=NYC,hit-state;desc=HIT-CLUSTER
set-cookie: location_data={"country_code":"US","postal_code":"redacted","geo_region":"redacted"}; path=/;
x-robots-tag: all
x-country: US
strict-transport-security: max-age=31557600; includeSubDomains
Cache-Control: max-age=0, must-revalidate, private
alt-svc: h3=":443";ma=86400,h3-29=":443";ma=86400,h3-27=":443";ma=86400
transfer-encoding: chunked
trailer: server-timing

At the end of the body, it includes the data for this Trailer header.

server-timing: rtt; dur=38.4, retrans; dur=0, trailer-timestamp; dur=1701040119621

The Dart HttpClient isn't expecting this appended trailer (or it is malformed). Note how the first character is from the server and this is the 115 it says isn't the 13 it is expecting.

Source: ClientException: Failed to parse HTTP, 115 does not match 13

Request

Add support for the Trailer HTTP header to the Dart HttpClient.

devoncarew commented 7 months ago

cc @brianquinlan and @natebosch

brianquinlan commented 7 months ago

@TijnvandenEijnde Thanks for very the detailed bug report!

My first impression is that this would be difficult to support because the HttpClient API assumes that the complete set of headers have been received before streaming the response.

I wonder if using package:cupertino_http or package:cronet_http would fix this problem.

natebosch commented 7 months ago

this would be difficult to support because the HttpClient API assumes that the complete set of headers have been received before streaming the response.

I think either silently ignoring the trailing header (assuming we cannot or should not add to the header map) or throwing a more actionable exception would be an improvement on the current behavior.

TijnvandenEijnde commented 7 months ago

@brianquinlan You are welcome and thank you for the response!

It would be nice if the regular HttpClient could handle it. However, having to redesign the HttpClient for what I think is an edge case does not make much sense.

I have seen the above-mentioned URL with the Trailer header work in Android applications so I think package:cronet_http will fix the problem. I will test it out in the future and give you an update.

brianquinlan commented 5 months ago

@TijnvandenEijnde That's so much and let us know if package:cronet_http fixes the problem.