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

add a spec compliant parser for Accept HTTP header values #1618

Closed
Tracked by #80
Geal opened this issue Aug 25, 2022 · 2 comments · Fixed by #1610
Closed
Tracked by #80

add a spec compliant parser for Accept HTTP header values #1618

Geal opened this issue Aug 25, 2022 · 2 comments · Fixed by #1610
Assignees

Comments

@Geal
Copy link
Contributor

Geal commented Aug 25, 2022

we need to look at values from the Accept header in various places:

fn prefers_html(accept_header: &HeaderValue) -> bool {
accept_header
.to_str()
.map(|accept_str| {
accept_str
.split(',')
.map(|a| a.trim())
.any(|a| a == "text/html")
})
.unwrap_or_default()
}

and

https://github.com/apollographql/router/pull/1610/files#diff-3bc7e7515da7351dcdea41fed01f6e494bde5289b83b981b424c6d67358b25d6R157-R160

Splitting directly on , or ; is not enough because there can be quoted strings, so we need a correct parser here

@glasser
Copy link
Member

glasser commented Aug 25, 2022

see also picoHz/mediatype#4

@glasser
Copy link
Member

glasser commented Aug 26, 2022

Hey look what we just lazywebbed into existence: https://docs.rs/mediatype/latest/mediatype/struct.MediaTypeList.html

Geal added a commit that referenced this issue Aug 26, 2022
Fix #1618 

Since deferred responses can come back as multipart responses, we must
check that the client supports that content type. This will allow older
clients to show a meaningful error message instead of a parsing error if
the `@defer` directive is used but they don't support it
@abernix abernix mentioned this issue Sep 2, 2022
25 tasks
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.

2 participants