diff --git a/backend/app/migrator/backup_test.go b/backend/app/migrator/backup_test.go index c9c86879..1e4d4b10 100644 --- a/backend/app/migrator/backup_test.go +++ b/backend/app/migrator/backup_test.go @@ -1,6 +1,7 @@ package migrator import ( + "compress/gzip" "context" "fmt" "io" @@ -50,9 +51,7 @@ func TestBackup_MakeBackup(t *testing.T) { expFile := fmt.Sprintf("/tmp/remark-backups.test/backup-site1-%s.gz", time.Now().Format("20060102")) assert.Equal(t, expFile, fname) - fi, err := os.Lstat(expFile) - assert.NoError(t, err) - assert.Equal(t, int64(52), fi.Size()) + assert.Equal(t, exportedPayload, gzContent(t, expFile)) } func TestBackup_Do(t *testing.T) { @@ -71,15 +70,31 @@ func TestBackup_Do(t *testing.T) { bk.Do(ctx) expFile := fmt.Sprintf("/tmp/remark-backups.test/backup-site1-%s.gz", time.Now().Format("20060102")) - fi, err := os.Lstat(expFile) - assert.NoError(t, err) - assert.Equal(t, int64(52), fi.Size()) + assert.Equal(t, exportedPayload, gzContent(t, expFile)) }) } +const exportedPayload = "some export blah blah 1234567890" + +// the compressed size is not assertable: it moves with the compress/flate version +func gzContent(t *testing.T, name string) string { + t.Helper() + fh, err := os.Open(name) //nolint:gosec // path is built by the test + require.NoError(t, err) + defer func() { assert.NoError(t, fh.Close()) }() + + gz, err := gzip.NewReader(fh) + require.NoError(t, err) + defer func() { assert.NoError(t, gz.Close()) }() + + b, err := io.ReadAll(gz) + require.NoError(t, err) + return string(b) +} + type mockExporter struct{} func (mock *mockExporter) Export(w io.Writer, _ string) (int, error) { - _, err := w.Write([]byte("some export blah blah 1234567890")) + _, err := w.Write([]byte(exportedPayload)) return 1000, err } diff --git a/backend/app/rest/api/webfiles.go b/backend/app/rest/api/webfiles.go index d82d67f8..e5c3b120 100644 --- a/backend/app/rest/api/webfiles.go +++ b/backend/app/rest/api/webfiles.go @@ -4,6 +4,7 @@ import ( "errors" "io/fs" "path/filepath" + "strings" ) // webFiles serves /web from two sources: a name present in the frontend build is served from there, @@ -13,15 +14,38 @@ type webFiles struct { embedded fs.FS } -// Open looks the name up in the frontend build first. Only a missing file falls through to the -// embedded assets; every other error is returned so an unreadable file keeps reporting as one -// rather than being replaced by the embedded copy or reported as missing. +// Open resolves the name against both sources, and answers a missing .js with the .mjs sibling. +// The build stopped emitting .js while integrations still request it; the bundles carry no module +// syntax, so the same bytes serve both names. func (w webFiles) Open(name string) (fs.File, error) { // fs.ValidPath alone is not enough: it accepts names the operating system rejects, NUL among // them, and os.DirFS turns those into fs.ErrInvalid, which renders as 500 rather than 404 if _, err := filepath.Localize(name); err != nil || !fs.ValidPath(name) { return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrNotExist} } + + f, err := w.open(name) + if err == nil { + return f, nil + } + if !errors.Is(err, fs.ErrNotExist) || !strings.HasSuffix(name, ".js") { + return nil, err + } + + alias, aliasErr := w.open(strings.TrimSuffix(name, ".js") + ".mjs") + if aliasErr == nil { + return alias, nil + } + if !errors.Is(aliasErr, fs.ErrNotExist) { + return nil, aliasErr + } + return nil, err +} + +// open looks the name up in the frontend build first. Only a missing file falls through to the +// embedded assets; every other error is returned so an unreadable file keeps reporting as one +// rather than being replaced by the embedded copy or reported as missing. +func (w webFiles) open(name string) (fs.File, error) { f, err := w.frontend.Open(name) if err == nil { return f, nil diff --git a/backend/app/rest/api/webfiles_test.go b/backend/app/rest/api/webfiles_test.go index f82647e7..c4e994e5 100644 --- a/backend/app/rest/api/webfiles_test.go +++ b/backend/app/rest/api/webfiles_test.go @@ -54,6 +54,79 @@ func TestWebFiles_Open(t *testing.T) { } } +func TestWebFiles_OpenJSAlias(t *testing.T) { + frontend := fstest.MapFS{ + "embed.mjs": {Data: []byte("module embed")}, + "counter.js": {Data: []byte("operator's own counter")}, + "counter.mjs": {Data: []byte("module counter")}, + "widget.mjs": {Data: []byte("module widget")}, + } + embedded := fstest.MapFS{ + "legacy.mjs": {Data: []byte("module legacy")}, + "widget.js": {Data: []byte("embedded widget")}, + } + w := webFiles{frontend: frontend, embedded: embedded} + + tbl := []struct { + name string + lookup string + want string + wantErr error + }{ + {name: "missing js served from the mjs sibling", lookup: "embed.js", want: "module embed"}, + {name: "alias reaches the embedded assets too", lookup: "legacy.js", want: "module legacy"}, + {name: "a real js file wins over its sibling", lookup: "counter.js", want: "operator's own counter"}, + {name: "an embedded js wins over a frontend sibling", lookup: "widget.js", want: "embedded widget"}, + {name: "mjs is still served directly", lookup: "embed.mjs", want: "module embed"}, + {name: "neither name present", lookup: "absent.js", wantErr: fs.ErrNotExist}, + {name: "only js aliases, not other extensions", lookup: "embed.html", wantErr: fs.ErrNotExist}, + } + + for _, tt := range tbl { + t.Run(tt.name, func(t *testing.T) { + f, err := w.Open(tt.lookup) + if tt.wantErr != nil { + require.Error(t, err) + assert.ErrorIs(t, err, tt.wantErr) + return + } + require.NoError(t, err) + defer f.Close() + b, err := io.ReadAll(f) + require.NoError(t, err) + assert.Equal(t, tt.want, string(b)) + }) + } +} + +func TestWebFiles_OpenJSAliasNamesTheRequestedFile(t *testing.T) { + w := webFiles{frontend: fstest.MapFS{}, embedded: fstest.MapFS{}} + + _, err := w.Open("absent.js") + require.Error(t, err) + assert.ErrorIs(t, err, fs.ErrNotExist) + assert.Contains(t, err.Error(), "absent.js") + assert.NotContains(t, err.Error(), "absent.mjs") +} + +func TestWebFiles_OpenJSAliasUnreadableSibling(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("root ignores file permissions") + } + + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "embed.mjs"), []byte("module embed"), 0o000)) + w := webFiles{frontend: os.DirFS(dir), embedded: fstest.MapFS{}} + + f, err := w.Open("embed.js") + require.Error(t, err) + assert.ErrorIs(t, err, fs.ErrPermission) + assert.NotErrorIs(t, err, fs.ErrNotExist, "an unreadable sibling must not render as 404") + if err == nil { + _ = f.Close() + } +} + // TestEmptyFS_ServesNothing pins the stand-in used when the frontend source cannot be opened: // every name must report as missing rather than panicking, since it backs a nil-free fallback. func TestEmptyFS_ServesNothing(t *testing.T) { diff --git a/e2e/widgets_test.go b/e2e/widgets_test.go index ca58ccff..4d7a150d 100644 --- a/e2e/widgets_test.go +++ b/e2e/widgets_test.go @@ -104,6 +104,44 @@ func TestWidgets_CounterFillsInTheCommentCount(t *testing.T) { }) } +func TestWidgets_LegacyJSURLLoadsAsAClassicScript(t *testing.T) { + thread := threadURL(t) + + poster := newPage(t) + frame := openThread(t, poster) + signInAnon(t, frame, "aliastester") + postComment(t, frame, "alias "+runID) + + want := strconv.Itoa(commentCount(t, poster, thread)) + + page := newPage(t) + pauseForAuthLimit() + _, err := page.Goto(baseURL + "/web/privacy.html") + require.NoError(t, err) + + _, err = page.Evaluate(`([host, url]) => { + window.remark_config = { host, site_id: 'remark' }; + const node = document.createElement('span'); + node.className = 'remark42__counter'; + node.dataset.url = url; + document.body.appendChild(node); + }`, []any{baseURL, thread}) + require.NoError(t, err) + + counter := page.Locator(".remark42__counter") + blank, err := pollText(counter) + require.NoError(t, err) + require.Empty(t, blank, "the counter must start blank or the assertion below is vacuous") + + _, err = page.AddScriptTag(playwright.PageAddScriptTagOptions{URL: playwright.String(baseURL + "/web/counter.js")}) + require.NoError(t, err) + + eventually(t, waitTimeout, "the legacy counter.js url never filled the counter", func() bool { + txt, ierr := pollText(counter) + return ierr == nil && txt == want + }) +} + // commentCount asks the API what the counter should be showing func commentCount(t *testing.T, page playwright.Page, url string) int { t.Helper() diff --git a/frontend/apps/remark42/app/embed.test.ts b/frontend/apps/remark42/app/embed.test.ts new file mode 100644 index 00000000..d7375d11 --- /dev/null +++ b/frontend/apps/remark42/app/embed.test.ts @@ -0,0 +1,53 @@ +process.env.REMARK_NODE = 'remark42'; + +const MARKER = 'data-remark42-iframe'; +const MARKED = `iframe[${MARKER}]`; + +async function mount(placeholder = '') { + document.body.innerHTML = `