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

net/http: add Server.OptionsHandler to allow custom handling of OPTIONS * #41773

Open
simonmittag opened this issue Oct 3, 2020 · 7 comments
Open

Comments

@simonmittag
Copy link

@simonmittag simonmittag commented Oct 3, 2020

What version of Go are you using (go version)?

1.15.2

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

go env Output
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/simonmittag/Library/Caches/go-build"
GOENV="/Users/simonmittag/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/simonmittag/projects/gopath/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/simonmittag/projects/gopath"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/opt/go/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/opt/go/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/fx/nx1t7v3s6xjc55_tvv1fkwgw0000gn/T/go-build132638656=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Global options requests are described here: https://tools.ietf.org/html/rfc7231#page-31

   An OPTIONS request with an asterisk ("*") as the request-target
   (Section 5.3 of [RFC7230]) applies to the server in general rather
   than to a specific resource.

Run this request against an instance of golang http.Server (I'm using this one: https://github.com/simonmittag/j8a/blob/master/server.go), like so:

curl -v -X OPTIONS --request-target '*' --http1.1 https://j8a.io

What did you see?

A canned response from net.http built-in serverHandler, bypassing any handlers I specify.

> OPTIONS * HTTP/1.1
> Host: j8a.io
> User-Agent: curl/7.72.0
> Accept: */*
>
* TLSv1.3 (IN), TLS handshake, Newsession Ticket (4):
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Content-Length: 0
< Date: Sat, 03 Oct 2020 22:13:13 GMT

What did you expect to see?

Would like to customise the server response to this request and include "Allow" HTTP headers to limit the global HTTP methods made available by the server instance. However response for this request is hardcoded into net/http serverHandler: https://github.com/golang/go/blob/master/src/net/http/server.go, line 2859 which overrides handler with the built-in globalOptionsHandler

func (sh serverHandler) ServeHTTP(rw ResponseWriter, req *Request) {
	handler := sh.srv.Handler
	if handler == nil {
		handler = DefaultServeMux
	}
	if req.RequestURI == "*" && req.Method == "OPTIONS" {
		handler = globalOptionsHandler{}
	}
	handler.ServeHTTP(rw, req)
}

Can you allow users to specify this handler as an argument to http.Server?

server := &http.Server{
    GlobalOptionsHandler: myHandler,
}
@simonmittag simonmittag changed the title Allow customised response to global 'OPTIONS *' request to http.Server proposal: Allow customised response to global 'OPTIONS *' request to http.Server Oct 3, 2020
@gopherbot gopherbot added this to the Proposal milestone Oct 3, 2020
@ianlancetaylor ianlancetaylor added this to Incoming in Proposals Oct 14, 2020
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Oct 14, 2020

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Oct 14, 2020

Adding Server.GlobalOptionsHandler (perhaps drop Global ... just OptionsHandler) sounds good to me.

@rsc rsc changed the title proposal: Allow customised response to global 'OPTIONS *' request to http.Server proposal: net/http: add Server.OptionsHandler to allow custom handling of OPTIONS * Oct 14, 2020
@rsc rsc moved this from Incoming to Active in Proposals Oct 14, 2020
@rsc
Copy link
Contributor

@rsc rsc commented Oct 21, 2020

This is a pretty niche thing but seems small enough. Does anyone object to adding this?

@rsc
Copy link
Contributor

@rsc rsc commented Oct 28, 2020

Based on the discussion, this seems like a likely accept.

@rsc rsc moved this from Active to Likely Accept in Proposals Oct 28, 2020
@rsc
Copy link
Contributor

@rsc rsc commented Nov 4, 2020

No change in consensus, so accepted.

@rsc rsc moved this from Likely Accept to Accepted in Proposals Nov 4, 2020
@rsc rsc changed the title proposal: net/http: add Server.OptionsHandler to allow custom handling of OPTIONS * net/http: add Server.OptionsHandler to allow custom handling of OPTIONS * Nov 4, 2020
@rsc rsc modified the milestones: Proposal, Backlog Nov 4, 2020
@simonmittag
Copy link
Author

@simonmittag simonmittag commented Jan 5, 2021

@rsc what happens with this proposal now that it has been accepted? Do you accept pull requests? Anything I can do to help get this implemented, please let me know.

@odeke-em
Copy link
Member

@odeke-em odeke-em commented Mar 7, 2021

@simonmittag thank you for this feature request, and congrats on having it accepted. Thanks Russ, Ian, Brad and the proposal review committee for the review!

Yes we do accept Pull Requests as well as Change Lists (CLs). If using PRs, please read https://golang.org/doc/contribute#sending_a_change_github, and sign the Google CLA and then you'll be set. The resource is a good place to start https://golang.org/doc/contribute. Once you have submitted the change, please let me know and myself and other reviewers will take a look. Thank you, and I look forward to having you as a new Go contributor!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Proposals
Accepted
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants