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.

Sign up for GitHub

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

Jump to bottom

786 fix files base_path sorting #803

Merged
merged 11 commits into from
Jul 10, 2024
Merged

786 fix files base_path sorting #803

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
1abae9e
added test case to TestHTTPServer_ServeHTTP()
johakoch Feb 9, 2024
3e5849c
for files base_path route: first add wildcard, then sort
johakoch Feb 8, 2024
09f2868
added changelog entry
johakoch Feb 9, 2024
1a14940
added more test cases
johakoch Feb 14, 2024
fb95834
fixed getAPIErrorTemplate()
johakoch Feb 14, 2024
221a9f4
fixed HTML tag order
johakoch Feb 14, 2024
278954e
amended the changelog entry
johakoch Feb 14, 2024
65a50e4
Merge branch 'master' into 786-fix-files-base_path-sorting
johakoch Apr 8, 2024
82161f7
Merge branch 'master' into 786-fix-files-base_path-sorting
malud Apr 17, 2024
76b22d3
Merge branch 'master' into 786-fix-files-base_path-sorting
johakoch May 8, 2024
78412d5
Merge branch 'master' into 786-fix-files-base_path-sorting
johakoch Jul 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ Unreleased changes are available as `coupergateway/couper:edge` container.
* Storing of digit-starting string object keys in [request context](https://docs.couper.io/configuration/variables#request) and of digit-starting string header field names in [request](https://docs.couper.io/configuration/variables#request) variable ([#799](https://github.com/coupergateway/couper/pull/799))
* Use of boolean values for the `headers` attribute or [modifiers](https://docs.couper.io/configuration/modifiers) ([#805](https://github.com/coupergateway/couper/pull/805))
* Duplicate [CORS](https://docs.couper.io/configuration/block/cors) response headers (with backend sending CORS response headers, too) ([#804](https://github.com/coupergateway/couper/pull/804))
* Erroneously sending 404 when serving from [`files`](https://docs.couper.io/configuration/block/files) due to wrong registration of `base_path`s, and when serving from multiple `files` or [`spa`](https://docs.couper.io/configuration/block/spa) in combination with [`api`](https://docs.couper.io/configuration/block/api) due to wrong selecting of the API error template ([#803](https://github.com/coupergateway/couper/pull/803))

* **Dependencies**
* build with go 1.22 ([#810](https://github.com/coupergateway/couper/pull/810))
Expand Down
2 changes: 1 addition & 1 deletion config/runtime/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ func NewServerConfiguration(conf *config.Couper, log *logrus.Entry, memStore *ca
append(serverBodies, append(fileBodies, filesConf.Remain)...), fileHandler, "",
)

err = setRoutesFromHosts(serverConfiguration, portsHosts, serverOptions.FilesBasePaths[i], fileHandler, files)
err = setRoutesFromHosts(serverConfiguration, portsHosts, utils.JoinOpenAPIPath(serverOptions.FilesBasePaths[i], "/**"), fileHandler, files)
if err != nil {
return nil, err
}
Expand Down
64 changes: 53 additions & 11 deletions server/http_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ func TestHTTPServer_ServeHTTP(t *testing.T) {
{"spa/01_couper.hcl", []requestCase{
{
testRequest{http.MethodGet, "http://anyserver:8080/"},
expectation{http.StatusOK, []byte(`<html><body><title>1.0</title></body></html>`), nil, "spa"},
expectation{http.StatusOK, []byte(`<html><title>1.0</title><body></body></html>`), nil, "spa"},
},
{
testRequest{http.MethodGet, "http://anyserver:8080/app"},
Expand Down Expand Up @@ -272,14 +272,56 @@ func TestHTTPServer_ServeHTTP(t *testing.T) {
expectation{http.StatusNotFound, []byte("<html>route not found error</html>\n"), http.Header{"Couper-Error": {"route not found error"}}, ""},
},
}},
{"files/03_couper.hcl", []requestCase{
{
testRequest{http.MethodGet, "http://anyserver:8080/"},
expectation{http.StatusOK, []byte(`<html lang="en">index</html>`), nil, "file"},
},
{
testRequest{http.MethodGet, "http://anyserver:8080/assets/foo.js"},
expectation{http.StatusOK, []byte(`console.log("foo");`), nil, "file"},
},
}},
{"files_api/01_couper.hcl", []requestCase{
{
testRequest{http.MethodGet, "http://anyserver:8080/"},
expectation{http.StatusOK, []byte(`<html lang="en">index</html>`), nil, "file"},
},
{
testRequest{http.MethodGet, "http://anyserver:8080/assets/foo.js"},
expectation{http.StatusOK, []byte(`console.log("foo");`), nil, "file"},
},
}},
{"files_api/02_couper.hcl", []requestCase{
{
testRequest{http.MethodGet, "http://anyserver:8080/"},
expectation{http.StatusOK, []byte(`<html lang="en">index</html>`), nil, "file"},
},
}},
{"spa_api/01_couper.hcl", []requestCase{
{
testRequest{http.MethodGet, "http://anyserver:8080/foo"},
expectation{http.StatusOK, []byte("<html><title>SPA_01</title><body></body></html>\n"), nil, "spa"},
},
{
testRequest{http.MethodGet, "http://anyserver:8080/spa/foo"},
expectation{http.StatusOK, []byte("<html><title>SPA_01</title><body></body></html>\n"), nil, "spa"},
},
}},
{"spa_api/02_couper.hcl", []requestCase{
{
testRequest{http.MethodGet, "http://anyserver:8080/foo"},
expectation{http.StatusOK, []byte("<html><title>SPA_01</title><body></body></html>\n"), nil, "spa"},
},
}},
{"files_spa_api/01_couper.hcl", []requestCase{
{
testRequest{http.MethodGet, "http://anyserver:8080/"},
expectation{http.StatusOK, []byte("<html><body><title>SPA_01</title>{\"default\":\"true\"}</body></html>\n"), nil, "spa"},
expectation{http.StatusOK, []byte("<html><title>SPA_01</title><body>{\"default\":\"true\"}</body></html>\n"), nil, "spa"},
},
{
testRequest{http.MethodGet, "http://anyserver:8080/foo"},
expectation{http.StatusOK, []byte("<html><body><title>SPA_01</title>{\"default\":\"true\"}</body></html>\n"), nil, "spa"},
expectation{http.StatusOK, []byte("<html><title>SPA_01</title><body>{\"default\":\"true\"}</body></html>\n"), nil, "spa"},
},
}},
{"api/01_couper.hcl", []requestCase{
Expand Down Expand Up @@ -345,35 +387,35 @@ func TestHTTPServer_ServeHTTP(t *testing.T) {
},
{
testRequest{http.MethodGet, "http://anyserver:8080/"},
expectation{http.StatusOK, []byte("<html><body><title>FS_01</title></body></html>\n"), http.Header{"Content-Type": {"text/html; charset=utf-8"}}, "file"},
expectation{http.StatusOK, []byte("<html><title>FS_01</title><body></body></html>\n"), http.Header{"Content-Type": {"text/html; charset=utf-8"}}, "file"},
},
{
testRequest{http.MethodGet, "http://anyserver:8080/spa1"},
expectation{http.StatusOK, []byte("<html><body><title>SPA_01</title></body></html>\n"), http.Header{"Content-Type": {"text/html; charset=utf-8"}}, "spa"},
expectation{http.StatusOK, []byte("<html><title>SPA_01</title><body></body></html>\n"), http.Header{"Content-Type": {"text/html; charset=utf-8"}}, "spa"},
},
{
testRequest{http.MethodGet, "http://example.com:8080/"},
expectation{http.StatusOK, []byte("<html><body><title>FS_01</title></body></html>\n"), http.Header{"Content-Type": {"text/html; charset=utf-8"}}, "file"},
expectation{http.StatusOK, []byte("<html><title>FS_01</title><body></body></html>\n"), http.Header{"Content-Type": {"text/html; charset=utf-8"}}, "file"},
},
{
testRequest{http.MethodGet, "http://example.org:9876/"},
expectation{http.StatusOK, []byte("<html><body><title>FS_01</title></body></html>\n"), http.Header{"Content-Type": {"text/html; charset=utf-8"}}, "file"},
expectation{http.StatusOK, []byte("<html><title>FS_01</title><body></body></html>\n"), http.Header{"Content-Type": {"text/html; charset=utf-8"}}, "file"},
},
{
testRequest{http.MethodGet, "http://couper.io:8080/"},
expectation{http.StatusOK, []byte("<html><body><title>FS_02</title></body></html>\n"), http.Header{"Content-Type": {"text/html; charset=utf-8"}}, "file"},
expectation{http.StatusOK, []byte("<html><title>FS_02</title><body></body></html>\n"), http.Header{"Content-Type": {"text/html; charset=utf-8"}}, "file"},
},
{
testRequest{http.MethodGet, "http://couper.io:8080/spa2"},
expectation{http.StatusOK, []byte("<html><body><title>SPA_02</title></body></html>\n"), http.Header{"Content-Type": {"text/html; charset=utf-8"}}, "spa"},
expectation{http.StatusOK, []byte("<html><title>SPA_02</title><body></body></html>\n"), http.Header{"Content-Type": {"text/html; charset=utf-8"}}, "spa"},
},
{
testRequest{http.MethodGet, "http://example.net:9876/"},
expectation{http.StatusOK, []byte("<html><body><title>FS_02</title></body></html>\n"), http.Header{"Content-Type": {"text/html; charset=utf-8"}}, "file"},
expectation{http.StatusOK, []byte("<html><title>FS_02</title><body></body></html>\n"), http.Header{"Content-Type": {"text/html; charset=utf-8"}}, "file"},
},
{
testRequest{http.MethodGet, "http://v-server3.com:8080/"},
expectation{http.StatusOK, []byte("<html><body><title>FS_03</title></body></html>\n"), http.Header{"Content-Type": {"text/html; charset=utf-8"}}, "file"},
expectation{http.StatusOK, []byte("<html><title>FS_03</title><body></body></html>\n"), http.Header{"Content-Type": {"text/html; charset=utf-8"}}, "file"},
},
{
testRequest{http.MethodGet, "http://v-server3.com:8080/spa2"},
Expand Down
23 changes: 9 additions & 14 deletions server/mux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
"github.com/coupergateway/couper/errors"
"github.com/coupergateway/couper/handler"
"github.com/coupergateway/couper/handler/middleware"
"github.com/coupergateway/couper/utils"
)

// Mux is a http request router and dispatches requests
Expand Down Expand Up @@ -100,7 +99,7 @@ func (m *Mux) RegisterConfigured() {
}

for _, path := range sortedPathPatterns(m.opts.FileRoutes) {
mustAddRoute(m.fileRoot, utils.JoinOpenAPIPath(path, "/**"), m.opts.FileRoutes[path], false)
mustAddRoute(m.fileRoot, path, m.opts.FileRoutes[path], false)
}

for _, path := range sortedPathPatterns(m.opts.SPARoutes) {
Expand Down Expand Up @@ -219,12 +218,9 @@ func (m *Mux) getAPIErrorTemplate(reqPath string) (*errors.Template, *config.API
filesPaths = m.opts.ServerOptions.FilesBasePaths
}

for _, spaPath := range spaPaths {
for _, filesPath := range filesPaths {
if isAPIError(path, filesPath, spaPath, reqPath) {
return m.opts.ServerOptions.APIErrTpls[api], api
}
}
basePaths := append(spaPaths, filesPaths...)
if isAPIError(path, reqPath, basePaths...) {
return m.opts.ServerOptions.APIErrTpls[api], api
}
}

Expand Down Expand Up @@ -263,13 +259,12 @@ func mustAddRoute(root *gmux.Router, path string, handler http.Handler, trailing

// isAPIError checks the path w/ and w/o the
// trailing slash against the request path.
func isAPIError(apiPath, filesBasePath, spaBasePath, reqPath string) bool {
func isAPIError(apiPath, reqPath string, filesOrSpaBasePaths ...string) bool {
if matchesPath(apiPath, reqPath) {
if isConfigured(filesBasePath) && apiPath == filesBasePath {
return false
}
if isConfigured(spaBasePath) && apiPath == spaBasePath {
return false
for _, filesOrSpaBasePath := range filesOrSpaBasePaths {
if isConfigured(filesOrSpaBasePath) && apiPath == filesOrSpaBasePath {
return false
}
}

return true
Expand Down
10 changes: 10 additions & 0 deletions server/testdata/integration/files/03_couper.hcl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
server "files" {
files "htdocs" {
base_path = "/"
document_root = "./"
}
files "assets" {
base_path = "/assets"
document_root = "./"
}
}
1 change: 1 addition & 0 deletions server/testdata/integration/files/foo.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
console.log("foo");
12 changes: 12 additions & 0 deletions server/testdata/integration/files_api/01_couper.hcl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
server "files" {
files "htdocs" {
base_path = "/"
document_root = "./"
}
files "assets" {
base_path = "/assets"
document_root = "./"
}
api {
}
}
8 changes: 8 additions & 0 deletions server/testdata/integration/files_api/02_couper.hcl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
server "files" {
files "htdocs" {
base_path = "/"
document_root = "./"
}
api {
}
}
1 change: 1 addition & 0 deletions server/testdata/integration/files_api/foo.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
console.log("foo");
1 change: 1 addition & 0 deletions server/testdata/integration/files_api/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<html lang="en">index</html>
2 changes: 1 addition & 1 deletion server/testdata/integration/files_spa_api/01_app.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
<html><body><title>SPA_01</title>__BOOTSTRAP_DATA__</body></html>
<html><title>SPA_01</title><body>__BOOTSTRAP_DATA__</body></html>
2 changes: 1 addition & 1 deletion server/testdata/integration/files_spa_api/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
<html><body><title>FS</title></body></html>
<html><title>FS</title><body></body></html>
2 changes: 1 addition & 1 deletion server/testdata/integration/spa/app.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
<html><body><title>1.0</title></body></html>
<html><title>1.0</title><body></body></html>
1 change: 1 addition & 0 deletions server/testdata/integration/spa_api/01_app.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<html><title>SPA_01</title><body></body></html>
14 changes: 14 additions & 0 deletions server/testdata/integration/spa_api/01_couper.hcl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
server "spa" {
spa "spa1" {
base_path = "/"
bootstrap_file = "01_app.html"
paths = ["/**"]
}
spa "spa2" {
base_path = "/spa"
bootstrap_file = "01_app.html"
paths = ["/**"]
}
api {
}
}
9 changes: 9 additions & 0 deletions server/testdata/integration/spa_api/02_couper.hcl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
server "spa" {
spa "spa1" {
base_path = "/"
bootstrap_file = "01_app.html"
paths = ["/**"]
}
api {
}
}
2 changes: 1 addition & 1 deletion server/testdata/integration/vhosts/01_app.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
<html><body><title>SPA_01</title></body></html>
<html><title>SPA_01</title><body></body></html>
2 changes: 1 addition & 1 deletion server/testdata/integration/vhosts/02_app.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
<html><body><title>SPA_02</title></body></html>
<html><title>SPA_02</title><body></body></html>
2 changes: 1 addition & 1 deletion server/testdata/integration/vhosts/htdocs_01/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
<html><body><title>FS_01</title></body></html>
<html><title>FS_01</title><body></body></html>
2 changes: 1 addition & 1 deletion server/testdata/integration/vhosts/htdocs_02/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
<html><body><title>FS_02</title></body></html>
<html><title>FS_02</title><body></body></html>
2 changes: 1 addition & 1 deletion server/testdata/integration/vhosts/htdocs_03/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
<html><body><title>FS_03</title></body></html>
<html><title>FS_03</title><body></body></html>
Loading