Closed DartBot closed 10 months ago
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.
This comment was originally written by softwaredev...@gmail.com
What about copying the API dart:io has?
dart:io.HttpClientResponse:
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…
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.
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.
@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?
I'm no longer on the Dart team, sorry!
how to fix this, any work around?
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));
}
}
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. |
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!
@brianquinlan your PR was merged, but it seems to have been rolled back?
What is the status of this?
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?
Originally opened as dart-lang/sdk#21802
This issue was originally filed by jirkad...@gmail.com
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)