From 34e018d16cdeaf3ed028f90b64d670062f0d3ef8 Mon Sep 17 00:00:00 2001 From: sakirr Date: Wed, 12 Aug 2026 16:02:08 +0530 Subject: [PATCH] Fix: mock OSS registry server serves file bytes directly instead of through html/template (#7297) * Fix: mock OSS registry server serves file bytes directly instead of through html/template ossHandler parsed static file content as a Go template before serving it, with no placeholders actually used anywhere. On a parse failure the error branch didn't stop execution and fell through to the success render right after. Turns out worse than that: Parse returns a nil *Template on failure, so Execute on it panics. Drops the templating, writes bytes directly. Added tests, including a fixture that reproduces the parse failure; confirmed it panics on the old code and passes with the fix. Fixes #7296 Signed-off-by: sakirr05 * Chore: retrigger CI Signed-off-by: sakirr05 --------- Signed-off-by: sakirr05 Co-authored-by: sakirr05 --- .../testdata/template-edge-case/broken.txt | 1 + e2e/addon/mock/vela_addon_mock_server.go | 31 ++----- e2e/addon/mock/vela_addon_mock_server_test.go | 90 +++++++++++++++++++ 3 files changed, 99 insertions(+), 23 deletions(-) create mode 100644 e2e/addon/mock/testdata/template-edge-case/broken.txt create mode 100644 e2e/addon/mock/vela_addon_mock_server_test.go diff --git a/e2e/addon/mock/testdata/template-edge-case/broken.txt b/e2e/addon/mock/testdata/template-edge-case/broken.txt new file mode 100644 index 000000000..8bcdf36e8 --- /dev/null +++ b/e2e/addon/mock/testdata/template-edge-case/broken.txt @@ -0,0 +1 @@ +plain content with an unbalanced action delimiter: {{ .Whatever diff --git a/e2e/addon/mock/vela_addon_mock_server.go b/e2e/addon/mock/vela_addon_mock_server.go index 279d99e07..9d9575861 100644 --- a/e2e/addon/mock/vela_addon_mock_server.go +++ b/e2e/addon/mock/vela_addon_mock_server.go @@ -20,7 +20,6 @@ import ( "embed" "encoding/xml" "fmt" - "html/template" "io/fs" "log" "net/http" @@ -70,38 +69,24 @@ var ossHandler http.HandlerFunc = func(rw http.ResponseWriter, req *http.Request } } data, err := xml.Marshal(res) - error := map[string]error{"error": err} - // Make and parse the data - t, err := template.New("").Parse(string(data)) if err != nil { - // Render the data - t.Execute(rw, error) + http.Error(rw, err.Error(), http.StatusInternalServerError) + return } - // Render the data - t.Execute(rw, data) - + _, _ = rw.Write(data) } else { - found := false for _, p := range paths { if queryPath == p.path { file, err := testData.ReadFile(path.Join("testdata", queryPath)) - error := map[string]error{"error": err} - // Make and parse the data - t, err := template.New("").Parse(string(file)) if err != nil { - // Render the data - t.Execute(rw, error) + http.Error(rw, err.Error(), http.StatusInternalServerError) + return } - found = true - t.Execute(rw, file) - break + _, _ = rw.Write(file) + return } } - if !found { - nf := "not found" - t, _ := template.New("").Parse(nf) - t.Execute(rw, nf) - } + _, _ = rw.Write([]byte("not found")) } } diff --git a/e2e/addon/mock/vela_addon_mock_server_test.go b/e2e/addon/mock/vela_addon_mock_server_test.go new file mode 100644 index 000000000..f69f3485a --- /dev/null +++ b/e2e/addon/mock/vela_addon_mock_server_test.go @@ -0,0 +1,90 @@ +/* +Copyright 2026 The KubeVela Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package main + +import ( + "encoding/xml" + "net/http/httptest" + "testing" + + "github.com/oam-dev/kubevela/pkg/addon" +) + +func TestOssHandlerServesFileBytesExactly(t *testing.T) { + want, err := testData.ReadFile("testdata/sample/metadata.yaml") + if err != nil { + t.Fatalf("failed to read fixture directly: %v", err) + } + + req := httptest.NewRequest("GET", "/sample/metadata.yaml", nil) + rec := httptest.NewRecorder() + ossHandler(rec, req) + + got := rec.Body.Bytes() + if string(got) != string(want) { + t.Fatalf("served bytes do not match source file exactly\nwant: %q\ngot: %q", want, got) + } +} + +// TestOssHandlerServesTemplateLikeBytesUnchanged guards against the handler +// treating served file content as a Go template. A file containing an +// unbalanced "{{" fails html/template.Parse; the handler must still return +// the file's bytes untouched rather than a template-error render. +func TestOssHandlerServesTemplateLikeBytesUnchanged(t *testing.T) { + want, err := testData.ReadFile("testdata/template-edge-case/broken.txt") + if err != nil { + t.Fatalf("failed to read fixture directly: %v", err) + } + + req := httptest.NewRequest("GET", "/template-edge-case/broken.txt", nil) + rec := httptest.NewRecorder() + ossHandler(rec, req) + + got := rec.Body.Bytes() + if string(got) != string(want) { + t.Fatalf("served bytes do not match source file exactly\nwant: %q\ngot: %q", want, got) + } +} + +func TestOssHandlerNotFound(t *testing.T) { + req := httptest.NewRequest("GET", "/does-not-exist.yaml", nil) + rec := httptest.NewRecorder() + ossHandler(rec, req) + + if got := rec.Body.String(); got != "not found" { + t.Fatalf("expected \"not found\", got %q", got) + } +} + +func TestOssHandlerListReturnsValidXML(t *testing.T) { + req := httptest.NewRequest("GET", "/?prefix=sample", nil) + rec := httptest.NewRecorder() + ossHandler(rec, req) + + var res addon.ListBucketResult + if err := xml.Unmarshal(rec.Body.Bytes(), &res); err != nil { + t.Fatalf("response is not valid XML: %v\nbody: %s", err, rec.Body.String()) + } + if res.Count == 0 { + t.Fatalf("expected at least one file under prefix \"sample\", got 0") + } + for _, f := range res.Files { + if f.Name[:len("sample")] != "sample" { + t.Fatalf("file %q does not match requested prefix", f.Name) + } + } +}