feature: http badrequest code instead of 500, update tests, modernize

.github/workflows/linux.yml

@@ -42,7 +42,7 @@ jobs:
           echo "dir=$(composer config cache-files-dir)" >> $GITHUB_OUTPUT
 
       - name: Init Composer Cache # Docs: <https://git.io/JfAKn#php---composer>
-        uses: actions/cache@v3
+        uses: actions/cache@v4
         with:
           path: ${{ steps.composer-cache.outputs.dir }}
           key: ${{ runner.os }}-composer-${{ matrix.php }}-${{ hashFiles('**/composer.json') }}
@@ -52,7 +52,7 @@ jobs:
         run: cd tests/php_test_files && composer update --prefer-dist --no-progress --ansi
 
       - name: Init Go modules Cache # Docs: <https://git.io/JfAKn#go---modules>
-        uses: actions/cache@v3
+        uses: actions/cache@v4
         with:
           path: ~/go/pkg/mod
           key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }}
@@ -95,25 +95,35 @@ jobs:
         uses: actions/upload-artifact@v4
         with:
           name: coverage
-          path: ./tests/coverage-ci/httpu.out
+          path: ./tests/coverage-ci
 
   codecov:
     name: Upload codecov
     runs-on: ubuntu-latest
     needs:
       - http_test
-
     timeout-minutes: 60
     steps:
+      - name: Check out code
+        uses: actions/checkout@v6
       - name: Download code coverage results
         uses: actions/download-artifact@v8
+        with:
+          name: coverage
+          path: coverage
       - run: |
           echo 'mode: atomic' > summary.txt
-          tail -q -n +2 *.out >> summary.txt
-          sed -i '2,${/roadrunner/!d}' summary.txt
-
+          tail -q -n +2 coverage/*.out >> summary.txt
+          awk '
+            NR == 1 { print; next }
+            /^github\.com\/roadrunner-server\/http\/v5\// {
+              sub(/^github\.com\/roadrunner-server\/http\/v5\//, "", $0)
+              print
+            }
+          ' summary.txt > summary.filtered.txt
+          mv summary.filtered.txt summary.txt
       - name: upload to codecov
         uses: codecov/codecov-action@v5 # Docs: <https://github.com/codecov/codecov-action>
         with:
-          file: summary.txt
+          files: summary.txt
           fail_ci_if_error: false

.golangci.yml

@@ -38,7 +38,7 @@ linters:
       threshold: 100
     goconst:
       min-len: 2
-      min-occurrences: 3
+      min-occurrences: 10
     gocyclo:
       min-complexity: 20
     godot:

handler/handler.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	stderr "errors"
 	"html/template"
+	"io"
 	"net/http"
 	"sync"
 	"time"
@@ -140,6 +141,8 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		status := http.StatusInternalServerError
 		if _, ok := stderr.AsType[*http.MaxBytesError](err); ok {
 			status = http.StatusRequestEntityTooLarge
+		} else if stderr.Is(err, io.EOF) || stderr.Is(err, io.ErrUnexpectedEOF) {
+			status = http.StatusBadRequest
 		}
 		http.Error(w, errors.E(op, err).Error(), status)
 		h.log.Error(

handler/handler_test.go

@@ -0,0 +1,236 @@
+package handler
+
+import (
+	"context"
+	"crypto/tls"
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"strings"
+	"testing"
+
+	"github.com/roadrunner-server/errors"
+	"github.com/roadrunner-server/http/v5/api"
+	"github.com/roadrunner-server/http/v5/config"
+	"github.com/roadrunner-server/pool/payload"
+	"github.com/roadrunner-server/pool/pool"
+	staticPool "github.com/roadrunner-server/pool/pool/static_pool"
+	"github.com/roadrunner-server/pool/worker"
+	"go.uber.org/zap"
+)
+
+// mockPool satisfies api.Pool. Only Exec is exercised by the tests below.
+type mockPool struct{ execErr error }
+
+func (m *mockPool) Workers() []*worker.Process           { return nil }
+func (m *mockPool) RemoveWorker(_ context.Context) error { return nil }
+func (m *mockPool) AddWorker() error                     { return nil }
+func (m *mockPool) Exec(_ context.Context, _ *payload.Payload, _ chan struct{}) (chan *staticPool.PExec, error) {
+	return nil, m.execErr
+}
+func (m *mockPool) Reset(_ context.Context) error { return nil }
+func (m *mockPool) Destroy(_ context.Context)     {}
+
+func newTestHandler(t *testing.T, cfg *config.Config, p api.Pool) *Handler {
+	t.Helper()
+	h, err := NewHandler(cfg, p, zap.NewNop())
+	if err != nil {
+		t.Fatal(err)
+	}
+	return h
+}
+
+func defaultCfg() *config.Config {
+	return &config.Config{
+		MaxRequestSize:    1024,
+		InternalErrorCode: 500,
+		Uploads: &config.Uploads{
+			Dir:       os.TempDir(),
+			Forbidden: map[string]struct{}{},
+			Allowed:   map[string]struct{}{},
+		},
+	}
+}
+
+// ── Group A: ServeHTTP errors before pool.Exec (nil pool is safe) ────────────
+
+func TestServeHTTP_InvalidMultipart_Returns400(t *testing.T) {
+	h := newTestHandler(t, defaultCfg(), nil)
+
+	// Boundary declared in header but body has no valid multipart parts → EOF.
+	req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(""))
+	req.Header.Set("Content-Type", "multipart/form-data; boundary=1111")
+
+	rr := httptest.NewRecorder()
+	h.ServeHTTP(rr, req)
+
+	if rr.Code != http.StatusBadRequest {
+		t.Errorf("expected 400, got %d", rr.Code)
+	}
+}
+
+func TestServeHTTP_StreamBody_MaxBytesExceeded_Returns413(t *testing.T) {
+	h := newTestHandler(t, defaultCfg(), nil)
+
+	req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader("this body is too long"))
+	req.Header.Set("Content-Type", "application/json")
+
+	rr := httptest.NewRecorder()
+	// Wrap body so that reading more than 5 bytes returns *http.MaxBytesError.
+	req.Body = http.MaxBytesReader(rr, req.Body, 5)
+
+	h.ServeHTTP(rr, req)
+
+	if rr.Code != http.StatusRequestEntityTooLarge {
+		t.Errorf("expected 413, got %d", rr.Code)
+	}
+}
+
+func TestServeHTTP_TruncatedMultipart_Returns400(t *testing.T) {
+	h := newTestHandler(t, defaultCfg(), nil)
+
+	// Multipart body with an open part but no closing boundary → ErrUnexpectedEOF.
+	body := "--1111\r\nContent-Disposition: form-data; name=\"f\"\r\n\r\nval"
+	req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(body))
+	req.Header.Set("Content-Type", "multipart/form-data; boundary=1111")
+
+	rr := httptest.NewRecorder()
+	h.ServeHTTP(rr, req)
+
+	if rr.Code != http.StatusBadRequest {
+		t.Errorf("expected 400, got %d", rr.Code)
+	}
+}
+
+// ── Group B: Direct calls to handleError, URI, FetchIP ───────────────────────
+
+func TestHandleError_NoFreeWorkers_SetsNoWorkersHeader(t *testing.T) {
+	h := newTestHandler(t, defaultCfg(), nil)
+
+	rr := httptest.NewRecorder()
+	h.handleError(rr, errors.E(errors.NoFreeWorkers))
+
+	if got := rr.Header().Get(noWorkers); got != trueStr {
+		t.Errorf("expected No-Workers: true, got %q", got)
+	}
+	if rr.Code != 500 {
+		t.Errorf("expected status 500, got %d", rr.Code)
+	}
+}
+
+func TestHandleError_CustomInternalCode(t *testing.T) {
+	cfg := defaultCfg()
+	cfg.InternalErrorCode = 503
+	h := newTestHandler(t, cfg, nil)
+
+	rr := httptest.NewRecorder()
+	h.handleError(rr, fmt.Errorf("boom"))
+
+	if rr.Code != 503 {
+		t.Errorf("expected 503, got %d", rr.Code)
+	}
+}
+
+func TestHandleError_DebugMode_WritesEscapedError(t *testing.T) {
+	cfg := defaultCfg()
+	cfg.Pool = &pool.Config{Debug: true}
+	h := newTestHandler(t, cfg, nil)
+
+	rr := httptest.NewRecorder()
+	h.handleError(rr, fmt.Errorf("boom<script>"))
+
+	body := rr.Body.String()
+	if strings.Contains(body, "<script>") {
+		t.Error("response body contains unescaped <script> tag (XSS risk)")
+	}
+	if !strings.Contains(body, "&lt;script&gt;") {
+		t.Errorf("expected HTML-escaped error in body, got: %q", body)
+	}
+}
+
+func TestURI_PlainHTTP(t *testing.T) {
+	r := httptest.NewRequest(http.MethodGet, "/path?q=1", nil)
+	r.Host = "example.com"
+
+	got := URI(r)
+	want := "http://example.com/path?q=1"
+	if got != want {
+		t.Errorf("URI() = %q, want %q", got, want)
+	}
+}
+
+func TestURI_TLSRequest_HTTPSScheme(t *testing.T) {
+	r := httptest.NewRequest(http.MethodGet, "/path?q=1", nil)
+	r.Host = "example.com"
+	r.TLS = &tls.ConnectionState{}
+
+	got := URI(r)
+	want := "https://example.com/path?q=1"
+	if got != want {
+		t.Errorf("URI() = %q, want %q", got, want)
+	}
+}
+
+func TestURI_StripsCRLFInjection(t *testing.T) {
+	r := httptest.NewRequest(http.MethodGet, "/path", nil)
+	r.Host = "example.com"
+	// Inject CRLF into the raw query — a classic HTTP response-splitting vector.
+	r.URL.RawQuery = "param=value\r\nX-Injected: true"
+
+	got := URI(r)
+	if strings.ContainsAny(got, "\r\n") {
+		t.Errorf("URI() result contains CRLF characters: %q", got)
+	}
+}
+
+func TestFetchIP_StripPortFromIPv4(t *testing.T) {
+	got := FetchIP("127.0.0.1:8080", zap.NewNop())
+	if got != "127.0.0.1" {
+		t.Errorf("FetchIP() = %q, want %q", got, "127.0.0.1")
+	}
+}
+
+func TestFetchIP_BareIPv6_NoPort(t *testing.T) {
+	// "::1" contains colons but is not host:port — SplitHostPort fails,
+	// ParseIP succeeds.
+	got := FetchIP("::1", zap.NewNop())
+	if got != "::1" {
+		t.Errorf("FetchIP() = %q, want %q", got, "::1")
+	}
+}
+
+// ── Group C: mockPool tests ───────────────────────────────────────────────────
+
+func TestServeHTTP_PoolExecError_Returns500(t *testing.T) {
+	mp := &mockPool{execErr: fmt.Errorf("worker died")}
+	h := newTestHandler(t, defaultCfg(), mp)
+
+	req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(`{"key":"val"}`))
+	req.Header.Set("Content-Type", "application/json")
+
+	rr := httptest.NewRecorder()
+	h.ServeHTTP(rr, req)
+
+	if rr.Code != 500 {
+		t.Errorf("expected 500, got %d", rr.Code)
+	}
+}
+
+func TestServeHTTP_NoFreeWorkers_SetsHeader(t *testing.T) {
+	mp := &mockPool{execErr: errors.E(errors.NoFreeWorkers)}
+	h := newTestHandler(t, defaultCfg(), mp)
+
+	req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(`{"key":"val"}`))
+	req.Header.Set("Content-Type", "application/json")
+
+	rr := httptest.NewRecorder()
+	h.ServeHTTP(rr, req)
+
+	if got := rr.Header().Get(noWorkers); got != trueStr {
+		t.Errorf("expected No-Workers: true, got %q", got)
+	}
+	if rr.Code != 500 {
+		t.Errorf("expected 500, got %d", rr.Code)
+	}
+}

handler/uploads.go

@@ -44,15 +44,13 @@ func (u *Uploads) MarshalJSON() ([]byte, error) {
 // will be handled individually.
 func (u *Uploads) Open(log *zap.Logger, dir string, forbid, allow map[string]struct{}) {
 	var wg sync.WaitGroup
-	for i := range u.list {
-		wg.Add(1)
-		go func(f *FileUpload) {
-			defer wg.Done()
+	for _, f := range u.list {
+		wg.Go(func() {
 			err := f.Open(dir, forbid, allow)
 			if err != nil && log != nil {
 				log.Error("error opening the file", zap.Error(err))
 			}
-		}(u.list[i])
+		})
 	}
 
 	wg.Wait()

tests/configs/.rr-http-otel.yaml

@@ -11,23 +11,13 @@ server:
 http:
   address: 127.0.0.1:43239
   max_request_size: 1024
-  middleware: [ gzip, otel ]
+  middleware: [ gzip, tracetestOtel ]
   pool:
     num_workers: 2
     max_jobs: 0
     allocate_timeout: 60s
     destroy_timeout: 1s
 
-otel:
-  resource:
-    service_name: "rr_test"
-    service_version: "1.0.0"
-    service_namespace: "RR-Shop"
-    service_instance_id: "UUID"
-  insecure: false
-  compress: true
-  exporter: stderr
-
 logs:
   mode: development
   level: info

tests/configs/.rr-http-otel2.yaml

@@ -14,22 +14,12 @@ server:
 http:
   address: 127.0.0.1:43239
   max_request_size: 1024
-  middleware: [ gzip, otel ]
+  middleware: [ gzip, tracetestOtel ]
   pool:
     num_workers: 2
     allocate_timeout: 60s
     destroy_timeout: 1s
 
-otel:
-  resource:
-    service_name: "rr_test"
-    service_version: "1.0.0"
-    service_namespace: "RR-Shop2"
-    service_instance_id: "UUID2"
-  insecure: false
-  compress: true
-  exporter: stderr
-
 logs:
   mode: development
   level: debug