Fill the instance URL into the embedded frontend at serve time, and stop pinning compressor output in tests (#2198)

* Assert what the image endpoints promise rather than the compressor's output

Three tests pinned the exact bytes or the exact length of an encoded
image, so they fail on any toolchain whose deflate or png encoder emits
something different. CI pins go 1.25 and passes; go 1.27 fails all three,
while the images themselves are perfectly valid.

TestRest_QR now decodes both the golden file and the response and
compares the pixels, which is the same assertion about the qr code and
none about the encoder. The two resize cases assert the decoded image
fits the box resize was given and touches one of its sides, which is what
fitting to a box means and what the function actually promises.

Resolves #2200.

* Fill the instance URL into the embedded frontend at serve time

The widget falls back to a compiled-in URL whenever a page omits
`remark_config.host`. The bundler cannot know that URL, so it emits
`{% REMARK_URL %}` and each distribution substitutes it: the docker image
rewrites the files under its web root at container start, and the release
binary, which serves the build embedded in itself, had nothing doing it.
`prepare-release-assets.sh` filled the marker with `http://127.0.0.1:8080`
before the embed instead, so every copy of the binary shipped pointing at
the visitor's own loopback address, and on an https site the request is
blocked as mixed content besides.

It has been that way since v1.11.0, the first release to embed the
frontend, and the earlier binaries embedded none, so the tarball has never
served a correctly addressed widget.

The placeholder now survives into the embedded copy and the file server
fills it with the configured `REMARK_URL` as it serves, which is what the
docker image already does to its own copy. The image no longer bakes the
loopback address into its embedded copy either, so the fallback it keeps
for a missing web root is correct rather than misleading.

Substituted in html, js and mjs, the same set `docker-init.sh` rewrites,
and the served size is the substituted one so a response is neither
truncated nor left hanging.

Nothing exercised the marker the frontend build emits wherever the instance
url belongs. Every page in the suite sets `remark_config.host` from its own
origin, so the compiled-in fallback is never read, and a distribution that
stopped substituting would keep the suite green.

Two tests. The first reads the served bundles and pages back and asserts the
marker is gone from each and that what replaced it is this instance. The
second covers what the substitution is for: the widget document carries no
host of its own, since `iframe.html` builds its config from a query string the
parent never puts one in, so everything it requests is addressed with the
compiled-in url. It asserts the widget renders and that the config request
went to this instance.

The demo pages cannot show the second. Their loader builds the bundle's own
script url from `remark_config.host`, so a page without one never gets as far
as loading the widget.

Verified by disabling both substitution paths, the serve-time one and the
docker image's, and rebuilding: both tests fail. Editing the files on disk is
not enough, since the file server substitutes as it serves.

The served body now depends on remarkURL, but cacheControl builds its
etag from version and path only. An operator who notices the widget is
addressed to the wrong host, corrects REMARK_URL and restarts the same
binary gets 304 on revalidation, so the client keeps a bundle pointing
at the old host. Cache-Control is no-cache, so it revalidates every time
and never ages out of that state either.

That is the exact situation this substitution exists to fix, so the
validator has to carry the url.
This commit is contained in:
Dmitry Verkhoturov
2026-08-22 13:15:27 -05:00
committed by GitHub
parent 23be25d84a
commit 4793c1cd2c
10 changed files with 337 additions and 22 deletions
+1 -1
View File
@@ -60,7 +60,7 @@ git push origin backend/vX.Y.Z
GoReleaser must ignore `backend/*` tags in `.goreleaser.yml` so release notes and current-tag detection use only product tags. Docker image publishing stays separate and is handled by the existing Docker workflow.
For local artifact runs, install GoReleaser, Go 1.25, Node 24+, PNPM 10, and Perl, then use `make release`. The target runs a snapshot/no-publish GoReleaser build, leaves local artifacts and metadata in `dist/`, and cleans generated frontend embed files after GoReleaser exits. Do not run raw `goreleaser release` for local artifacts unless you also run `./scripts/cleanup-release-assets.sh` afterward.
For local artifact runs, install GoReleaser, Go 1.25, Node 24+ and PNPM 10, then use `make release`. The target runs a snapshot/no-publish GoReleaser build, leaves local artifacts and metadata in `dist/`, and cleans generated frontend embed files after GoReleaser exits. Do not run raw `goreleaser release` for local artifacts unless you also run `./scripts/cleanup-release-assets.sh` afterward.
## Milestones and Issue Labels
-1
View File
@@ -62,7 +62,6 @@ RUN apk --no-cache add gcc libc-dev
ADD backend /build/backend
# to embed the frontend files statically into Remark42 binary
COPY --from=build-frontend /srv/frontend/apps/remark42/public/ /build/backend/app/cmd/web/
RUN find /build/backend/app/cmd/web/ -regex '.*\.\(html\|js\|mjs\)$' -print -exec sed -i "s|{% REMARK_URL %}|http://127.0.0.1:8080|g" {} \;
WORKDIR /build/backend
RUN echo go version: `go version`
+14 -4
View File
@@ -394,7 +394,7 @@ func (s *Rest) routes() http.Handler {
log.Printf("[WARN] no embedded frontend, serving built-in assets only: %v", err)
embeddedFrontend = emptyFS{}
}
addFileServer(router, embeddedFrontend, s.WebRoot, s.Version)
addFileServer(router, embeddedFrontend, s.WebRoot, s.Version, s.RemarkURL)
return router
}
@@ -510,7 +510,7 @@ func (s *Rest) configCtrl(w http.ResponseWriter, r *http.Request) {
// serves /web from the frontend build, falling back to the assets embedded in the binary for
// names the build does not produce. the frontend build is read from webRoot on disk, or from the
// copy embedded at app/cmd/web when that directory is absent.
func addFileServer(r *routegroup.Bundle, embeddedFrontend fs.FS, webRoot, version string) {
func addFileServer(r *routegroup.Bundle, embeddedFrontend fs.FS, webRoot, version, remarkURL string) {
frontendFS := embeddedFrontend
if _, err := os.Stat(webRoot); err == nil {
@@ -520,12 +520,22 @@ func addFileServer(r *routegroup.Bundle, embeddedFrontend fs.FS, webRoot, versio
log.Printf("[INFO] run file server, embedded")
}
webFS := http.StripPrefix("/web", http.FileServer(http.FS(webFiles{frontend: frontendFS, embedded: webassets.FS})))
// wrapped rather than substituted once at startup: the disk root can change under a running
// server, and the docker image has already substituted its copy, where this is a no-op
sources := templatedFS{
fs: webFiles{frontend: frontendFS, embedded: webassets.FS},
remarkURL: remarkURL,
}
webFS := http.StripPrefix("/web", http.FileServer(http.FS(sources)))
r.HandleFunc("GET /web", http.RedirectHandler("/web/", http.StatusMovedPermanently).ServeHTTP)
r.With(rateLimiter(20),
R.Timeout(10*time.Second),
cacheControl(time.Hour, version),
// the served body now depends on remarkURL, so it has to be part of the validator. Without
// it an operator who corrects a wrong REMARK_URL and restarts the same binary keeps getting
// 304 on revalidation, and the client keeps a bundle addressed to the old host for good,
// since no-cache means it revalidates rather than aging out
cacheControl(time.Hour, version+":"+remarkURL),
).HandleFunc("GET /web/", func(w http.ResponseWriter, r *http.Request) {
// don't show dirs, just serve files
if strings.HasSuffix(r.URL.Path, "/") && len(r.URL.Path) > 1 && r.URL.Path != ("/web/") {
+19 -5
View File
@@ -4,6 +4,7 @@ import (
"bytes"
"encoding/json"
"fmt"
"image/png"
"io"
"mime/multipart"
"net/http"
@@ -986,13 +987,26 @@ func TestRest_QR(t *testing.T) {
assert.Equal(t, "image/png", r.Header.Get("Content-Type"))
assert.Equal(t, http.StatusOK, r.StatusCode)
// compare the image
// compare the decoded image rather than the encoded bytes: the pixels are what the endpoint
// promises, while the byte stream is whatever the toolchain's png encoder produces, and
// pinning that fails on a go release that changes it
fh, err := os.Open("testdata/qr_test.png")
defer func() { assert.NoError(t, fh.Close()) }()
assert.NoError(t, err)
img, err := io.ReadAll(fh)
assert.NoError(t, err)
assert.Equal(t, img, bdy)
require.NoError(t, err)
want, err := png.Decode(fh)
require.NoError(t, err)
got, err := png.Decode(bytes.NewReader(bdy))
require.NoError(t, err, "the endpoint did not return a decodable png")
require.Equal(t, want.Bounds(), got.Bounds(), "the qr code is not the size it used to be")
for y := want.Bounds().Min.Y; y < want.Bounds().Max.Y; y++ {
for x := want.Bounds().Min.X; x < want.Bounds().Max.X; x++ {
if want.At(x, y) != got.At(x, y) {
t.Fatalf("the qr code differs at %d,%d: want %v, got %v", x, y, want.At(x, y), got.At(x, y))
}
}
}
}
func TestRest_Info(t *testing.T) {
+1 -1
View File
@@ -189,7 +189,7 @@ func TestRest_FileServerBackendAssets(t *testing.T) {
func TestRest_FileServerEmbeddedFrontend(t *testing.T) {
frontend := fstest.MapFS{"index.html": {Data: []byte("embedded frontend index")}}
router := routegroup.New(http.NewServeMux())
addFileServer(router, frontend, filepath.Join(t.TempDir(), "absent"), "test-version")
addFileServer(router, frontend, filepath.Join(t.TempDir(), "absent"), "test-version", "https://remark.example.com")
ts := httptest.NewServer(router)
defer ts.Close()
+70
View File
@@ -1,7 +1,9 @@
package api
import (
"bytes"
"errors"
"io"
"io/fs"
"path/filepath"
"strings"
@@ -68,3 +70,71 @@ type emptyFS struct{}
func (emptyFS) Open(name string) (fs.File, error) {
return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrNotExist}
}
// remarkURLPlaceholder is what the frontend build carries wherever the instance URL belongs. The
// bundler cannot know that URL, so it emits this marker and every distribution fills it in: the
// docker image rewrites the files under the web root at container start, and the binary, which
// serves the build embedded in itself and has nothing to rewrite, does it here.
const remarkURLPlaceholder = "{% REMARK_URL %}"
// templatedFS fills the instance URL into the files carrying the placeholder. Without it the
// binary serves whatever the build baked in, which is a host no visitor can reach, and the widget
// falls back to it whenever a page omits remark_config.host.
type templatedFS struct {
fs fs.FS
remarkURL string
}
// Open substitutes in the file types the frontend templates, and hands everything else through
// untouched so images and stylesheets keep streaming from their original source
func (t templatedFS) Open(name string) (fs.File, error) {
f, err := t.fs.Open(name)
if err != nil || !templatedName(name) {
return f, err
}
info, err := f.Stat()
if err != nil || info.IsDir() {
return f, err
}
body, err := io.ReadAll(f)
if cerr := f.Close(); err == nil {
err = cerr
}
if err != nil {
return nil, err
}
body = bytes.ReplaceAll(body, []byte(remarkURLPlaceholder), []byte(t.remarkURL))
return &memFile{Reader: bytes.NewReader(body), info: sizedInfo{FileInfo: info, size: int64(len(body))}}, nil
}
// templatedName reports whether the frontend templates this file type. It mirrors the set the
// docker image rewrites, so both distributions substitute in the same files
func templatedName(name string) bool {
switch filepath.Ext(name) {
case ".html", ".js", ".mjs":
return true
}
return false
}
// memFile is a substituted file held in memory. The file server needs a seeker to answer range
// requests and to sniff a content type, which a substituted body no longer has on disk
type memFile struct {
*bytes.Reader
info fs.FileInfo
}
func (f *memFile) Stat() (fs.FileInfo, error) { return f.info, nil }
func (f *memFile) Close() error { return nil }
// sizedInfo reports the length after substitution. The file server writes Content-Length from it,
// so reporting the length on disk would truncate the response or leave the client waiting
type sizedInfo struct {
fs.FileInfo
size int64
}
func (i sizedInfo) Size() int64 { return i.size }
+123
View File
@@ -3,11 +3,15 @@ package api
import (
"io"
"io/fs"
"net/http"
"net/http/httptest"
"os"
"path/filepath"
"testing"
"testing/fstest"
"github.com/go-pkgz/routegroup"
"github.com/umputun/remark42/backend/app/webassets"
"github.com/stretchr/testify/assert"
@@ -218,3 +222,122 @@ func TestWebFiles_OpenUnreadableFrontendFile(t *testing.T) {
_ = f.Close()
}
}
func TestTemplatedFS_SubstitutesTheInstanceURL(t *testing.T) {
const placeholder = "host: '" + remarkURLPlaceholder + "'"
source := fstest.MapFS{
"iframe.html": {Data: []byte(placeholder)},
"embed.mjs": {Data: []byte(placeholder)},
"embed.js": {Data: []byte(placeholder)},
"remark.css": {Data: []byte(placeholder)},
"nothing.html": {Data: []byte("no marker here")},
}
tfs := templatedFS{fs: source, remarkURL: "https://remark.example.com"}
tbl := []struct {
name string
want string
}{
{"iframe.html", "host: 'https://remark.example.com'"},
{"embed.mjs", "host: 'https://remark.example.com'"},
{"embed.js", "host: 'https://remark.example.com'"},
// the docker image rewrites html, js and mjs and nothing else, and a stylesheet carrying
// the marker would mean the frontend started templating a file type this does not cover
{"remark.css", placeholder},
{"nothing.html", "no marker here"},
}
for _, tt := range tbl {
t.Run(tt.name, func(t *testing.T) {
f, err := tfs.Open(tt.name)
require.NoError(t, err)
defer func() { assert.NoError(t, f.Close()) }()
body, err := io.ReadAll(f)
require.NoError(t, err)
assert.Equal(t, tt.want, string(body))
info, err := f.Stat()
require.NoError(t, err)
assert.Equal(t, int64(len(tt.want)), info.Size(),
"the size has to be the substituted one, or the response is truncated or left hanging")
assert.Equal(t, tt.name, info.Name())
})
}
}
func TestTemplatedFS_PassesErrorsThrough(t *testing.T) {
tfs := templatedFS{fs: fstest.MapFS{}, remarkURL: "https://remark.example.com"}
_, err := tfs.Open("absent.html")
assert.ErrorIs(t, err, fs.ErrNotExist)
}
// TestRest_FileServerFillsInTheInstanceURL covers the reason templatedFS exists: the binary serves
// the frontend build embedded in itself, and nothing else fills the placeholder in for it.
func TestRest_FileServerFillsInTheInstanceURL(t *testing.T) {
frontend := fstest.MapFS{
"embed.mjs": {Data: []byte("host=\"" + remarkURLPlaceholder + "\"")},
"logo.svg": {Data: []byte(remarkURLPlaceholder)},
"plain.html": {Data: []byte("nothing to fill in")},
}
router := routegroup.New(http.NewServeMux())
addFileServer(router, frontend, filepath.Join(t.TempDir(), "absent"), "test-version", "https://remark.example.com")
ts := httptest.NewServer(router)
defer ts.Close()
t.Run("the bundle carries the configured url", func(t *testing.T) {
body, code := get(t, ts.URL+"/web/embed.mjs")
assert.Equal(t, http.StatusOK, code)
assert.Equal(t, `host="https://remark.example.com"`, body)
})
t.Run("the legacy js name carries it too", func(t *testing.T) {
body, code := get(t, ts.URL+"/web/embed.js")
assert.Equal(t, http.StatusOK, code)
assert.Equal(t, `host="https://remark.example.com"`, body)
})
t.Run("other types are served untouched", func(t *testing.T) {
body, code := get(t, ts.URL+"/web/logo.svg")
assert.Equal(t, http.StatusOK, code)
assert.Equal(t, remarkURLPlaceholder, body)
})
t.Run("a file without the marker is unchanged", func(t *testing.T) {
body, code := get(t, ts.URL+"/web/plain.html")
assert.Equal(t, http.StatusOK, code)
assert.Equal(t, "nothing to fill in", body)
})
}
// TestRest_FileServerEtagVariesWithTheInstanceURL covers the case this substitution exists for. An
// operator who notices the widget is addressed to the wrong host corrects REMARK_URL and restarts,
// and the binary and so the version is unchanged. If the validator ignores remarkURL the client
// revalidates, gets 304 and keeps the bundle pointing at the old host. Cache-Control is no-cache,
// so it revalidates every time and never ages out of that state.
func TestRest_FileServerEtagVariesWithTheInstanceURL(t *testing.T) {
frontend := fstest.MapFS{"embed.mjs": {Data: []byte("host=\"" + remarkURLPlaceholder + "\"")}}
etagFor := func(remarkURL string) string {
router := routegroup.New(http.NewServeMux())
addFileServer(router, frontend, filepath.Join(t.TempDir(), "absent"), "test-version", remarkURL)
ts := httptest.NewServer(router)
defer ts.Close()
resp, err := http.Get(ts.URL + "/web/embed.mjs")
require.NoError(t, err)
defer resp.Body.Close()
require.Equal(t, http.StatusOK, resp.StatusCode)
return resp.Header.Get("Etag")
}
first := etagFor("https://old.example.com")
second := etagFor("https://new.example.com")
require.NotEmpty(t, first, "the file server has to send a validator at all")
assert.NotEqual(t, first, second,
"same version and same path, different instance url: the validator has to change or the "+
"client keeps a bundle addressed to the old host")
}
+20 -4
View File
@@ -41,13 +41,15 @@ func TestService_SaveAndLoad(t *testing.T) {
assert.Equal(t, "test_id", store.LoadCalls()[0].ID)
}
// the resized dimensions are what resize promises; the encoded length is whatever the compressor
// in the toolchain happens to produce, and pinning it fails on a go release that changes it
func TestService_Resize(t *testing.T) {
img, err := readAndValidateImage(gopherPNG(), 1500)
assert.NoError(t, err)
assert.Equal(t, 1462, len(img))
assert.NotEmpty(t, img)
img = resize(img, 32, 32)
assert.Equal(t, 1135, len(img))
assertImageFits(t, img, 32, 32)
}
func TestService_ResizeJpeg(t *testing.T) {
@@ -57,10 +59,24 @@ func TestService_ResizeJpeg(t *testing.T) {
img, err := readAndValidateImage(fh, 32000)
assert.NoError(t, err)
assert.InDelta(t, 16756, len(img), 100)
assert.NotEmpty(t, img)
img = resize(img, 400, 300)
assert.InDelta(t, 10913, len(img), 100)
assertImageFits(t, img, 400, 300)
}
// assertImageFits decodes the image and checks it is inside the box resize was given, and that it
// touches one side of it, which is what fitting to a box rather than merely shrinking means
func assertImageFits(t *testing.T, data []byte, limitW, limitH int) {
t.Helper()
cfg, _, err := image.DecodeConfig(bytes.NewReader(data))
require.NoError(t, err, "the resized image does not decode")
assert.LessOrEqual(t, cfg.Width, limitW, "wider than the box it was resized into")
assert.LessOrEqual(t, cfg.Height, limitH, "taller than the box it was resized into")
assert.True(t, cfg.Width == limitW || cfg.Height == limitH,
"%dx%d touches neither side of the %dx%d box, so it was not fitted to it", cfg.Width, cfg.Height, limitW, limitH)
}
func TestService_SaveTooLarge(t *testing.T) {
+83
View File
@@ -0,0 +1,83 @@
//go:build e2e
package e2e
import (
"fmt"
neturl "net/url"
"strings"
"testing"
"github.com/mxschmitt/playwright-go"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// remarkURLPlaceholder is what the frontend build emits wherever the instance url belongs. The
// bundler cannot know that url, so every distribution fills the marker in: the docker image
// rewrites its web root at container start, the binary substitutes as it serves.
const remarkURLPlaceholder = "{% REMARK_URL %}"
// TestAssets_InstanceURLIsFilledIn covers the substitution, which nothing else here exercises.
// The demo pages set remark_config.host from location.origin themselves, so the compiled-in
// fallback is never read and the marker could survive into a release without a test noticing.
func TestAssets_InstanceURLIsFilledIn(t *testing.T) {
page := newPage(t)
pauseForAuthLimit()
_, err := page.Goto(baseURL + "/web/")
require.NoError(t, err)
// every file type the distributions rewrite, not only the one the widget happens to load
for _, name := range []string{"embed.mjs", "counter.mjs", "last-comments.mjs", "index.html"} {
status, body := pageFetch(t, page, "GET", baseURL+"/web/"+name, nil)
require.Equal(t, 200, status, name)
assert.NotContains(t, body, remarkURLPlaceholder, "%s still carries the marker", name)
}
// and the value that replaced it is this instance, not some other host. only the bundles
// carry it; index.html sets the host from the page's own origin
status, body := pageFetch(t, page, "GET", baseURL+"/web/embed.mjs", nil)
require.Equal(t, 200, status)
assert.Contains(t, body, baseURL, "embed.mjs should fall back to this instance")
}
// TestAssets_WidgetRunsOnTheCompiledInURL is the behavior the substitution exists for.
//
// The widget document carries no host: iframe.html builds remark_config out of its own query
// string, and the parent never passes one, so everything the bundle requests is addressed with
// the url the build was substituted with. A marker left in place leaves the widget asking for
// `{% REMARK_URL %}/api/v1/config` and rendering nothing.
//
// The demo pages cannot show this. Their loader builds the bundle's own script url from
// remark_config.host, so a page without one never gets as far as loading the widget.
func TestAssets_WidgetRunsOnTheCompiledInURL(t *testing.T) {
page := newPage(t)
var configURL string
page.OnRequest(func(r playwright.Request) {
if configURL == "" && strings.Contains(r.URL(), "/api/v1/config") {
configURL = r.URL()
}
})
pauseForAuthLimit()
_, err := page.Goto(fmt.Sprintf("%s/web/iframe.html?site_id=remark&url=%s",
baseURL, neturl.QueryEscape(threadURL(t))))
require.NoError(t, err)
// the document really had no host of its own, or the fallback was never reached
host, err := page.Evaluate(`() => window.remark_config && window.remark_config.host`)
require.NoError(t, err)
require.Nil(t, host, "the widget document should carry no host")
// it rendered, so the url it was addressing resolved
waitVisible(t, page.Locator(commentFormSel).First())
eventually(t, waitTimeout, "the widget never asked for its config", func() bool {
return configURL != ""
})
assert.True(t, strings.HasPrefix(configURL, baseURL),
"the widget addressed %q rather than this instance", configURL)
assert.NotContains(t, configURL, remarkURLPlaceholder)
}
+6 -6
View File
@@ -7,7 +7,7 @@ PUBLIC_DIR="$APP_DIR/public"
EMBED_DIR="$ROOT/backend/app/cmd/web"
PREPARED_MARKER="$EMBED_DIR/.release-assets-prepared"
for cmd in git pnpm perl; do
for cmd in git pnpm; do
if ! command -v "$cmd" >/dev/null 2>&1; then
echo "error: $cmd is required to build release assets" >&2
exit 1
@@ -48,11 +48,11 @@ mkdir -p "$EMBED_DIR"
cp -R "$PUBLIC_DIR"/. "$EMBED_DIR"/
find "$EMBED_DIR" -type f \( -name '*.html' -o -name '*.js' -o -name '*.mjs' \) \
-exec perl -pi -e 's|\{\% REMARK_URL \%\}|http://127.0.0.1:8080|g' {} +
if grep -R "{% REMARK_URL %}" "$EMBED_DIR" >/dev/null; then
echo "error: unreplaced REMARK_URL placeholder in $EMBED_DIR" >&2
# the placeholder stays in: the server fills it with its own REMARK_URL when it serves these files,
# which is the only chance the binary gets. substituting a value here would bake one instance's
# address into every copy of the release
if ! grep -Rq "{% REMARK_URL %}" "$EMBED_DIR"; then
echo "error: no REMARK_URL placeholder in $EMBED_DIR, the build stopped templating it" >&2
exit 1
fi