diff --git a/go.mod b/go.mod index 5787c8b18..7b68c180c 100644 --- a/go.mod +++ b/go.mod @@ -64,6 +64,7 @@ require ( go.uber.org/mock v0.6.0 golang.org/x/crypto v0.53.0 golang.org/x/image v0.20.0 + golang.org/x/net v0.56.0 golang.org/x/term v0.44.0 golang.org/x/text v0.39.0 gopkg.in/gomail.v2 v2.0.0-20160411212932-81ebce5c23df @@ -170,7 +171,6 @@ require ( go.uber.org/zap v1.27.0 // indirect golang.org/x/arch v0.10.0 // indirect golang.org/x/exp v0.0.0-20240909161429-701f63a606c0 // indirect - golang.org/x/net v0.56.0 // indirect golang.org/x/sys v0.46.0 // indirect golang.org/x/tools v0.47.0 // indirect google.golang.org/protobuf v1.34.2 // indirect diff --git a/internal/controller/template_controller.go b/internal/controller/template_controller.go index 31cc5152a..0f2f5b68b 100644 --- a/internal/controller/template_controller.go +++ b/internal/controller/template_controller.go @@ -20,6 +20,7 @@ package controller import ( + "bytes" "encoding/json" "fmt" "html/template" @@ -50,13 +51,17 @@ import ( "github.com/apache/answer/ui" "github.com/gin-gonic/gin" "github.com/segmentfault/pacman/log" + "golang.org/x/net/html" ) var SiteUrl = "" type TemplateController struct { - scriptPath []string - cssPath string + scriptPath []string + // cssPath lists every stylesheet the frontend build emits, in document + // order; a build that emits more than one entry stylesheet needs all of + // them, not just the first, or server-rendered pages come back unstyled. + cssPath []string templateRenderController *templaterender.TemplateRenderController siteInfoService siteinfo_common.SiteInfoCommonService eventQueueService eventqueue.Service @@ -83,24 +88,64 @@ func NewTemplateController( questionService: questionService, } } -func GetStyle() (script []string, css string) { +func GetStyle() (script []string, css []string) { file, err := ui.Build.ReadFile("build/index.html") if err != nil { return } - scriptRegexp := regexp.MustCompile(``) - scriptData := scriptRegexp.FindAllStringSubmatch(string(file), -1) - for _, s := range scriptData { - if len(s) == 2 { - script = append(script, s[1]) + + // Script and stylesheet tags are read from the parsed document, so + // attribute order, attribute set (module vs classic scripts), and + // quoting do not matter. That shape has already changed once; a + // bundler change that breaks it now fails the guarding test instead + // of silently shipping pages with no JS or CSS. + doc, err := html.Parse(bytes.NewReader(file)) + if err != nil { + return + } + + attr := func(n *html.Node, key string) (string, bool) { + for _, a := range n.Attr { + if a.Key == key { + return a.Val, true + } } + return "", false + } + isStylesheet := func(n *html.Node) bool { + rel, ok := attr(n, "rel") + if !ok { + return false + } + for tok := range strings.FieldsSeq(rel) { + if strings.EqualFold(tok, "stylesheet") { + return true + } + } + return false } - cssRegexp := regexp.MustCompile(``) - cssListData := cssRegexp.FindStringSubmatch(string(file)) - if len(cssListData) == 2 { - css = cssListData[1] + var walk func(*html.Node) + walk = func(n *html.Node) { + if n.Type == html.ElementNode { + switch n.Data { + case "script": + if src, ok := attr(n, "src"); ok && src != "" { + script = append(script, src) + } + case "link": + if isStylesheet(n) { + if href, ok := attr(n, "href"); ok && href != "" { + css = append(css, href) + } + } + } + } + for c := n.FirstChild; c != nil; c = c.NextSibling { + walk(c) + } } + walk(doc) return } func (tc *TemplateController) SiteInfo(ctx *gin.Context) *schema.TemplateSiteInfoResp { @@ -560,7 +605,7 @@ func (tc *TemplateController) Page404(ctx *gin.Context) { func (tc *TemplateController) html(ctx *gin.Context, code int, tpl string, siteInfo *schema.TemplateSiteInfoResp, data gin.H) { prefix := "" - cssPath := "" + cssPath := make([]string, len(tc.cssPath)) scriptPath := make([]string, len(tc.scriptPath)) _ = plugin.CallCDN(func(fn plugin.CDN) error { @@ -572,7 +617,9 @@ func (tc *TemplateController) html(ctx *gin.Context, code int, tpl string, siteI if prefix[len(prefix)-1:] == "/" { prefix = strings.TrimSuffix(prefix, "/") } - cssPath = prefix + tc.cssPath + for i, path := range tc.cssPath { + cssPath[i] = prefix + path + } for i, path := range tc.scriptPath { scriptPath[i] = prefix + path } diff --git a/internal/controller/template_controller_test.go b/internal/controller/template_controller_test.go new file mode 100644 index 000000000..74c0db0a6 --- /dev/null +++ b/internal/controller/template_controller_test.go @@ -0,0 +1,78 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 controller + +import ( + "strings" + "testing" + + "github.com/apache/answer/ui" + "github.com/stretchr/testify/require" +) + +// GetStyle scrapes the script and stylesheet paths out of the built +// index.html and every server-rendered page reuses them. The scrape is +// coupled to the exact attribute order and attribute set that the frontend +// build tool writes into those tags, and nothing in the system reports a +// mismatch: the frontend build still succeeds, the dev server still works, +// the binary still compiles, and the server-rendered pages simply come back +// with no script tags and no stylesheet. +// +// Assert the coupling directly so a change to the emitted tag shape fails +// here instead of shipping. +func TestGetStyleResolvesBuiltAssets(t *testing.T) { + const builtIndexPath = "build/index.html" + + raw, err := ui.Build.ReadFile(builtIndexPath) + if err != nil { + t.Skipf("no frontend build embedded at %s; build the frontend and re-run: %v", builtIndexPath, err) + } + + scripts, css := GetStyle() + + require.NotEmpty(t, scripts, + "no script sources parsed out of %s; server-rendered pages would load without any JavaScript", builtIndexPath) + for i, src := range scripts { + require.NotEmpty(t, src, "script source %d parsed out of %s is empty", i, builtIndexPath) + } + + require.NotEmpty(t, css, + "no stylesheet href parsed out of %s; server-rendered pages would load unstyled", builtIndexPath) + for i, href := range css { + require.NotEmpty(t, href, + "stylesheet href %d parsed out of %s is empty; server-rendered pages would load unstyled", i, builtIndexPath) + } + + // Finding every stylesheet matters as much as finding one. The build emits + // more than a single entry stylesheet, and a parser that stopped at the + // first one would still satisfy every assertion above while half the page's + // CSS silently stopped loading. That regression has happened once already. + // + // Count them again by a deliberately different and cruder method than the + // parser uses, so the two have to agree. It is a lower bound: a build that + // quotes attributes differently drives this to zero and the comparison + // simply stops constraining, which is why it supplements the assertions + // above rather than replacing them. + declared := strings.Count(string(raw), `rel="stylesheet"`) + require.GreaterOrEqual(t, len(css), declared, + "%s declares at least %d stylesheets but only %d were parsed out of it; "+ + "server-rendered pages would load missing part of their CSS", + builtIndexPath, declared, len(css)) +} diff --git a/ui/template/header.html b/ui/template/header.html index d5d9a18ac..f9f0468b6 100644 --- a/ui/template/header.html +++ b/ui/template/header.html @@ -34,7 +34,9 @@ - + {{range $path := .cssPath}} + + {{end}}