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

x/net/http: support content negotiation #19307

Open
kevinburke opened this Issue Feb 27, 2017 · 26 comments

Comments

Projects
None yet
@kevinburke
Copy link
Contributor

kevinburke commented Feb 27, 2017

Content negotiation is, roughly, the process of figuring out what content type the response should take, based on an Accept header present in the request.

An example might be an image server that figures out which image format to send to the client, or an API that wants to return HTML to browsers but JSON to command line clients.

It's tricky to get right because the client may accept multiple content types, and the server may have multiple types available, and it can be difficult to match these correctly. I think this is a good fit for Go standard (or adjacent) libraries because:

  • there's a formal specification for how it should behave: https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html
  • it's annoying to implement yourself, correctly; you have to write a mini-parser.
  • it would take one function to implement, which makes it annoying to import an entire third party library for (assuming you find the right one)

I've seen people hack around this in various ways:

  • checking whether the Accept header contains a certain string
  • checking for the first matching value,
  • returning different content types based on the User-Agent
  • requiring different URI's to get different content.

There's a sample implementation here with a pretty good API: https://godoc.org/github.com/golang/gddo/httputil#NegotiateContentType

// NegotiateContentType returns the best offered content type for the request's
// Accept header. If two offers match with equal weight, then the more specific
// offer is preferred.  For example, text/* trumps */*. If two offers match
// with equal weight and specificity, then the offer earlier in the list is
// preferred. If no offers match, then defaultOffer is returned.
func NegotiateContentType(r *http.Request, offers []string, defaultOffer string) string

offers are content-types that the server can respond with, and can include wildcards like text/* or */*. defaultOffer is the default content type, if nothing in offers matches. The returned value never has wildcards.

So you'd call it with something like

availableTypes := []string{"application/json", "text/plain", "text/html", "text/*"}
ctype: = NegotiateContentType(req, availableTypes, "application/json")

If the first value in offers is text/* and the client requests text/plain, NegotiateContentType will return text/plain. This is why you have to have a default - you can't just return the first value in offers because it might include a wildcard.

In terms of where it could live, I'm guessing that net/http is frozen at this point. Maybe one of the packages in x/net would be a good fit? There is also a similar function for parsing Accept-Language headers in x/text/language. Open to ideas.

@quentinmit

This comment has been minimized.

Copy link
Contributor

quentinmit commented Feb 27, 2017

Funny you should mention this, I was just looking for this for use in x/perf. Your proposed API is reasonable, but you should explicitly document what happens in exceptional circumstances:

  • What happens when multiple content types end up with the same q? I think I would expect the first one in offers to be chosen.
  • What happen if the Accept header(s) cannot be parsed?
@kevinburke

This comment has been minimized.

Copy link
Contributor Author

kevinburke commented Feb 28, 2017

What happens when multiple content types end up with the same q? I think I would expect the first one in offers to be chosen.

That sounds reasonable to me.

What happen if the Accept header(s) cannot be parsed?

The spec includes a grammar but doesn't really say what happens if you can't match it, other than you should return a 406 if you can't match anything. I think we should just return the default?

@quentinmit quentinmit added this to the Proposal milestone Feb 28, 2017

@kevinburke

This comment has been minimized.

Copy link
Contributor Author

kevinburke commented Mar 1, 2017

@bradfitz suggested (if we want to go forward with this proposal) it could live in net/http/httputil.

@bradfitz

This comment has been minimized.

Copy link
Member

bradfitz commented Mar 1, 2017

I also mentioned that it seems like defaultOffer string could go away and we say that the first element in the slice was the default.

@kevinburke

This comment has been minimized.

Copy link
Contributor Author

kevinburke commented Mar 1, 2017

Sorry, I tried to edit the description to cover that case.

I also mentioned that it seems like defaultOffer string could go away and we say that the first element in the slice was the default.

I think the problem with this would be, if you had "text/*" as the first element in offers, it would match if the client sent text/plain and text/plain would be returned from the function.

If we return the first value in offers as the default, then the response value would contain a wildcard ("text/*"), which it doesn't in any other circumstance. Maybe that's okay.

@jimmyfrasche

This comment has been minimized.

Copy link
Member

jimmyfrasche commented Mar 1, 2017

@kevinburke What's wrong with returning text/plain if the first offer is text/*? I don't see why it has to return the exact string passed in—don't you need to know the negotiated type to set the header in the response?

Also, if negotiation fails does it return the empty string? I'd rather have an error to distinguish between expected failure, negotiation failed, and unexpected failure, bad request.

@rsc rsc changed the title proposal: add a function for HTTP content negotiation proposal: net/http: support content negotiation Mar 6, 2017

@bradfitz

This comment has been minimized.

Copy link
Member

bradfitz commented Mar 13, 2017

@kevinburke, any response to @jimmyfrasche? This seems fine if the simplified signature works. You want to prepare a CL?

@bradfitz

This comment has been minimized.

Copy link
Member

bradfitz commented Mar 13, 2017

Also, we like to make sure we append "; charset=utf-8" to Content-Types to be explicit. Can we make sure that that's still automatic?

@bradfitz bradfitz modified the milestones: Go1.9Maybe, Go1.10 Jul 20, 2017

@cellfish

This comment has been minimized.

Copy link

cellfish commented Jul 20, 2017

offers are content-types that the server can respond with, and can include wildcards like text/* or */*

I don't think wildcards should be allowed in the offers. The server knows exactly what content types it supports and returning a wildcard content type seems weird. Nor do I think the RFC says wildcards are OK in the content-type header.
Content-Type (https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.17) mentions media-type (https://www.w3.org/Protocols/rfc2616/rfc2616-sec3.html#sec3.7) that does not have wild cards.
Accept (https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.1) mentions media-range which includes wild cards.

I agree with @jimmyfrasche that returning an error seems better so that the caller can know the reason if a default (first offered) is returned. maybe even not return a default at all. If there is a default the caller can handle that rather than build it as part of this method. For example there is a difference between not finding a match in an existing accept header and when the accept header is missing IMO.

Since the RFC for Accept supports multiple parameters (such as charset) how about returning something more than just a string like:

type NegotiatedContentType struct {
   ContentType string
   Params map[string]string
}

func NegotiateContentType(r *http.Request, offers []string) (NegotiatedContentType, error)
@bradfitz

This comment has been minimized.

Copy link
Member

bradfitz commented Nov 15, 2017

@kevinburke, what's the status here?

It can also live in x/net if you want.

It's probably too late for Go 1.10, though.

@bradfitz bradfitz modified the milestones: Go1.10, Go1.11 Nov 15, 2017

@kevinburke

This comment has been minimized.

Copy link
Contributor Author

kevinburke commented Nov 16, 2017

It's too late and I'm buried in work stuff unfortunately, I probably won't have time to try to offer an implementation. I remember when I tried to implement it I ran into non-trivial problems with the design and some kinds of inputs.

@bradfitz bradfitz modified the milestones: Go1.11, Unplanned Nov 16, 2017

@bradfitz bradfitz changed the title proposal: net/http: support content negotiation x/net/http: support content negotiation Nov 16, 2017

@cellfish

This comment has been minimized.

Copy link

cellfish commented Nov 17, 2017

I can do this.
Any feedback on my proposal from 7/20 or do you wanna stick with just returning (string, error)?

@ericbottard

This comment has been minimized.

Copy link

ericbottard commented Mar 7, 2018

Any chance this could be decoupled from http? Content-Type (and arguably Accept) can be seen in non-http contexts and nothing in the logic described above ties it to http (apart from the initial retrieval of the header value). Other (non http) systems (eg messaging) may use headers, and this negotiator could be useful

@bradfitz

This comment has been minimized.

Copy link
Member

bradfitz commented Mar 7, 2018

@cellfish, do Params happen in practice? I think just a string seems fine. People who care about more details can use https://golang.org/pkg/mime/#ParseMediaType

@cellfish

This comment has been minimized.

Copy link

cellfish commented Mar 8, 2018

@ericbottard, when you say decoupled from http, would you consider httputils still being an ok place because having a function that just takes a string seems trivial.
@bradfitz, The only time I've seen params used in practice it was a pretty one-off case with application specific logic. I just figured that in the case someone actually care the library have probably parsed the parts anyway so why not return them structured... If you just care about the string the String method on the struct would obviously return things "nicely". For example if the accept header contains text/html;q=0.8;charset=UTF-8 and that is the best match I would assume you want the returning string to just be text/html;charset=UTF-8 (dropping the q-value). But I guess it is true that parsing can be simplified if params are only preserved and only the q-value is actually parsed (and dropped).

@ericbottard

This comment has been minimized.

Copy link

ericbottard commented Mar 8, 2018

@cellfish Having functions not taking http related (like http.Request) parameters is a first step. The functions not residing in http related packages is a second, nice to have, step. How about https://golang.org/pkg/mime ?

akshayjshah added a commit to akshayjshah/negotiate that referenced this issue Apr 21, 2018

Add content type negotiation
Add a simple wrapper around gddo's content-type negotiation that behaves
more like the proposal in golang/go#19307.
Validation of offers is necessarily simplistic, since this package aims
to delegate nearly all work to the vendored copy of gddo.
@twifkak

This comment has been minimized.

Copy link

twifkak commented Sep 26, 2018

@bradfitz The new Signed Exchanges spec makes use of params during content negotiation [ref]. AMP Packager is a Go server that needs to perform that negotiation [ref].

@lokhman

This comment has been minimized.

Copy link

lokhman commented Oct 24, 2018

What's the status of this proposal? Is there any chance to have this implemented in the nearest future?

@bontibon

This comment has been minimized.

Copy link
Contributor

bontibon commented Oct 26, 2018

@lokhman The proposal has been accepted and is marked "help wanted". Anyone can, and is encouraged, to send a CL that implements this.

@kevinburke

This comment has been minimized.

Copy link
Contributor Author

kevinburke commented Oct 26, 2018

I tried implementing this and ran into some problems/complexity with the implementation/behavior that made me unhappy with the whole thing. I dropped it and don't have plans to work further on it.

@lokhman

This comment has been minimized.

Copy link

lokhman commented Oct 31, 2018

@bontibon @kevinburke I'm currently working on one of my open-source projects that requires exactly this functionality (sort of a comprehensive web framework), so I decided to read about the topic, analyse various existing solutions and create own variant. I hope it should cover all edge cases, do the job quickly and memory efficiently.

So if you see this implementation valuable, please use it in the proposal or contribute to the existing project. The current version of the code is here: gowl/header.go.

slimsag added a commit to sourcegraph/sourcegraph that referenced this issue Nov 6, 2018

go get github.ceom/golang/gddo/httputil
Pulling this in for content type negotation; see golang/go#19307

slimsag added a commit to sourcegraph/sourcegraph that referenced this issue Nov 7, 2018

go get github.ceom/golang/gddo/httputil
Pulling this in for content type negotation; see golang/go#19307

slimsag added a commit to sourcegraph/sourcegraph that referenced this issue Nov 8, 2018

go get github.ceom/golang/gddo/httputil
Pulling this in for content type negotation; see golang/go#19307

slimsag added a commit to sourcegraph/sourcegraph that referenced this issue Nov 13, 2018

Raw repo contents API (#800)
* go get github.ceom/golang/gddo/httputil

Pulling this in for content type negotation; see golang/go#19307

* xlang/vfsutil: expose gitserver archive fetching

* xlang/vfsutil: add ArchiveOpts.Format (for tar archive fetching)

* xlang/vfsutil: add ArchiveOpts.RelativePath

* add raw repo contents serving HTTP API

nicksnyder added a commit to sourcegraph/sourcegraph that referenced this issue Nov 19, 2018

Raw repo contents API (#800)
* go get github.ceom/golang/gddo/httputil

Pulling this in for content type negotation; see golang/go#19307

* xlang/vfsutil: expose gitserver archive fetching

* xlang/vfsutil: add ArchiveOpts.Format (for tar archive fetching)

* xlang/vfsutil: add ArchiveOpts.RelativePath

* add raw repo contents serving HTTP API

@mvdan mvdan added the NeedsFix label Jan 22, 2019

@mvdan

This comment has been minimized.

Copy link
Member

mvdan commented Jan 22, 2019

@kevinburke said he doesn't plan to work further on this, so I'm unassigning him and clarifying that this needs a fix.

@kshitij10496

This comment has been minimized.

Copy link
Contributor

kshitij10496 commented Mar 8, 2019

If no one is currently working on this, I would be happy to take it up. 😄

@agnivade

This comment has been minimized.

Copy link
Member

agnivade commented Mar 8, 2019

Go for it. No need to ask for permission. :)

@rothskeller

This comment has been minimized.

Copy link

rothskeller commented Mar 9, 2019

Permit me to note that Accept is not the only header that needs this sort of parsing. Accept-Encoding and Accept-Language have nearly the same syntax and need virtually the same parsing. A generic implementation would be welcome.

@fgm

This comment has been minimized.

Copy link

fgm commented Mar 10, 2019

Notice that there is already such a negotiation in the x/text/language package, in the ParseAcceptLanguage function

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.