Merge pull request #2750 from deads2k/deads-bump-gorestful

race condition in go-restful
This commit is contained in:
bgrant0607 2014-12-04 09:08:53 -08:00
commit 2fe2963b12
11 changed files with 81 additions and 43 deletions

4
Godeps/Godeps.json generated
View File

@ -54,8 +54,8 @@
},
{
"ImportPath": "github.com/emicklei/go-restful",
"Comment": "v1.1.2-38-gab99062",
"Rev": "ab990627e3546d3c6c8ab45c4d887a3d66b1b6ab"
"Comment": "v1.1.2-50-g692a500",
"Rev": "692a50017a7049b26cf7ea4ccfc0d8c77369a793"
},
{
"ImportPath": "github.com/fsouza/go-dockerclient",

View File

@ -1,5 +1,8 @@
Change history of go-restful
=
2014-11-27
- (api add) PrettyPrint per response. (as proposed in #167)
2014-11-12
- (api add) ApiVersion(.) for documentation in Swagger UI

View File

@ -98,6 +98,10 @@ func (c *Container) Add(service *WebService) *Container {
log.Fatalf("[restful] WebService with duplicate root path detected:['%v']", each)
}
}
// if rootPath was not set then lazy initialize it
if len(service.rootPath) == 0 {
service.Path("/")
}
c.webServices = append(c.webServices, service)
return c
}
@ -230,7 +234,7 @@ func (c Container) computeAllowedMethods(req *Request) []string {
methods := []string{}
requestPath := req.Request.URL.Path
for _, ws := range c.RegisteredWebServices() {
matches := ws.compiledPathExpression().Matcher.FindStringSubmatch(requestPath)
matches := ws.pathExpr.Matcher.FindStringSubmatch(requestPath)
if matches != nil {
finalMatch := matches[len(matches)-1]
for _, rt := range ws.Routes() {

View File

@ -121,7 +121,7 @@ func (c CurlyRouter) detectWebService(requestTokens []string, webServices []*Web
var best *WebService
score := -1
for _, each := range webServices {
matches, eachScore := c.computeWebserviceScore(requestTokens, each.compiledPathExpression().tokens)
matches, eachScore := c.computeWebserviceScore(requestTokens, each.pathExpr.tokens)
if matches && (eachScore > score) {
best = each
score = eachScore

View File

@ -32,7 +32,7 @@ func TestCurlyDetectWebService(t *testing.T) {
var wss = []*WebService{ws1, ws2, ws3, ws4, ws5, ws7}
for _, each := range wss {
t.Logf("path=%s,toks=%v\n", each.compiledPathExpression().Source, each.compiledPathExpression().tokens)
t.Logf("path=%s,toks=%v\n", each.pathExpr.Source, each.pathExpr.tokens)
}
router := CurlyRouter{}
@ -79,7 +79,7 @@ func Test_detectWebService(t *testing.T) {
requestPath := fix.path
requestTokens := tokenizePath(requestPath)
for _, ws := range wss {
serviceTokens := ws.compiledPathExpression().tokens
serviceTokens := ws.pathExpr.tokens
matches, score := router.computeWebserviceScore(requestTokens, serviceTokens)
t.Logf("req=%s,toks:%v,ws=%s,toks:%v,score=%d,matches=%v", requestPath, requestTokens, ws.RootPath(), serviceTokens, score, matches)
}
@ -189,6 +189,7 @@ func TestCurly_ISSUE_34_2(t *testing.T) {
// clear && go test -v -test.run TestCurly_JsonHtml ...restful
func TestCurly_JsonHtml(t *testing.T) {
ws1 := new(WebService)
ws1.Path("/")
ws1.Route(ws1.GET("/some.html").To(curlyDummy).Consumes("*/*").Produces("text/html"))
req, _ := http.NewRequest("GET", "/some.html", nil)
req.Header.Set("Accept", "application/json")
@ -205,6 +206,7 @@ func TestCurly_JsonHtml(t *testing.T) {
func TestCurly_ISSUE_137(t *testing.T) {
ws1 := new(WebService)
ws1.Route(ws1.GET("/hello").To(curlyDummy))
ws1.Path("/")
req, _ := http.NewRequest("GET", "/", nil)
_, route, _ := CurlyRouter{}.SelectRoute([]*WebService{ws1}, req)
t.Log(route)
@ -217,6 +219,7 @@ func TestCurly_ISSUE_137(t *testing.T) {
func TestCurly_ISSUE_137_2(t *testing.T) {
ws1 := new(WebService)
ws1.Route(ws1.GET("/hello").To(curlyDummy))
ws1.Path("/")
req, _ := http.NewRequest("GET", "/hello/bob", nil)
_, route, _ := CurlyRouter{}.SelectRoute([]*WebService{ws1}, req)
t.Log(route)

View File

@ -135,11 +135,10 @@ func (r RouterJSR311) selectRoutes(dispatcher *WebService, pathRemainder string)
func (r RouterJSR311) detectDispatcher(requestPath string, dispatchers []*WebService) (*WebService, string, error) {
filtered := &sortableDispatcherCandidates{}
for _, each := range dispatchers {
pathExpr := each.compiledPathExpression()
matches := pathExpr.Matcher.FindStringSubmatch(requestPath)
matches := each.pathExpr.Matcher.FindStringSubmatch(requestPath)
if matches != nil {
filtered.candidates = append(filtered.candidates,
dispatcherCandidate{each, matches[len(matches)-1], len(matches), pathExpr.LiteralCount, pathExpr.VarCount})
dispatcherCandidate{each, matches[len(matches)-1], len(matches), each.pathExpr.LiteralCount, each.pathExpr.VarCount})
}
}
if len(filtered.candidates) == 0 {

View File

@ -34,6 +34,11 @@ func TestDetectDispatcher(t *testing.T) {
ws7 := new(WebService).Path("/{p}/q")
var dispatchers = []*WebService{ws1, ws2, ws3, ws4, ws5, ws6, ws7}
wc := NewContainer()
for _, each := range dispatchers {
wc.Add(each)
}
router := RouterJSR311{}
ok := true

View File

@ -27,11 +27,12 @@ type Response struct {
routeProduces []string // mime-types what the Route says it can produce
statusCode int // HTTP status code that has been written explicity (if zero then net/http has written 200)
contentLength int // number of bytes written for the response body
prettyPrint bool // controls the indentation feature of XML and JSON serialization. It is initialized using var PrettyPrintResponses.
}
// Creates a new response based on a http ResponseWriter.
func NewResponse(httpWriter http.ResponseWriter) *Response {
return &Response{httpWriter, "", []string{}, http.StatusOK, 0} // empty content-types
return &Response{httpWriter, "", []string{}, http.StatusOK, 0, PrettyPrintResponses} // empty content-types
}
// If Accept header matching fails, fall back to this type, otherwise
@ -50,6 +51,11 @@ func (r Response) InternalServerError() Response {
return r
}
// PrettyPrint changes whether this response must produce pretty (line-by-line, indented) JSON or XML output.
func (r *Response) PrettyPrint(bePretty bool) {
r.prettyPrint = bePretty
}
// AddHeader is a shortcut for .Header().Add(header,value)
func (r Response) AddHeader(header string, value string) Response {
r.Header().Add(header, value)
@ -119,7 +125,7 @@ func (r *Response) WriteAsXml(value interface{}) error {
if value == nil { // do not write a nil representation
return nil
}
if PrettyPrintResponses {
if r.prettyPrint {
output, err = xml.MarshalIndent(value, " ", " ")
} else {
output, err = xml.Marshal(value)
@ -150,7 +156,7 @@ func (r *Response) WriteAsJson(value interface{}) error {
if value == nil { // do not write a nil representation
return nil
}
if PrettyPrintResponses {
if r.prettyPrint {
output, err = json.MarshalIndent(value, " ", " ")
} else {
output, err = json.Marshal(value)
@ -192,13 +198,15 @@ func (r *Response) WriteErrorString(status int, errorReason string) error {
// WriteHeader is overridden to remember the Status Code that has been written.
// Note that using this method, the status value is only written when
// - calling WriteEntity
// - or directly WriteAsXml,WriteAsJson.
// - or if the status is 204 (http.StatusNoContent)
// - calling WriteEntity,
// - or directly calling WriteAsXml or WriteAsJson,
// - or if the status is one for which no response is allowed (i.e.,
// 204 (http.StatusNoContent) or 304 (http.StatusNotModified))
func (r *Response) WriteHeader(httpStatus int) {
r.statusCode = httpStatus
// if 204 then WriteEntity will not be called so we need to pass this code
if http.StatusNoContent == httpStatus {
if http.StatusNoContent == httpStatus ||
http.StatusNotModified == httpStatus {
r.ResponseWriter.WriteHeader(httpStatus)
}
}

View File

@ -9,7 +9,7 @@ import (
func TestWriteHeader(t *testing.T) {
httpWriter := httptest.NewRecorder()
resp := Response{httpWriter, "*/*", []string{"*/*"}, 0, 0}
resp := Response{httpWriter, "*/*", []string{"*/*"}, 0, 0, true}
resp.WriteHeader(123)
if resp.StatusCode() != 123 {
t.Errorf("Unexpected status code:%d", resp.StatusCode())
@ -18,7 +18,7 @@ func TestWriteHeader(t *testing.T) {
func TestNoWriteHeader(t *testing.T) {
httpWriter := httptest.NewRecorder()
resp := Response{httpWriter, "*/*", []string{"*/*"}, 0, 0}
resp := Response{httpWriter, "*/*", []string{"*/*"}, 0, 0, true}
if resp.StatusCode() != http.StatusOK {
t.Errorf("Unexpected status code:%d", resp.StatusCode())
}
@ -31,7 +31,7 @@ type food struct {
// go test -v -test.run TestMeasureContentLengthXml ...restful
func TestMeasureContentLengthXml(t *testing.T) {
httpWriter := httptest.NewRecorder()
resp := Response{httpWriter, "*/*", []string{"*/*"}, 0, 0}
resp := Response{httpWriter, "*/*", []string{"*/*"}, 0, 0, true}
resp.WriteAsXml(food{"apple"})
if resp.ContentLength() != 76 {
t.Errorf("Incorrect measured length:%d", resp.ContentLength())
@ -41,17 +41,27 @@ func TestMeasureContentLengthXml(t *testing.T) {
// go test -v -test.run TestMeasureContentLengthJson ...restful
func TestMeasureContentLengthJson(t *testing.T) {
httpWriter := httptest.NewRecorder()
resp := Response{httpWriter, "*/*", []string{"*/*"}, 0, 0}
resp := Response{httpWriter, "*/*", []string{"*/*"}, 0, 0, true}
resp.WriteAsJson(food{"apple"})
if resp.ContentLength() != 22 {
t.Errorf("Incorrect measured length:%d", resp.ContentLength())
}
}
// go test -v -test.run TestMeasureContentLengthJsonNotPretty ...restful
func TestMeasureContentLengthJsonNotPretty(t *testing.T) {
httpWriter := httptest.NewRecorder()
resp := Response{httpWriter, "*/*", []string{"*/*"}, 0, 0, false}
resp.WriteAsJson(food{"apple"})
if resp.ContentLength() != 16 {
t.Errorf("Incorrect measured length:%d", resp.ContentLength())
}
}
// go test -v -test.run TestMeasureContentLengthWriteErrorString ...restful
func TestMeasureContentLengthWriteErrorString(t *testing.T) {
httpWriter := httptest.NewRecorder()
resp := Response{httpWriter, "*/*", []string{"*/*"}, 0, 0}
resp := Response{httpWriter, "*/*", []string{"*/*"}, 0, 0, true}
resp.WriteErrorString(404, "Invalid")
if resp.ContentLength() != len("Invalid") {
t.Errorf("Incorrect measured length:%d", resp.ContentLength())
@ -61,7 +71,7 @@ func TestMeasureContentLengthWriteErrorString(t *testing.T) {
// go test -v -test.run TestStatusCreatedAndContentTypeJson_Issue54 ...restful
func TestStatusCreatedAndContentTypeJson_Issue54(t *testing.T) {
httpWriter := httptest.NewRecorder()
resp := Response{httpWriter, "application/json", []string{"application/json"}, 0, 0}
resp := Response{httpWriter, "application/json", []string{"application/json"}, 0, 0, true}
resp.WriteHeader(201)
resp.WriteAsJson(food{"Juicy"})
if httpWriter.HeaderMap.Get("Content-Type") != "application/json" {
@ -83,7 +93,7 @@ func (e errorOnWriteRecorder) Write(bytes []byte) (int, error) {
// go test -v -test.run TestLastWriteErrorCaught ...restful
func TestLastWriteErrorCaught(t *testing.T) {
httpWriter := errorOnWriteRecorder{httptest.NewRecorder()}
resp := Response{httpWriter, "application/json", []string{"application/json"}, 0, 0}
resp := Response{httpWriter, "application/json", []string{"application/json"}, 0, 0, true}
err := resp.WriteAsJson(food{"Juicy"})
if err.Error() != "fail" {
t.Errorf("Unexpected error message:%v", err)
@ -94,7 +104,7 @@ func TestLastWriteErrorCaught(t *testing.T) {
func TestAcceptStarStar_Issue83(t *testing.T) {
httpWriter := httptest.NewRecorder()
// Accept Produces
resp := Response{httpWriter, "application/bogus,*/*;q=0.8", []string{"application/json"}, 0, 0}
resp := Response{httpWriter, "application/bogus,*/*;q=0.8", []string{"application/json"}, 0, 0, true}
resp.WriteEntity(food{"Juicy"})
ct := httpWriter.Header().Get("Content-Type")
if "application/json" != ct {
@ -106,7 +116,7 @@ func TestAcceptStarStar_Issue83(t *testing.T) {
func TestAcceptSkipStarStar_Issue83(t *testing.T) {
httpWriter := httptest.NewRecorder()
// Accept Produces
resp := Response{httpWriter, " application/xml ,*/* ; q=0.8", []string{"application/json", "application/xml"}, 0, 0}
resp := Response{httpWriter, " application/xml ,*/* ; q=0.8", []string{"application/json", "application/xml"}, 0, 0, true}
resp.WriteEntity(food{"Juicy"})
ct := httpWriter.Header().Get("Content-Type")
if "application/xml" != ct {
@ -118,7 +128,7 @@ func TestAcceptSkipStarStar_Issue83(t *testing.T) {
func TestAcceptXmlBeforeStarStar_Issue83(t *testing.T) {
httpWriter := httptest.NewRecorder()
// Accept Produces
resp := Response{httpWriter, "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8", []string{"application/json"}, 0, 0}
resp := Response{httpWriter, "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8", []string{"application/json"}, 0, 0, true}
resp.WriteEntity(food{"Juicy"})
ct := httpWriter.Header().Get("Content-Type")
if "application/json" != ct {
@ -129,9 +139,19 @@ func TestAcceptXmlBeforeStarStar_Issue83(t *testing.T) {
// go test -v -test.run TestWriteHeaderNoContent_Issue124 ...restful
func TestWriteHeaderNoContent_Issue124(t *testing.T) {
httpWriter := httptest.NewRecorder()
resp := Response{httpWriter, "text/plain", []string{"text/plain"}, 0, 0}
resp := Response{httpWriter, "text/plain", []string{"text/plain"}, 0, 0, true}
resp.WriteHeader(http.StatusNoContent)
if httpWriter.Code != http.StatusNoContent {
t.Errorf("got %d want %d", httpWriter.Code, http.StatusNoContent)
}
}
// go test -v -test.run TestStatusCreatedAndContentTypeJson_Issue163 ...restful
func TestStatusCreatedAndContentTypeJson_Issue163(t *testing.T) {
httpWriter := httptest.NewRecorder()
resp := Response{httpWriter, "application/json", []string{"application/json"}, 0, 0, true}
resp.WriteHeader(http.StatusNotModified)
if httpWriter.Code != http.StatusNotModified {
t.Errorf("Got %d want %d", httpWriter.Code, http.StatusNotModified)
}
}

View File

@ -84,7 +84,7 @@ func (b *RouteBuilder) Doc(documentation string) *RouteBuilder {
func (b *RouteBuilder) Reads(sample interface{}) *RouteBuilder {
b.readSample = sample
typeAsName := reflect.TypeOf(sample).String()
bodyParameter := &Parameter{&ParameterData{Name: typeAsName}}
bodyParameter := &Parameter{&ParameterData{Name: "body"}}
bodyParameter.beBody()
bodyParameter.Required(true)
bodyParameter.DataType(typeAsName)

View File

@ -4,9 +4,7 @@ package restful
// Use of this source code is governed by a license
// that can be found in the LICENSE file.
import (
"log"
)
import "log"
// WebService holds a collection of Route values that bind a Http Method + URL Path to a function.
type WebService struct {
@ -21,19 +19,16 @@ type WebService struct {
apiVersion string
}
// compiledPathExpression ensures that the path is compiled into a RegEx for those routers that need it.
func (w *WebService) compiledPathExpression() *pathExpression {
if w.pathExpr == nil {
if len(w.rootPath) == 0 {
w.Path("/") // lazy initialize path
}
compiled, err := newPathExpression(w.rootPath)
if err != nil {
log.Fatalf("[restful] Invalid path:%s because:%v", w.rootPath, err)
}
w.pathExpr = compiled
// compilePathExpression ensures that the path is compiled into a RegEx for those routers that need it.
func (w *WebService) compilePathExpression() {
if len(w.rootPath) == 0 {
w.Path("/") // lazy initialize path
}
return w.pathExpr
compiled, err := newPathExpression(w.rootPath)
if err != nil {
log.Fatalf("[restful] invalid path:%s because:%v", w.rootPath, err)
}
w.pathExpr = compiled
}
// ApiVersion sets the API version for documentation purposes.
@ -49,6 +44,7 @@ func (w WebService) Version() string { return w.apiVersion }
// All Routes will be relative to this path.
func (w *WebService) Path(root string) *WebService {
w.rootPath = root
w.compilePathExpression()
return w
}