Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitlab-pages.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app.go7
-rw-r--r--internal/config/config.go4
-rw-r--r--internal/config/flags.go1
-rw-r--r--internal/httperrors/httperrors.go13
-rw-r--r--internal/httperrors/httperrors_test.go12
-rw-r--r--internal/urilimiter/urilimiter.go23
-rw-r--r--internal/urilimiter/urilimiter_test.go70
-rw-r--r--test/acceptance/urilimiter_test.go56
8 files changed, 183 insertions, 3 deletions
diff --git a/app.go b/app.go
index cddf8d49..3f286033 100644
--- a/app.go
+++ b/app.go
@@ -39,6 +39,7 @@ import (
"gitlab.com/gitlab-org/gitlab-pages/internal/serving/disk/zip"
"gitlab.com/gitlab-org/gitlab-pages/internal/source"
"gitlab.com/gitlab-org/gitlab-pages/internal/source/gitlab"
+ "gitlab.com/gitlab-org/gitlab-pages/internal/urilimiter"
"gitlab.com/gitlab-org/gitlab-pages/metrics"
)
@@ -292,10 +293,10 @@ func (a *theApp) buildHandlerPipeline() (http.Handler, error) {
handler = handlePanicMiddleware(handler)
handler = correlation.InjectCorrelationID(handler, correlationOpts...)
- // This MUST be the last handler!
- // This handler blocks unknown HTTP methods,
- // being the last means it will be evaluated first
+ // These middlewares MUST be added in the end.
+ // Being last means they will be evaluated first
// preventing any operation on bogus requests.
+ handler = urilimiter.NewMiddleware(handler, a.config.General.MaxURILength)
handler = rejectmethods.NewMiddleware(handler)
return handler, nil
diff --git a/internal/config/config.go b/internal/config/config.go
index 3e03f7d6..c6f91db0 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -47,6 +47,7 @@ type Config struct {
type General struct {
Domain string
MaxConns int
+ MaxURILength int
MetricsAddress string
RedirectHTTP bool
RootCertificate []byte
@@ -181,6 +182,7 @@ func loadConfig() (*Config, error) {
General: General{
Domain: strings.ToLower(*pagesDomain),
MaxConns: *maxConns,
+ MaxURILength: *maxURILength,
MetricsAddress: *metricsAddress,
RedirectHTTP: *redirectHTTP,
RootDir: *pagesRoot,
@@ -307,6 +309,8 @@ func LogConfig(config *Config) {
"enable-disk": config.GitLab.EnableDisk,
"auth-redirect-uri": config.Authentication.RedirectURI,
"auth-scope": config.Authentication.Scope,
+ "max-conns": config.General.MaxConns,
+ "max-uri-length": config.General.MaxURILength,
"zip-cache-expiration": config.Zip.ExpirationInterval,
"zip-cache-cleanup": config.Zip.CleanupInterval,
"zip-cache-refresh": config.Zip.RefreshInterval,
diff --git a/internal/config/flags.go b/internal/config/flags.go
index c61447c7..6c9bd4a6 100644
--- a/internal/config/flags.go
+++ b/internal/config/flags.go
@@ -51,6 +51,7 @@ var (
redirectURI = flag.String("auth-redirect-uri", "", "GitLab application redirect URI")
authScope = flag.String("auth-scope", "api", "Scope to be used for authentication (must match GitLab Pages OAuth application settings)")
maxConns = flag.Int("max-conns", 0, "Limit on the number of concurrent connections to the HTTP, HTTPS or proxy listeners, 0 for no limit")
+ maxURILength = flag.Int("max-uri-length", 1024, "Limit the length of URI, 0 for unlimited.")
insecureCiphers = flag.Bool("insecure-ciphers", false, "Use default list of cipher suites, may contain insecure ones like 3DES and RC4")
tlsMinVersion = flag.String("tls-min-version", "tls1.2", tls.FlagUsage("min"))
tlsMaxVersion = flag.String("tls-max-version", "", tls.FlagUsage("max"))
diff --git a/internal/httperrors/httperrors.go b/internal/httperrors/httperrors.go
index 8e61d590..a96e4d85 100644
--- a/internal/httperrors/httperrors.go
+++ b/internal/httperrors/httperrors.go
@@ -34,6 +34,14 @@ var (
<p>Make sure the address is correct and that the page hasn't moved.</p>
<p>Please contact your GitLab administrator if you think this is a mistake.</p>`,
}
+ content414 = content{
+ status: http.StatusRequestURITooLong,
+ title: "Request URI Too Long (414)",
+ statusString: "414",
+ header: "Request URI Too Long.",
+ subHeader: `<p>The URI provided was too long for the server to process.</p>
+ <p>Try to make the request URI shorter.</p>`,
+ }
content429 = content{
http.StatusTooManyRequests,
@@ -184,6 +192,11 @@ func Serve404(w http.ResponseWriter) {
serveErrorPage(w, content404)
}
+// Serve414 returns a 414 error response / HTML page to the http.ResponseWriter
+func Serve414(w http.ResponseWriter) {
+ serveErrorPage(w, content414)
+}
+
// Serve429 returns a 429 error response / HTML page to the http.ResponseWriter
func Serve429(w http.ResponseWriter) {
serveErrorPage(w, content429)
diff --git a/internal/httperrors/httperrors_test.go b/internal/httperrors/httperrors_test.go
index b003ce6f..671890f8 100644
--- a/internal/httperrors/httperrors_test.go
+++ b/internal/httperrors/httperrors_test.go
@@ -92,6 +92,18 @@ func TestServe404(t *testing.T) {
require.Contains(t, w.Content(), content404.subHeader)
}
+func TestServe414(t *testing.T) {
+ w := newTestResponseWriter(httptest.NewRecorder())
+ Serve414(w)
+ require.Equal(t, w.Header().Get("Content-Type"), "text/html; charset=utf-8")
+ require.Equal(t, w.Header().Get("X-Content-Type-Options"), "nosniff")
+ require.Equal(t, w.Status(), content414.status)
+ require.Contains(t, w.Content(), content414.title)
+ require.Contains(t, w.Content(), content414.statusString)
+ require.Contains(t, w.Content(), content414.header)
+ require.Contains(t, w.Content(), content414.subHeader)
+}
+
func TestServe500(t *testing.T) {
w := newTestResponseWriter(httptest.NewRecorder())
Serve500(w)
diff --git a/internal/urilimiter/urilimiter.go b/internal/urilimiter/urilimiter.go
new file mode 100644
index 00000000..65d664a5
--- /dev/null
+++ b/internal/urilimiter/urilimiter.go
@@ -0,0 +1,23 @@
+package urilimiter
+
+import (
+ "net/http"
+
+ "gitlab.com/gitlab-org/gitlab-pages/internal/httperrors"
+)
+
+func NewMiddleware(handler http.Handler, limit int) http.Handler {
+ if limit == 0 {
+ return handler
+ }
+
+ return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ if len(r.RequestURI) > limit {
+ httperrors.Serve414(w)
+
+ return
+ }
+
+ handler.ServeHTTP(w, r)
+ })
+}
diff --git a/internal/urilimiter/urilimiter_test.go b/internal/urilimiter/urilimiter_test.go
new file mode 100644
index 00000000..0ac89ea0
--- /dev/null
+++ b/internal/urilimiter/urilimiter_test.go
@@ -0,0 +1,70 @@
+package urilimiter
+
+import (
+ "fmt"
+ "io"
+ "net/http"
+ "net/http/httptest"
+ "testing"
+
+ "github.com/stretchr/testify/require"
+)
+
+func TestNewMiddleware(t *testing.T) {
+ handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ fmt.Fprint(w, "hello")
+ })
+
+ tests := map[string]struct {
+ limit int
+ url string
+ expectedStatus int
+ }{
+ "with_disabled_middleware": {
+ limit: 0,
+ url: "/index.html",
+ expectedStatus: http.StatusOK,
+ },
+ "with_limit_set_to_request_length": {
+ limit: 17,
+ url: "/index.html?q=a#b",
+ expectedStatus: http.StatusOK,
+ },
+ "with_uri_length_exceeding_the_limit": {
+ limit: 17,
+ url: "/index1.html?q=a#b",
+ expectedStatus: http.StatusRequestURITooLong,
+ },
+ "with_uri_length_exceeding_the_limit_with_query": {
+ limit: 17,
+ url: "/index.html?q=aa#b",
+ expectedStatus: http.StatusRequestURITooLong,
+ },
+ "with_uri_length_exceeding_the_limit_with_fragment": {
+ limit: 17,
+ url: "/index.html?q=a#bb",
+ expectedStatus: http.StatusRequestURITooLong,
+ },
+ }
+ for tn, tt := range tests {
+ t.Run(tn, func(t *testing.T) {
+ middleware := NewMiddleware(handler, tt.limit)
+
+ ww := httptest.NewRecorder()
+ rr := httptest.NewRequest(http.MethodGet, tt.url, nil)
+
+ middleware.ServeHTTP(ww, rr)
+
+ res := ww.Result()
+ defer res.Body.Close()
+
+ require.Equal(t, tt.expectedStatus, res.StatusCode)
+ if tt.expectedStatus == http.StatusOK {
+ b, err := io.ReadAll(res.Body)
+ require.NoError(t, err)
+
+ require.Equal(t, "hello", string(b))
+ }
+ })
+ }
+}
diff --git a/test/acceptance/urilimiter_test.go b/test/acceptance/urilimiter_test.go
new file mode 100644
index 00000000..5e97921f
--- /dev/null
+++ b/test/acceptance/urilimiter_test.go
@@ -0,0 +1,56 @@
+package acceptance_test
+
+import (
+ "io"
+ "net/http"
+ "testing"
+
+ "github.com/stretchr/testify/require"
+)
+
+// TestURILimits proves fix for https://gitlab.com/gitlab-org/gitlab-pages/-/issues/659
+func TestURILimits(t *testing.T) {
+ tests := map[string]struct {
+ limit string
+ path string
+ expectedStatus int
+ }{
+ "with_disabled_limit": {
+ limit: "0",
+ path: "project/",
+ expectedStatus: http.StatusOK,
+ },
+ "with_limit_set_to_request_length": {
+ limit: "19",
+ path: "/project/index.html",
+ expectedStatus: http.StatusOK,
+ },
+ "with_uri_length_exceeding_the_limit": {
+ limit: "19",
+ path: "/project/index1.html",
+ expectedStatus: http.StatusRequestURITooLong,
+ },
+ }
+
+ for tn, tt := range tests {
+ t.Run(tn, func(t *testing.T) {
+ RunPagesProcess(t, withListeners([]ListenSpec{httpsListener}), withExtraArgument("max-uri-length", tt.limit))
+
+ rsp, err := GetPageFromListener(t, httpsListener, "group.gitlab-example.com", tt.path)
+ require.NoError(t, err)
+ defer func() {
+ require.NoError(t, rsp.Body.Close())
+ }()
+
+ require.Equal(t, tt.expectedStatus, rsp.StatusCode)
+
+ b, err := io.ReadAll(rsp.Body)
+ require.NoError(t, err)
+ if tt.expectedStatus == http.StatusOK {
+ require.Equal(t, "project-subdir\n", string(b))
+ } else {
+ require.Contains(t, string(b), "Request URI Too Long")
+ }
+ })
+ }
+}
generated by cgit v1.2.3 (git 2.25.1) at 2024-08-23 10:18:34 +0300