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

fields.get returning an option for not found #82

Closed
guybedford opened this issue Nov 23, 2023 · 9 comments · Fixed by #91
Closed

fields.get returning an option for not found #82

guybedford opened this issue Nov 23, 2023 · 9 comments · Fixed by #91

Comments

@guybedford
Copy link
Contributor

Currently fields.get returns an empty list for a header that has no entry.

It might be useful to explicitly make this an option?

Alternatively, a fields.has might be useful for these kinds of checks as well.

A length check on a list for existence just feels a bit unintuitive even though it's semantically correct.

@pchickey
Copy link
Contributor

I don't think the interface should return option<list<item>> because that implies there is a semantic difference between none and some([]), and that isn't the case in HTTP. So, I believe that a non-existent header should still return [].

I am open to adding fields.has: func(field-name) -> bool.

@PiotrSikora
Copy link
Collaborator

There is a difference between fields.get("Foo") -> None (no Foo header on the wire) and fields.get("Foo") -> Some([]) (Foo header present without a value).

@pchickey
Copy link
Contributor

I was not aware that HTTP headers could be present without a value assigned - can you point me to an RFC where that is described?

@PiotrSikora
Copy link
Collaborator

I was not aware that HTTP headers could be present without a value assigned - can you point me to an RFC where that is described?

From RFC9110, sec. 5.5:

HTTP field values consist of a sequence of characters in a format defined by the field's grammar.
Each field's grammar is usually defined using ABNF ([RFC5234]).

field-value    = *field-content

From RFC5234, sec. 3.6:

3.6. Variable Repetition: *Rule

The operator "*" preceding an element indicates repetition. The full
form is:

    <a>*<b>element

where <a> and <b> are optional decimal values, indicating at least
<a> and at most <b> occurrences of the element.

Default values are 0 and infinity so that *<element> allows any
number, including zero; 1*<element> requires at least one;
3*3<element> allows exactly 3; and 1*2<element> allows one or two.

i.e. HTTP field values can contain zero characters.

Most notably, Accept-Encoding header has a special meaning for an empty header value. From RFC9110, sec. 12.5.3:

An Accept-Encoding header field with a field value that is empty implies that the user agent does not want any content coding in response.

@pchickey
Copy link
Contributor

pchickey commented Dec 1, 2023

Thanks! We should make this change. We also need a way for setters to make a field present with no values.

@pchickey
Copy link
Contributor

pchickey commented Dec 1, 2023

What if [""] was used to represent a present but empty field value? I think this would compose with all the existing constructors, setters, and getters, without needing a change to the type signatures.

@pchickey
Copy link
Contributor

pchickey commented Dec 4, 2023

For some prior art, whatwg fetch uses the empty string to represent a present but empty header. See the second example here: https://fetch.spec.whatwg.org/#example-header-list-get-decode-split

@guybedford
Copy link
Contributor Author

guybedford commented Dec 4, 2023

@pchickey the whatwg implementation also uses strings not lists for multiple values afaict - var h = new Headers({ a: 'b' }); h.append('A', 'c'); h.get('a'); returns the string "b, c". It also always uses comma separation in the underlying implementation - sending the header A: b, c instead of a: b\na: c.

@PiotrSikora
Copy link
Collaborator

What if [""] was used to represent a present but empty field value? I think this would compose with all the existing constructors, setters, and getters, without needing a change to the type signatures.

I think that's fine, and even required to represent a case when a key has multiple values, one of which is empty (e.g. Ok("foo", "", "bar")).

However, returning Ok([]) when the field doesn't exist feels very non-intuitive, especially because result is used in other parts of this specification, and it sounds that the primarily justification for this design choice is to avoid making changes to the yet-to-be-finalized API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants