Request validation fails on localhost

[bjmc]

Hello, I'm not sure if this is a bug strictly-speaking, but we found it to be unexpected behavior.

Say you have a basic spec like this:

openapi: "3.0.0"
info:
  title: Simple API Example
  version: 0.1.0
servers:
- url: "https://example.com/api/"
  description: Deployed absolute URL
- url: "http://localhost:8080/"
  description: Running locally
paths:
  /:
    get:
      summary: Application health-check.
      responses:
        200:
          description: Application name and version.
          content:
            application/json:
              schema:
                properties:
                  data:
                    type: object
                    properties:
                      name:
                        type: string
                        example: "Sample App"
                      version:
                        type: string
                        example: "0.1.0"

And then a simple web app that includes validation using kin-openapi/openapi3filter:

package main

import (
	"context"
	"flag"
	"fmt"
	"io"
	"log"
	"net/http"
	"path/filepath"

	"github.com/getkin/kin-openapi/openapi3filter"
)

func main() {
	// OpenAPIv3 request validation:
	specPath, _ := filepath.Abs("./spec.yaml")
	oa3router := openapi3filter.NewRouter().WithSwaggerFromFile(specPath)

	validationHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		ctx := context.TODO()
		log.Printf("request.URL is %s", r.URL)
		route, pathParams, _ := oa3router.FindRoute(r.Method, r.URL)
		// Validate request
		requestValidationInput := &openapi3filter.RequestValidationInput{
			Request:    r,
			PathParams: pathParams,
			Route:      route,
		}
		if err := openapi3filter.ValidateRequest(ctx, requestValidationInput); err != nil {
			w.WriteHeader(400)
			io.WriteString(w, err.Error()+"\n")
		} else {
			fmt.Fprint(w, "{\"version\": \"0.1.0\", \"name\": \"Sample App\"}\n")
		}
	})

	httpAddr := flag.String("http.addr", ":8080", "HTTP listen address")
	log.Printf("HTTP server starting on %v\n", *httpAddr)
	log.Fatal(http.ListenAndServe(*httpAddr, validationHandler))
}

If you run $ go run main.go and make a request using $ curl localhost:8080 you'll find you get an invalid route response.

The logging output is:

2019/09/27 14:24:17 HTTP server starting on :8080
2019/09/27 14:24:21 request.URL is /

As far as I can tell the issue is the same one described in this StackOverflow thread

what you get from Go's http.Request.URL is the raw URL (as parsed by the library). In the case you're getting, you're accessing the URL from a relative path, hence the lack of a Host or Scheme in the URL object.

Since the request is made from the same machine, it's only passing a relative URL, and openapi-kin doesn't know how to validate that request successfully. Even explicitly passing a Host header doesn't help:

$ curl localhost:8080/api -H 'Host: example.com'
invalid route

Maybe the FindRoute() method should take into account Host headers? Or maybe there's someplace that URL.ResolveReference() should be called and it's not?

I'm willing to work on putting together a PR, if there's consensus for what a good fix would look like. Otherwise, I'd just ask for extra documentation that kin-openapi validation doesn't work properly on localhost.

Thanks for your work on a very useful tool.

[fenollp]

From a quick inspection it looks like the error you're not checking is relevant:

		route, pathParams, _ := oa3router.FindRoute(r.Method, r.URL)
// panic(_) = http: panic serving 127.0.0.1:60896: Does not match any server

The invalid route comes from the fact that route is nil.

Turns out that the host part of r.URL is unset (not sure why it's this way).
See https://stackoverflow.com/a/23152483/1418165
Basically the net/http.Request URL needs some filling before the call to FindRoute:

                r.URL.Scheme = "http"
                r.URL.Host = r.Host

If you happen to know why http.HandlerFunc doesn't fill out these URL fields, I'm interested!

BTW if you're down these functions could probably do with some polish:

• kin-openapi/openapi3/server.go

  Lines 23 to 35 in 05685eb

  	func (servers Servers) MatchURL(parsedURL *url.URL) (*Server, []string, string) {
  	rawURL := parsedURL.String()
  	if i := strings.IndexByte(rawURL, '?'); i >= 0 {
  	rawURL = rawURL[:i]
  	}
  	for _, server := range servers {
  	pathParams, remaining, ok := server.MatchRawURL(rawURL)
  	if ok {
  	return server, pathParams, remaining
  	}
  	}
  	return nil, nil, ""
  	}

• kin-openapi/openapi3/server.go

  Lines 63 to 101 in 05685eb

  	func (server Server) MatchRawURL(input string) ([]string, string, bool) {
  	pattern := server.URL
  	var params []string
  	for len(pattern) > 0 {
  	c := pattern[0]
  	if len(pattern) == 1 && c == '/' {
  	break
  	}
  	if c == '{' {
  	// Find end of pattern
  	i := strings.IndexByte(pattern, '}')
  	if i < 0 {
  	return nil, "", false
  	}
  	pattern = pattern[i+1:]
  	// Find next '.' or '/'
  	i = strings.IndexAny(input, "./")
  	if i < 0 {
  	i = len(input)
  	}
  	params = append(params, input[:i])
  	input = input[i:]
  	continue
  	}
  	if len(input) == 0 || input[0] != c {
  	return nil, "", false
  	}
  	pattern = pattern[1:]
  	input = input[1:]
  	}
  	if input == "" {
  	input = "/"
  	}
  	if input[0] != '/' {
  	return nil, "", false
  	}
  	return params, input, true
  	}

  
  Surely URL matching has already been solved somewhere else, maybe even in net/url?

[bjmc]

Turns out that the host part of r.URL is unset (not sure why it's this way).

Right, that's the issue I was pointing to in the linked StackOverflow thread.

My understanding (as a golang neophyte) is that the scheme/host are unset because the HTTP request on localhost is a relative URL, it only includes the path, not the host. http.Request.URL is probably behaving correctly, but I don't know how kin-openapi should handle that situation.

Should it treat relative URLs as always matching a server?

Should it never match any server and always fail? (This is basically what it's doing now, which seems inconvenient but you could argue is correct behaviour)

Require an explicit host header? (and then respect that?)

How do other openapi request-validation libraries handle this situation?

[fenollp]

Are we sure that net/http leaves r.URL.Host empty only in the case of localhost?
I'm not sure that's the case and I can't take a look at net/http right now. Can you investigate?

Once the above point proved then yes we should be doing some smarter matching.

[bjmc]

I'll dig into this more later, but according to this r.URL.Host is commonly empty and it sounds to me like maybe this library should be using r.Host to match against the Servers section of the OpenAPI spec document.

The value of r.URL.Host and r.Host are almost always different. On a proxy server, r.URL.Host is the host of the target server and r.Host is the host of the proxy server itself. When not connecting through a proxy, the client does not specify a host in the request URI. In this scenario, r.URL.Host is the empty string.

For reference net/http docs say:

// For server requests, the URL is parsed from the URI
// supplied on the Request-Line as stored in RequestURI.  For
// most requests, fields other than Path and RawQuery will be
// empty. (See RFC 7230, Section 5.3)

[fenollp]

I’m not sure I checked r.RequestURI.
This sounds more and more like FindRoute should be taking the whole http request and do it’s magic from there.

[antonjah]

@fenollp Is this something you're working on? Otherwise, I think I might look into it myself

[bjmc]

We hacked in a workaround for our local tests, but I'd be happy to see a proper upstream fix. Is there consensus on what the desired behavior should be?

My suggestions are:

• HTTP Host headers are respected when performing the request validation.
• You can explicitly list localhost (or 127.0.0.1, as needed) in the servers section of the spec to allow local requests to pass validation even in the absence of a Host header.

[fenollp]

@antonjah I am not working on a fix for this. PRs welcomed :)

[os]

We hacked in a workaround for our local tests, but I'd be happy to see a proper upstream fix. Is there consensus on what the desired behavior should be?

My suggestions are:

• HTTP Host headers are respected when performing the request validation.
• You can explicitly list localhost (or 127.0.0.1, as needed) in the servers section of the spec to allow local requests to pass validation even in the absence of a Host header.

I just found that setting an empty list for the servers section ("servers": []) works as well.

[prabhay-gupta-ef]

I had the same problem and I did the following to address the issue in a more elegant way. Basically updated the URL before invoking FindRoute method. This solution will work in all environments.

url := context.Request().URL
url.Host = context.Request().Host
url.Scheme = context.Scheme()
route, pathParams, _ := router.FindRoute(c.Request().Method, url)

Updated my servers in spec to include localhost server.

servers:
  - url: http://localhost:80
    description: For local development and testing.

[fenollp]

Work on #210 has progressed. Now routers implement FindRoute which takes in a whole *http.Request.
This should get in there.