Skip to content
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

package:http abstracts request headers as a Map<string, string>; that does not work for repeating headers #24

Closed
DartBot opened this issue Jun 5, 2015 · 17 comments · Fixed by #1114
Assignees
Labels
contributions-welcome Contributions welcome to help resolve this (the resolution is expected to be clear from the issue) type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@DartBot
Copy link

DartBot commented Jun 5, 2015

Originally opened as dart-lang/sdk#21802

This issue was originally filed by [email protected]


Package http represents request headers as a Map<string, string>, both in requests as well as in responses. That does not work for repeating headers, In that case one needs to fold them into a single string. Which is not always practical or possible.

What steps will reproduce the problem?

The most problematic header is Set-Cookie since the concatenation of multiple cookies into one string is tricky to parse.

Please provide any additional information below.

Some browsers and libraries e.g. http.cookies in Python can deal with folded Set-Cookie headers.

RFC 6265 says "don't do that" (section 3)

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/3276024?v=3" align="left" width="48" height="48"hspace="10"> Comment by anders-sandholm


Added Area-Pkg, Pkg-Http, Triaged labels.

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/188?v=3" align="left" width="48" height="48"hspace="10"> Comment by nex3


The plan here for both http and shelf is to provide a way to access headers that preserves repetition for cases like this. Something like a multiHeaders hash or a getMultiHeader method would work here. There will probably also be a more specialized way of accessing parsed cookies as well (issue #20).

Unfortunately I probably won't have time to work on this for a while yet. If you want to take a stab, I can certainly review patches!


Added PatchesWelcome label.

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

This comment was originally written by [email protected]


What about copying the API dart:io has?

dart:io.HttpClientResponse:

  • List<Cookie> get cookies
  • HttpHeaders get headers

dart:io.HttpHeaders seems quite a high-level general class. I would've preferred if it conformed to Map<String, List<String>> interface, though then there would be the problem of naming the set() and add() methods. The Map.addAll would actually mean calling set() for each value, not add())… So it might be better as it is.

dart.html does headers wrong as well, on HttpRequest (which stands for both the request and the response) they have
 - String getResponseHeader(String header)
 - Map<String, String> get responseHeaders
 - String getAllResponseHeaders()

but it does not matter much because the browser leaves out set-cookie headers http://www.w3.org/TR/XMLHttpRequest/#the-getallresponseheaders%28%29-method .

Anyways, I'd prefer having the api dart.io has everywhere, over inventing yet another one. There are three different APIs already: dart.io.httpclient, http, dart.html.httprequest and there is also chrome.cookies in package:chrome (which encapsulates what different languages would call a cookie jar). Then there are various related cookie and session packages on Pub… Angular.dart has their own cookie classes…

@DartBot DartBot added type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) contributions-welcome Contributions welcome to help resolve this (the resolution is expected to be clear from the issue) labels Jun 5, 2015
@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/188?v=3" align="left" width="48" height="48"hspace="10"> Comment by nex3


The dart:io style of having an idiosyncratic Map-like object is much less usable than having an actual Map in terms of both interoperability and conceptual overhead. It also considers headers to be multi-valued by default, which makes the much-more-common case of working with single-valued headers unduly difficult.

There's certainly room to do better in the package:http API than what it does now, including borrowing some ideas from dart:io like providing access to parsed versions of well-known headers. But providing a better API is the reason this package exists, and this is a case where it can do so.

@RedHatter
Copy link

This really needs to be addressed. In its current state the http package is completely unusable when dealing with an api that uses repeating headers.

@cachapa
Copy link

cachapa commented Mar 12, 2021

@nex3 I believe it is your name tagged in the TODOs:
https://github.com/dart-lang/http/blob/master/lib/src/base_request.dart#L85
https://github.com/dart-lang/http/blob/master/lib/src/base_response.dart#L30

This is blocking a project I'm currently working on and was wondering if it could be given some attention, maybe reviewing #479 from @GabrielTavernini as a temporary workaround?

@nex3
Copy link
Member

nex3 commented Mar 12, 2021

I'm no longer on the Dart team, sorry!

@allasca
Copy link

allasca commented Sep 2, 2021

how to fix this, any work around?

@medz
Copy link

medz commented Dec 22, 2023

This is the header I implemented by imitating the web API, which is perfectly compatible with duplicate headers

import 'dart:convert';

/// This Fetch API interface allows you to perform various actions on HTTP request and response headers. These actions include retrieving, setting, adding to, and removing. A Headers object has an associated header list, which is initially empty and consists of zero or more name and value pairs.  You can add to this using methods like append() (see Examples.) In all methods of this interface, header names are matched by case-insensitive byte sequence.
///
/// [MDN Reference](https://developer.mozilla.org/docs/Web/API/Headers)
class Headers {
  final List<(String, String)> _storage;

  /// Internal constructor, to create a new instance of `Headers`.
  const Headers._(this._storage);

  /// The Headers() constructor creates a new Headers object.
  ///
  /// [MDN Reference](https://developer.mozilla.org/docs/Web/API/Headers/Headers)
  factory Headers([Object? init]) => Headers._((init,).toStorage());

  /// Appends a new value onto an existing header inside a Headers object, or
  /// adds the header if it does not already exist.
  ///
  /// [MDN Reference](https://developer.mozilla.org/docs/Web/API/Headers/append)
  void append(String name, String value) => _storage.add((name, value));

  /// Deletes a header from a Headers object.
  ///
  /// [MDN Reference](https://developer.mozilla.org/docs/Web/API/Headers/delete)
  void delete(String name) =>
      _storage.removeWhere((element) => element.$1.equals(name));

  /// Returns an iterator allowing to go through all key/value pairs contained
  /// in this object.
  ///
  /// [MDN Reference](https://developer.mozilla.org/docs/Web/API/Headers/entries)
  Iterable<(String, String)> entries() sync* {
    for (final (name, value) in _storage) {
      // https://fetch.spec.whatwg.org/#ref-for-forbidden-response-header-name%E2%91%A0
      if (name.equals('set-cookie')) continue;

      yield (name, value);
    }
  }

  /// Executes a provided function once for each key/value pair in this Headers object.
  ///
  /// [MDN Reference](https://developer.mozilla.org/en-US/docs/Web/API/Headers/forEach)
  void forEach(void Function(String value, String name, Headers parent) fn) =>
      entries().forEach((element) => fn(element.$2, element.$1, this));

  /// Returns a String sequence of all the values of a header within a Headers
  /// object with a given name.
  ///
  /// [MDN Reference](https://developer.mozilla.org/docs/Web/API/Headers/get)
  String? get(String name) {
    return switch (_storage.valuesOf(name)) {
      Iterable<String> values when values.isNotEmpty => values.join(', '),
      _ => null,
    };
  }

  /// Returns an array containing the values of all Set-Cookie headers
  /// associated with a response.
  ///
  /// [MDN Reference](https://developer.mozilla.org/docs/Web/API/Headers/getSetCookie)
  Iterable<String> getSetCookie() => _storage.valuesOf('Set-Cookie');

  /// Returns a boolean stating whether a Headers object contains a certain header.
  ///
  /// [MDN Reference](https://developer.mozilla.org/docs/Web/API/Headers/has)
  bool has(String name) => _storage.any((element) => element.$1.equals(name));

  /// Returns an iterator allowing you to go through all keys of the key/value
  /// pairs contained in this object.
  ///
  /// [MDN Reference](https://developer.mozilla.org/docs/Web/API/Headers/keys)
  Iterable<String> keys() => _storage.map((e) => e.$1).toSet();

  /// Sets a new value for an existing header inside a Headers object, or adds
  /// the header if it does not already exist.
  ///
  /// [MDN Reference](https://developer.mozilla.org/docs/Web/API/Headers/set)
  void set(String name, String value) => this
    ..delete(name)
    ..append(name, value);

  /// Returns an iterator allowing you to go through all values of the
  /// key/value pairs contained in this object.
  ///
  /// [MDN Reference](https://developer.mozilla.org/docs/Web/API/Headers/values)
  Iterable<String> values() => keys().map((e) => get(e)).whereType();
}

extension on String {
  bool equals(String other) => other.toLowerCase() == toLowerCase();
}

extension on Iterable<(String, String)> {
  Iterable<String> valuesOf(String name) =>
      where((element) => element.$1.equals(name)).map((e) => e.$2);
}

extension on (Object?,) {
  List<(String, String)> toStorage() {
    return switch (this.$1) {
      Headers value => value.toStorage(),
      String value => value.toStorage(),
      Iterable<String> value => value.toStorage(),
      Iterable<(String, String)> value => value.toList(),
      Iterable<Iterable<String>> value => value.toStorage(),
      Map<String, String> value => value.toStorage(),
      Map<String, Iterable<String>> value => value.toStorage(),
      _ => [],
    };
  }
}

extension on Map<String, Iterable<String>> {
  List<(String, String)> toStorage() {
    return entries
        .map((e) => e.value.map((value) => (e.key, value)))
        .expand((e) => e)
        .toList();
  }
}

extension on Map<String, String> {
  List<(String, String)> toStorage() =>
      entries.map((e) => (e.key, e.value)).toList();
}

extension on Iterable<Iterable<String>> {
  List<(String, String)> toStorage() {
    final storage = <(String, String)>[];
    for (final element in this) {
      switch (element) {
        case Iterable<String> value when value.length == 2:
          storage.add((value.first, value.last));
          break;
        case Iterable<String> value when value.length == 1:
          final pair = value.first.toHeadersPair();
          if (pair != null) storage.add(pair);
          break;
        case Iterable<String> value when value.length > 2:
          for (final element in value.skip(1)) {
            storage.add((value.first, element));
          }
          break;
      }
    }

    return storage;
  }
}

extension on Iterable<String> {
  List<(String, String)> toStorage() =>
      map((e) => e.toHeadersPair()).whereType<(String, String)>().toList();
}

extension on Headers {
  List<(String, String)> toStorage() => entries().toList();
}

extension on String {
  /// Converts a string to a list of headers.
  List<(String, String)> toStorage() =>
      const LineSplitter().convert(this).toStorage();

  /// Parses to a header pair.
  (String, String)? toHeadersPair() {
    final index = indexOf(':');
    if (index == -1) return null;

    return (substring(0, index), substring(index + 1));
  }
}

@brianquinlan
Copy link
Collaborator

I'm taking a look at this right now. Here are the implementations and how they handle cookies/headers:

Client Implementation
IOClient Headers represented as Map<String, List<String>>. Can present cookies as structured objects
BrowserClient Headers represented as List<String, String>. Set-Cookie headers stripped.
CupertinoClient Headers represented as Map<String, String>. Method to convert Set-Cookie header into a structured NSHTTPCookie object.
CronetClient Headers represented as List<String> or Map<String, List<string>>. No high-level cookie representation.
FetchClient Headers represented as List<String>. Set-Cookie headers stripped.

@brianquinlan
Copy link
Collaborator

We could add a new List<String> representation of header values to BaseResponse:

class BaseResponse {
  ...
  Map<String, List<String>> headersFieldValueList;
}

Would that be sufficient? Getting dart:io cookies would be a matter of:

cookies =  [for (var value in response.headersFieldValueList['set-cookie']) Cookie.fromSetCookieValue(value)];

Feedback very welcome!

@valerauko
Copy link

@brianquinlan your PR was merged, but it seems to have been rolled back?

What is the status of this?

@valerauko
Copy link

Sorry, I just realized that your PR only handled this case for the Response types.

Is there any plan for do the same with Request types?

@RenautMestdagh
Copy link

In this solution, values for the same headers are concatenated using commas. This leads to an issue when multiple Set-Cookie headers are present, particularly when one of them includes a date, such as:

__Secure-authjs.session-token=eyJ.....mI; Path=/; Expires=Thu, 13 Feb 2025 23:06:09 GMT; Secure; HttpOnly; SameSite=lax

The comma in the date causes difficulty in easily separating the individual Set-Cookie headers.

@brianquinlan
Copy link
Collaborator

HeadersWithSplitValues does correctly parse cookies. You can look at the tests here:

group('.headersSplitValues', () {

@brianquinlan
Copy link
Collaborator

Sorry, I just realized that your PR only handled this case for the Response types.

Is there any plan for do the same with Request types?

No, because it is harder to do that in a non-breaking way. Is it not possible to just construct the comma-separated list of header values yourself?

@valerauko
Copy link

Is it not possible to just construct the comma-separated list of header values yourself?

It is, though in the case of Cookie it's semicolon-separated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributions-welcome Contributions welcome to help resolve this (the resolution is expected to be clear from the issue) type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants