Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions internal/app/media_usage.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,12 @@ func mediaUsageInputs(usages []MediaUsageProjection) []domain.EntryMediaUsage {
return inputs
}

func validatePostMediaProjections(usages []MediaUsageProjection) error {
if err := domain.ValidateEntryMediaUsages(mediaUsageInputs(usages)); err != nil {
func validatePostMediaProjections(kind domain.EntryKind, usages []MediaUsageProjection) error {
validate := domain.ValidateEntryMediaUsages
if kind == domain.KindArticle {
validate = domain.ValidateArticleMediaUsages
}
if err := validate(mediaUsageInputs(usages)); err != nil {
return err
}
return validateMediaProjectionDimensions(usages)
Expand Down
2 changes: 1 addition & 1 deletion internal/app/posts.go
Original file line number Diff line number Diff line change
Expand Up @@ -1516,7 +1516,7 @@ func loadQuotedPost(ctx context.Context, store QuoteReadStore, viewer Viewer, id
}

func renderPost(entry domain.Entry, view articleView, media []MediaUsageProjection, location *domain.Location, cards []LinkCardProjection, commentCount int, hasQuote ...bool) (Post, error) {
if err := validatePostMediaProjections(media); err != nil {
if err := validatePostMediaProjections(entry.Kind, media); err != nil {
return Post{}, storedEntryError(entry.ID)
}
// A list read carries only the card, so there is no body to check or
Expand Down
2 changes: 1 addition & 1 deletion internal/domain/article_doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const (
// MaxArticleDepth lets the renderer recurse without guarding the stack.
MaxArticleDepth = 10
// MaxArticleInlineImages matches the position ceiling on entry_media.
MaxArticleInlineImages = 50
MaxArticleInlineImages = 512
// A formula's source stays bounded independently of the whole document.
MaxArticleMathCodePoints = 4096
// MaxArticleTableColumns and MaxArticleTableRows bound one table.
Expand Down
5 changes: 3 additions & 2 deletions internal/domain/entry_media.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package domain

import (
"fmt"
"strings"
"unicode/utf8"
)
Expand All @@ -17,7 +18,7 @@ const (
// MaxEntryMediaBytes is the aggregate logical usage budget for one post.
MaxEntryMediaBytes int64 = 50 << 20
// MaxArticleMediaBytes is the same budget for an article, which places up
// to fifty pictures where a moment places nine. Holding an article to the
// to hundreds of pictures where a moment places nine. Holding an article to the
// moment's total would make the picture limit unreachable in practice.
MaxArticleMediaBytes int64 = 250 << 20
// MaxCommentMediaBytes is the aggregate logical usage budget for one comment.
Expand All @@ -42,7 +43,7 @@ func ValidateEntryMediaUsages(usages []EntryMediaUsage) error {
// pictures an article body places. The projection is derived from the body, so
// a failure here means the body and its projection disagree.
func ValidateArticleMediaUsages(usages []EntryMediaUsage) error {
return validateMediaUsages(usages, MaxArticleInlineImages, "an article may place at most 50 pictures")
return validateMediaUsages(usages, MaxArticleInlineImages, fmt.Sprintf("an article may place at most %d pictures", MaxArticleInlineImages))
}

// ValidateCommentMediaUsages applies the same ordering and alt contract with
Expand Down
38 changes: 38 additions & 0 deletions internal/storage/sqlite/article_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package sqlite

import (
"context"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -449,3 +450,40 @@ func TestEveryArticleWriteKeepsTheReclamationClockHonest(t *testing.T) {
}
assertMediaClocksAgree(t, db, "discarding the last reference")
}

func TestLargeArticleSurvivesDraftPublishAndEdit(t *testing.T) {
t.Parallel()
ctx := testAccountContext()
db, service := articleServiceFixture(t, time.UnixMilli(1000).UTC())
media := articleMediaID(t, db, "c")
images := make([]string, domain.MaxArticleInlineImages)
for i := range images {
images[i] = articleImage(media, "diagram")
}
body := []byte(`{"schema_version":1,"type":"doc","content":[` + strings.Join(images, ",") + `]}`)
draft, err := service.SaveArticleDraft(ctx, app.SaveArticleDraftInput{Title: "Illustrated article", BodyJSON: body})
if err != nil {
t.Fatal(err)
}
read, err := service.ArticleDraft(ctx, draft.ID)
if err != nil {
t.Fatal(err)
}
if len(read.Draft.InlineMedia()) != domain.MaxArticleInlineImages {
t.Fatal("draft lost image placements")
}
post, err := service.PublishArticleDraft(ctx, app.PublishArticleDraftInput{DraftID: draft.ID})
if err != nil {
t.Fatal(err)
}
if len(post.Media) != domain.MaxArticleInlineImages {
t.Fatalf("published placements = %d", len(post.Media))
}
edited, err := service.EditArticle(ctx, post.Entry.ID)
if err != nil {
t.Fatal(err)
}
if edited.Draft.Body.Content[0].MediaID.String() != media || len(edited.Draft.InlineMedia()) != domain.MaxArticleInlineImages {
t.Fatal("working copy lost original placements")
}
}
2 changes: 1 addition & 1 deletion internal/storage/sqlite/articles_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ func TestArticleStorageHoldsThePictureLimitsOfEachKind(t *testing.T) {
t.Fatal(err)
}
if err := writeArticle(t, db, full, false); err != nil {
t.Fatalf("fifty pictures must be accepted: %v", err)
t.Fatalf("the article picture limit must be accepted: %v", err)
}

// A moment is still nine, now that the column no longer says so.
Expand Down
4 changes: 2 additions & 2 deletions internal/storage/sqlite/database_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -416,10 +416,10 @@ VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, NULL)`, mediaTwo, strings.Repeat("b", 64), "m
assertWriteRejected(t, db, `INSERT INTO entry_media VALUES (?, ?, 3, ?)`, entryOne, mediaOne, "\u3000\u00a0")
assertWriteRejected(t, db, `INSERT INTO entry_media VALUES (?, ?, 3, ?)`, entryOne, mediaOne, strings.Repeat("界", 1001))
assertWriteRejected(t, db, `INSERT INTO entry_media VALUES (?, ?, 3, ?)`, entryOne, mediaOne, []byte("not text"))
// An article body places its own pictures, so the column now stops at 50
// An article body places its own pictures, so the column now stops at the article limit
// and the nine a moment may carry is a rule the domain holds
// (domain.MaxEntryMediaUsages), not this check.
assertWriteRejected(t, db, `INSERT INTO entry_media VALUES (?, ?, 50, NULL)`, entryOne, mediaOne)
assertWriteRejected(t, db, `INSERT INTO entry_media VALUES (?, ?, 512, NULL)`, entryOne, mediaOne)
assertWriteRejected(t, db, `INSERT INTO entry_media VALUES (?, ?, 3, NULL)`, entryOne, entryThree)
assertWriteRejected(t, db, `
INSERT INTO media VALUES (?, ?, ?, 'bad.bmp', 'image/bmp', 1, 1, 1, 1, NULL)`, entryThree, strings.Repeat("c", 64), "media/original/cc/c.bmp")
Expand Down
55 changes: 55 additions & 0 deletions internal/storage/sqlite/migration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -284,3 +284,58 @@ INSERT INTO browser_account_members(set_id,user_id,session_id,last_used_at_ms) V
t.Fatalf("reopening invalidated a post-upgrade collection: %v, %v", revoked, err)
}
}

func TestArticleMediaCapacityUpgradePreservesPlacements(t *testing.T) {
t.Parallel()
pool := openRawMigrationPool(t)
migrations, err := embeddedMigrations()
if err != nil {
t.Fatal(err)
}
ctx := testAccountContext()
if err := applyMigrations(ctx, pool, migrations[:14], time.Unix(1, 0)); err != nil {
t.Fatal(err)
}
_, err = pool.ExecContext(ctx, `
INSERT INTO users(id,username,display_name,role,status,created_at_ms,updated_at_ms) VALUES ('11111111111111111111111111111111','writer','Writer','admin','active',1,1);
INSERT INTO entries(id,kind,visibility,published_at_ms,created_at_ms,updated_at_ms,author_user_id) VALUES ('01ARZ3NDEKTSV4RRFFQ69G5FAV','article','everyone',1,1,1,'11111111111111111111111111111111');
INSERT INTO media(id,sha256,storage_path,original_name,mime_type,byte_size,width,height,created_at_ms) VALUES ('01ARZ3NDEKTSV4RRFFQ69G5FAY',?,'media/original/aa/' || ? || '.png','diagram.png','image/png',100,10,10,1);
INSERT INTO article_drafts(id,title,body_json,body_schema_version,created_at_ms,updated_at_ms,author_user_id) VALUES ('01ARZ3NDEKTSV4RRFFQ69G5FG1','Draft','{"schema_version":1,"type":"doc","content":[]}',1,1,1,'11111111111111111111111111111111');
INSERT INTO entry_media(entry_id,media_id,position,alt_text) VALUES ('01ARZ3NDEKTSV4RRFFQ69G5FAV','01ARZ3NDEKTSV4RRFFQ69G5FAY',49,'published alt');
INSERT INTO article_draft_media(draft_id,media_id,position,alt_text) VALUES ('01ARZ3NDEKTSV4RRFFQ69G5FG1','01ARZ3NDEKTSV4RRFFQ69G5FAY',49,'draft alt');`, strings.Repeat("a", 64), strings.Repeat("a", 64))
if err != nil {
t.Fatal(err)
}
if err := applyMigrations(ctx, pool, migrations, time.Unix(2, 0)); err != nil {
t.Fatal(err)
}
for _, table := range []string{"entry_media", "article_draft_media"} {
var alt string
if err := pool.QueryRowContext(ctx, "SELECT alt_text FROM "+table+" WHERE position = 49").Scan(&alt); err != nil {
t.Fatal(err)
}
want := "published alt"
if table == "article_draft_media" {
want = "draft alt"
}
if alt != want {
t.Fatalf("%s alt = %q", table, alt)
}
if _, err := pool.ExecContext(ctx, "INSERT INTO "+table+" SELECT "+map[string]string{"entry_media": "entry_id", "article_draft_media": "draft_id"}[table]+",media_id,511,alt_text FROM "+table+" WHERE position=49"); err != nil {
t.Fatalf("expanded %s: %v", table, err)
}
if _, err := pool.ExecContext(ctx, "UPDATE "+table+" SET position=512 WHERE position=511"); err == nil {
t.Fatalf("%s accepted an out-of-range position", table)
}
}
if _, err := pool.ExecContext(ctx, "DELETE FROM media WHERE id = ?", mediaOne); err == nil {
t.Fatal("referenced originals must remain protected")
}
if _, err := pool.ExecContext(ctx, "DELETE FROM article_drafts WHERE id = ?", draftOneID); err != nil {
t.Fatal(err)
}
var count int
if err := pool.QueryRowContext(ctx, "SELECT count(*) FROM article_draft_media").Scan(&count); err != nil || count != 0 {
t.Fatalf("draft cascade count=%d err=%v", count, err)
}
}
39 changes: 39 additions & 0 deletions internal/storage/sqlite/migrations/0015_article_media_capacity.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
-- Expand article placements while preserving ordered references and alt text.
-- Moment and comment limits remain enforced by their domain validators.

CREATE TABLE article_draft_media_expanded (
draft_id TEXT NOT NULL REFERENCES article_drafts(id) ON DELETE CASCADE,
media_id TEXT NOT NULL REFERENCES media(id) ON DELETE RESTRICT,
position INTEGER NOT NULL CHECK (position BETWEEN 0 AND 511),
alt_text TEXT CHECK (alt_text IS NULL OR length(alt_text) <= 1000),
PRIMARY KEY (draft_id, position)
) STRICT;
INSERT INTO article_draft_media_expanded (draft_id, media_id, position, alt_text)
SELECT draft_id, media_id, position, alt_text FROM article_draft_media;
DROP TABLE article_draft_media;
ALTER TABLE article_draft_media_expanded RENAME TO article_draft_media;
CREATE INDEX article_draft_media_media_idx ON article_draft_media (media_id);

CREATE TABLE entry_media_expanded (
entry_id TEXT NOT NULL REFERENCES entries(id) ON DELETE CASCADE,
media_id TEXT NOT NULL REFERENCES media(id) ON DELETE RESTRICT,
position INTEGER NOT NULL CHECK (position BETWEEN 0 AND 511),
alt_text TEXT CHECK (
alt_text IS NULL OR alt_text = '' OR (
length(alt_text) <= 1000
AND trim(alt_text, char(9) || char(10) || char(11) || char(12)
|| char(13) || char(32) || char(133) || char(160)
|| char(5760) || char(8192) || char(8193) || char(8194)
|| char(8195) || char(8196) || char(8197) || char(8198)
|| char(8199) || char(8200) || char(8201) || char(8202)
|| char(8232) || char(8233) || char(8239) || char(8287)
|| char(12288)) <> ''
)
),
PRIMARY KEY (entry_id, position)
) STRICT;
INSERT INTO entry_media_expanded (entry_id, media_id, position, alt_text)
SELECT entry_id, media_id, position, alt_text FROM entry_media;
DROP TABLE entry_media;
ALTER TABLE entry_media_expanded RENAME TO entry_media;
CREATE INDEX entry_media_media_idx ON entry_media (media_id);
12 changes: 9 additions & 3 deletions internal/storage/sqlite/posts.go
Original file line number Diff line number Diff line change
Expand Up @@ -288,8 +288,9 @@ func (store postStore) CommentCounts(ctx context.Context, ids []domain.ID) (map[

func (store postStore) EntryMediaUsages(ctx context.Context, entryID domain.ID) ([]app.MediaUsageProjection, error) {
rows, err := store.queries.QueryContext(ctx, `
SELECT usage.media_id, usage.position, usage.alt_text, media.mime_type, media.width, media.height
SELECT usage.media_id, usage.position, usage.alt_text, media.mime_type, media.width, media.height, entry.kind
FROM entry_media AS usage
JOIN entries AS entry ON entry.id = usage.entry_id
LEFT JOIN media ON media.id = usage.media_id
WHERE usage.entry_id = ?
ORDER BY usage.position`, entryID.String())
Expand All @@ -298,13 +299,14 @@ ORDER BY usage.position`, entryID.String())
}
defer func() { _ = rows.Close() }()
usages := make([]app.MediaUsageProjection, 0, domain.MaxEntryMediaUsages)
var kind domain.EntryKind
for rows.Next() {
var rawID string
var position int
var alt sql.NullString
var mimeType sql.NullString
var width, height sql.NullInt64
if err := rows.Scan(&rawID, &position, &alt, &mimeType, &width, &height); err != nil {
if err := rows.Scan(&rawID, &position, &alt, &mimeType, &width, &height, &kind); err != nil {
return nil, fmt.Errorf("scan entry media usage: %w", err)
}
id, err := domain.ParseID(rawID)
Expand All @@ -324,7 +326,11 @@ ORDER BY usage.position`, entryID.String())
if err := rows.Err(); err != nil {
return nil, fmt.Errorf("iterate entry media usages: %w", err)
}
if err := domain.ValidateEntryMediaUsages(mediaProjectionInputs(usages)); err != nil {
validate := domain.ValidateEntryMediaUsages
if kind == domain.KindArticle {
validate = domain.ValidateArticleMediaUsages
}
if err := validate(mediaProjectionInputs(usages)); err != nil {
return nil, domain.NewError(domain.CodeDataIntegrityFailed, "stored entry media usages violate the ordered contract", map[string]any{"post_id": entryID.String()})
}
return usages, nil
Expand Down
20 changes: 20 additions & 0 deletions web/src/api/public.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1381,6 +1381,26 @@ describe("post kind and the body it is allowed to carry", () => {
).toThrow(PublicApiError);
});

it("reads long article geometry and rejects excess placements", () => {
const media = Array.from({ length: 512 }, (_, index) => ({
media_id: `01K2ABCDEF0123456789AB${index.toString(16).toUpperCase().padStart(4, "0")}`,
mime_type: "image/png",
width: 160,
height: 80,
}));
const full = {
...article,
article: { ...article.article, body_media: media },
};
expect(parsePost(full).article?.bodyMedia).toHaveLength(512);
expect(() =>
parsePost({
...full,
article: { ...full.article, body_media: [...media, media[0]] },
}),
).toThrow(PublicApiError);
});

it("parses a moment, which carries its Markdown source", () => {
expect(parsePost(post)).toMatchObject({
kind: "moment",
Expand Down
6 changes: 3 additions & 3 deletions web/src/api/public.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1021,7 +1021,7 @@ function parseCommentReplyTarget(
* article the tenth time it showed a picture.
*/
const MAX_MOMENT_MEDIA = 9;
const MAX_ARTICLE_MEDIA = 50;
const MAX_ARTICLE_MEDIA = 512;

function mediaCeiling(kind: PostKind): number {
return kind === "article" ? MAX_ARTICLE_MEDIA : MAX_MOMENT_MEDIA;
Expand Down Expand Up @@ -1676,11 +1676,11 @@ function parseArticle(
if (Object.hasOwn(value, "body_media")) {
if (
!Array.isArray(value.body_media) ||
value.body_media.length > 50 ||
value.body_media.length > MAX_ARTICLE_MEDIA ||
value.body === null
) {
throw invalidResponse(
"article body media requires a document and at most 50 pictures",
`article body media requires a document and at most ${MAX_ARTICLE_MEDIA} pictures`,
);
}
const seen = new Set<string>();
Expand Down
39 changes: 39 additions & 0 deletions web/src/components/content/RichContent.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -207,3 +207,42 @@ describe("shared Tiptap document rendering", () => {
expect(window.location.pathname).toBe("/linlin");
});
});

it.each(["/api/v1/media/01K2ABCDEF0123456789ABCDE7", "/archive"])(
"routes %s through its owning transport",
(href) => {
const original = window.location.href;
window.history.replaceState({}, "", "/reader-check");
try {
render(
<NavigationProvider>
<RichContent
document={{
schema_version: 4,
type: "doc",
content: [
{
type: "paragraph",
content: [
{
type: "text",
text: "Destination",
marks: [{ type: "link", attrs: { href } }],
},
],
},
],
}}
/>
</NavigationProvider>,
);
const anchor = screen.getByRole("link", { name: "Destination" });
fireEvent.click(anchor);
expect(window.location.pathname).toBe(
href.startsWith("/api/") ? "/reader-check" : href,
);
} finally {
window.history.replaceState({}, "", original);
}
},
);
1 change: 1 addition & 0 deletions web/src/components/content/RichContent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ export function RichContent({
if (
!(anchor instanceof HTMLAnchorElement) ||
anchor.hasAttribute("download") ||
anchor.pathname.startsWith("/api/") ||
(anchor.target && anchor.target !== "_self") ||
anchor.origin !== window.location.origin
)
Expand Down
Loading
Loading