Data proxy: Fix encoded characters in URL path should be proxied encoded (#30597)

Fix encoded characters in URL path should be proxied as encoded in the data proxy.

Fixes #26870 
Fixes #31438

Co-authored-by: Arve Knudsen <arve.knudsen@gmail.com>
This commit is contained in:
Marcus Efraimsson 2021-03-17 12:17:41 +01:00 committed by GitHub
parent 68b05b8aaa
commit c0edf88f9f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 73 additions and 46 deletions

View File

@ -1,7 +1,7 @@
FROM golang:latest
FROM golang:latest
ADD main.go /
WORKDIR /
RUN go build -o main .
RUN GO111MODULE=off go build -o main .
EXPOSE 3011
ENTRYPOINT ["/main"]

View File

@ -1,7 +1,7 @@
FROM golang:latest
FROM golang:latest
ADD main.go /
WORKDIR /
RUN go build -o main .
RUN GO111MODULE=off go build -o main .
EXPOSE 3011
ENTRYPOINT ["/main"]

View File

@ -4,6 +4,7 @@ import (
"errors"
"fmt"
"net/http"
"regexp"
"github.com/grafana/grafana/pkg/api/datasource"
"github.com/grafana/grafana/pkg/api/pluginproxy"
@ -40,9 +41,7 @@ func (hs *HTTPServer) ProxyDataSourceRequest(c *models.ReqContext) {
return
}
// macaron does not include trailing slashes when resolving a wildcard path
proxyPath := ensureProxyPathTrailingSlash(c.Req.URL.Path, c.Params("*"))
proxyPath := getProxyPath(c)
proxy, err := pluginproxy.NewDataSourceProxy(ds, plugin, c, proxyPath, hs.Cfg)
if err != nil {
if errors.Is(err, datasource.URLValidationError{}) {
@ -55,14 +54,12 @@ func (hs *HTTPServer) ProxyDataSourceRequest(c *models.ReqContext) {
proxy.HandleRequest()
}
// ensureProxyPathTrailingSlash Check for a trailing slash in original path and makes
// sure that a trailing slash is added to proxy path, if not already exists.
func ensureProxyPathTrailingSlash(originalPath, proxyPath string) string {
if len(proxyPath) > 1 {
if originalPath[len(originalPath)-1] == '/' && proxyPath[len(proxyPath)-1] != '/' {
return proxyPath + "/"
}
}
var proxyPathRegexp = regexp.MustCompile(`^\/api\/datasources\/proxy\/[\d]+\/?`)
return proxyPath
func extractProxyPath(originalRawPath string) string {
return proxyPathRegexp.ReplaceAllString(originalRawPath, "")
}
func getProxyPath(c *models.ReqContext) string {
return extractProxyPath(c.Req.URL.EscapedPath())
}

View File

@ -7,28 +7,28 @@ import (
)
func TestDataProxy(t *testing.T) {
testCases := []struct {
desc string
origPath string
proxyPath string
exp string
}{
{
"Should append trailing slash to proxy path if original path has a trailing slash",
"/api/datasources/proxy/6/api/v1/query_range/",
"api/v1/query_range/",
"api/v1/query_range/",
},
{
"Should not append trailing slash to proxy path if original path doesn't have a trailing slash",
"/api/datasources/proxy/6/api/v1/query_range",
"api/v1/query_range",
"api/v1/query_range",
},
}
for _, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) {
assert.Equal(t, tc.exp, ensureProxyPathTrailingSlash(tc.origPath, tc.proxyPath))
})
}
t.Run("extractProxyPath", func(t *testing.T) {
testCases := []struct {
originalRawPath string
exp string
}{
{
"/api/datasources/proxy/1",
"",
},
{
"/api/datasources/proxy/1/some/thing",
"some/thing",
},
{
"/api/datasources/proxy/54/api/services/afsd%2Fafsd/operations",
"api/services/afsd%2Fafsd/operations",
},
}
for _, tc := range testCases {
t.Run("Given raw path, should extract expected proxy path", func(t *testing.T) {
assert.Equal(t, tc.exp, extractProxyPath(tc.originalRawPath))
})
}
})
}

View File

@ -179,20 +179,28 @@ func (proxy *DataSourceProxy) director(req *http.Request) {
switch proxy.ds.Type {
case models.DS_INFLUXDB_08:
req.URL.Path = util.JoinURLFragments(proxy.targetUrl.Path, "db/"+proxy.ds.Database+"/"+proxy.proxyPath)
req.URL.RawPath = util.JoinURLFragments(proxy.targetUrl.Path, "db/"+proxy.ds.Database+"/"+proxy.proxyPath)
reqQueryVals.Add("u", proxy.ds.User)
reqQueryVals.Add("p", proxy.ds.DecryptedPassword())
req.URL.RawQuery = reqQueryVals.Encode()
case models.DS_INFLUXDB:
req.URL.Path = util.JoinURLFragments(proxy.targetUrl.Path, proxy.proxyPath)
req.URL.RawPath = util.JoinURLFragments(proxy.targetUrl.Path, proxy.proxyPath)
req.URL.RawQuery = reqQueryVals.Encode()
if !proxy.ds.BasicAuth {
req.Header.Set("Authorization", util.GetBasicAuthHeader(proxy.ds.User, proxy.ds.DecryptedPassword()))
}
default:
req.URL.Path = util.JoinURLFragments(proxy.targetUrl.Path, proxy.proxyPath)
req.URL.RawPath = util.JoinURLFragments(proxy.targetUrl.Path, proxy.proxyPath)
}
unescapedPath, err := url.PathUnescape(req.URL.RawPath)
if err != nil {
logger.Error("Failed to unescape raw path", "rawPath", req.URL.RawPath, "error", err)
return
}
req.URL.Path = unescapedPath
if proxy.ds.BasicAuth {
req.Header.Set("Authorization", util.GetBasicAuthHeader(proxy.ds.BasicAuthUser,
proxy.ds.DecryptedBasicAuthPassword()))

View File

@ -527,7 +527,7 @@ func TestDataSourceProxy_requestHandling(t *testing.T) {
type setUpCfg struct {
headers map[string]string
writeCb func(w http.ResponseWriter)
writeCb func(w http.ResponseWriter, r *http.Request)
}
setUp := func(t *testing.T, cfgs ...setUpCfg) (*models.ReqContext, *models.DataSource) {
@ -539,7 +539,7 @@ func TestDataSourceProxy_requestHandling(t *testing.T) {
for _, cfg := range cfgs {
if cfg.writeCb != nil {
t.Log("Writing response via callback")
cfg.writeCb(w)
cfg.writeCb(w, r)
written = true
}
}
@ -607,7 +607,7 @@ func TestDataSourceProxy_requestHandling(t *testing.T) {
t.Run("Data source returns status code 401", func(t *testing.T) {
ctx, ds := setUp(t, setUpCfg{
writeCb: func(w http.ResponseWriter) {
writeCb: func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(401)
w.Header().Set("www-authenticate", `Basic realm="Access to the server"`)
_, err := w.Write([]byte("Not authenticated"))
@ -624,6 +624,28 @@ func TestDataSourceProxy_requestHandling(t *testing.T) {
assert.Equal(t, 400, proxy.ctx.Resp.Status(), "Status code 401 should be converted to 400")
assert.Empty(t, proxy.ctx.Resp.Header().Get("www-authenticate"))
})
t.Run("Data source should handle proxy path url encoding correctly", func(t *testing.T) {
var req *http.Request
ctx, ds := setUp(t, setUpCfg{
writeCb: func(w http.ResponseWriter, r *http.Request) {
req = r
w.WriteHeader(200)
_, err := w.Write([]byte("OK"))
require.NoError(t, err)
},
})
ctx.Req.Request = httptest.NewRequest("GET", "/api/datasources/proxy/1/path/%2Ftest%2Ftest%2F?query=%2Ftest%2Ftest%2F", nil)
proxy, err := NewDataSourceProxy(ds, plugin, ctx, "/path/%2Ftest%2Ftest%2F", &setting.Cfg{})
require.NoError(t, err)
proxy.HandleRequest()
require.NoError(t, writeErr)
require.NotNil(t, req)
require.Equal(t, "/path/%2Ftest%2Ftest%2F?query=%2Ftest%2Ftest%2F", req.RequestURI)
})
}
func TestNewDataSourceProxy_InvalidURL(t *testing.T) {