fix(settings): validate memo content length limit
This commit is contained in:
parent
66b9cb49c1
commit
45c3a79b12
6 changed files with 206 additions and 6 deletions
|
|
@ -210,6 +210,7 @@ func (s *APIV1Service) UpdateInstanceSetting(ctx context.Context, request *v1pb.
|
|||
return nil, status.Errorf(codes.FailedPrecondition, "instance setting %q is configured by the deployment", settingKeyString)
|
||||
}
|
||||
|
||||
applyInstanceSettingDefaults(request.Setting)
|
||||
// TODO: Apply update_mask if specified
|
||||
_ = request.UpdateMask
|
||||
|
||||
|
|
|
|||
|
|
@ -12,14 +12,25 @@ import (
|
|||
|
||||
v1pb "github.com/usememos/memos/proto/gen/api/v1"
|
||||
storepb "github.com/usememos/memos/proto/gen/store"
|
||||
"github.com/usememos/memos/store"
|
||||
)
|
||||
|
||||
func applyInstanceSettingDefaults(setting *v1pb.InstanceSetting) {
|
||||
memoRelatedSetting := setting.GetMemoRelatedSetting()
|
||||
if memoRelatedSetting == nil || memoRelatedSetting.ContentLengthLimit != 0 {
|
||||
return
|
||||
}
|
||||
memoRelatedSetting.ContentLengthLimit = store.DefaultContentLengthLimit
|
||||
}
|
||||
|
||||
func validateInstanceSetting(setting *v1pb.InstanceSetting) error {
|
||||
key, err := ExtractInstanceSettingKeyFromName(setting.Name)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
switch key {
|
||||
case storepb.InstanceSettingKey_MEMO_RELATED.String():
|
||||
return validateInstanceMemoRelatedSetting(setting.GetMemoRelatedSetting())
|
||||
case storepb.InstanceSettingKey_TAGS.String():
|
||||
return validateInstanceTagsSetting(setting.GetTagsSetting())
|
||||
case storepb.InstanceSettingKey_ACCESS.String():
|
||||
|
|
@ -29,6 +40,16 @@ func validateInstanceSetting(setting *v1pb.InstanceSetting) error {
|
|||
}
|
||||
}
|
||||
|
||||
func validateInstanceMemoRelatedSetting(setting *v1pb.InstanceSetting_MemoRelatedSetting) error {
|
||||
if setting == nil {
|
||||
return errors.New("memo related setting is required")
|
||||
}
|
||||
if setting.ContentLengthLimit < store.DefaultContentLengthLimit {
|
||||
return errors.Errorf("content_length_limit must be at least %d bytes", store.DefaultContentLengthLimit)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func validateInstanceAccessSetting(setting *v1pb.InstanceSetting_AccessSetting) error {
|
||||
if setting == nil {
|
||||
return errors.New("access setting is required")
|
||||
|
|
|
|||
|
|
@ -7,6 +7,8 @@ import (
|
|||
|
||||
"github.com/stretchr/testify/require"
|
||||
colorpb "google.golang.org/genproto/googleapis/type/color"
|
||||
"google.golang.org/grpc/codes"
|
||||
"google.golang.org/grpc/status"
|
||||
"google.golang.org/protobuf/types/known/fieldmaskpb"
|
||||
|
||||
v1pb "github.com/usememos/memos/proto/gen/api/v1"
|
||||
|
|
@ -466,6 +468,45 @@ func TestTestInstanceEmailSettingRequiresPasswordWhenSMTPIdentityChanges(t *test
|
|||
func TestUpdateInstanceSetting(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
|
||||
t.Run("UpdateInstanceSetting - memo related content length limit", func(t *testing.T) {
|
||||
ts := NewTestService(t)
|
||||
defer ts.Cleanup()
|
||||
|
||||
admin, err := ts.CreateHostUser(ctx, "memo-setting-admin")
|
||||
require.NoError(t, err)
|
||||
adminCtx := ts.CreateUserContext(ctx, admin.ID)
|
||||
settingForLimit := func(limit int32) *v1pb.InstanceSetting {
|
||||
return &v1pb.InstanceSetting{
|
||||
Name: "instance/settings/MEMO_RELATED",
|
||||
Value: &v1pb.InstanceSetting_MemoRelatedSetting_{
|
||||
MemoRelatedSetting: &v1pb.InstanceSetting_MemoRelatedSetting{
|
||||
ContentLengthLimit: limit,
|
||||
},
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
updated, err := ts.Service.UpdateInstanceSetting(adminCtx, &v1pb.UpdateInstanceSettingRequest{Setting: settingForLimit(0)})
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, int32(8192), updated.GetMemoRelatedSetting().GetContentLengthLimit())
|
||||
got, err := ts.Service.GetInstanceSetting(ctx, &v1pb.GetInstanceSettingRequest{Name: "instance/settings/MEMO_RELATED"})
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, int32(8192), got.GetMemoRelatedSetting().GetContentLengthLimit())
|
||||
|
||||
_, err = ts.Service.UpdateInstanceSetting(adminCtx, &v1pb.UpdateInstanceSettingRequest{Setting: settingForLimit(8191)})
|
||||
require.Equal(t, codes.InvalidArgument, status.Code(err))
|
||||
|
||||
for _, limit := range []int32{8192, 16384} {
|
||||
updated, err := ts.Service.UpdateInstanceSetting(adminCtx, &v1pb.UpdateInstanceSettingRequest{Setting: settingForLimit(limit)})
|
||||
require.NoError(t, err, "limit %d", limit)
|
||||
require.Equal(t, limit, updated.GetMemoRelatedSetting().GetContentLengthLimit())
|
||||
|
||||
got, err := ts.Service.GetInstanceSetting(ctx, &v1pb.GetInstanceSettingRequest{Name: "instance/settings/MEMO_RELATED"})
|
||||
require.NoError(t, err, "limit %d", limit)
|
||||
require.Equal(t, limit, got.GetMemoRelatedSetting().GetContentLengthLimit())
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("UpdateInstanceSetting - access setting", func(t *testing.T) {
|
||||
ts := NewTestService(t)
|
||||
defer ts.Cleanup()
|
||||
|
|
|
|||
|
|
@ -20,15 +20,35 @@ import { SettingList, SettingListItem, SettingPanel } from "./SettingList";
|
|||
import SettingSection from "./SettingSection";
|
||||
import useInstanceSettingUpdater, { buildInstanceSettingName } from "./useInstanceSettingUpdater";
|
||||
|
||||
const MIN_CONTENT_LENGTH_LIMIT = 8 * 1024;
|
||||
const MAX_CONTENT_LENGTH_LIMIT = 2_147_483_647;
|
||||
|
||||
const parseContentLengthLimit = (value: string): number | undefined => {
|
||||
if (value.trim() === "") {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const parsed = Number(value);
|
||||
if (!Number.isFinite(parsed) || !Number.isInteger(parsed)) {
|
||||
return undefined;
|
||||
}
|
||||
if (parsed < MIN_CONTENT_LENGTH_LIMIT || parsed > MAX_CONTENT_LENGTH_LIMIT) {
|
||||
return undefined;
|
||||
}
|
||||
return parsed;
|
||||
};
|
||||
|
||||
const MemoRelatedSettings = () => {
|
||||
const t = useTranslate();
|
||||
const saveInstanceSetting = useInstanceSettingUpdater();
|
||||
const { memoRelatedSetting: originalSetting } = useInstance();
|
||||
const [memoRelatedSetting, setMemoRelatedSetting] = useState<InstanceSetting_MemoRelatedSetting>(originalSetting);
|
||||
const [contentLengthLimitInput, setContentLengthLimitInput] = useState(String(originalSetting.contentLengthLimit));
|
||||
const [editingReaction, setEditingReaction] = useState<string>("");
|
||||
|
||||
useEffect(() => {
|
||||
setMemoRelatedSetting(originalSetting);
|
||||
setContentLengthLimitInput(String(originalSetting.contentLengthLimit));
|
||||
}, [originalSetting]);
|
||||
|
||||
const updatePartialSetting = (partial: Partial<InstanceSetting_MemoRelatedSetting>) => {
|
||||
|
|
@ -50,18 +70,34 @@ const MemoRelatedSettings = () => {
|
|||
};
|
||||
|
||||
const handleUpdateSetting = async () => {
|
||||
const contentLengthLimit = parseContentLengthLimit(contentLengthLimitInput);
|
||||
if (contentLengthLimit === undefined) {
|
||||
toast.error(
|
||||
t("setting.memo.content-length-limit-error", {
|
||||
min: MIN_CONTENT_LENGTH_LIMIT,
|
||||
max: MAX_CONTENT_LENGTH_LIMIT,
|
||||
}),
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
if (memoRelatedSetting.reactions.length === 0) {
|
||||
toast.error(t("setting.memo.reactions-required"));
|
||||
return;
|
||||
}
|
||||
|
||||
const updatedMemoRelatedSetting = create(InstanceSetting_MemoRelatedSettingSchema, {
|
||||
...memoRelatedSetting,
|
||||
contentLengthLimit,
|
||||
});
|
||||
|
||||
await saveInstanceSetting({
|
||||
key: InstanceSetting_Key.MEMO_RELATED,
|
||||
setting: create(InstanceSettingSchema, {
|
||||
name: buildInstanceSettingName(InstanceSetting_Key.MEMO_RELATED),
|
||||
value: {
|
||||
case: "memoRelatedSetting",
|
||||
value: memoRelatedSetting,
|
||||
value: updatedMemoRelatedSetting,
|
||||
},
|
||||
}),
|
||||
errorContext: "Update memo-related settings",
|
||||
|
|
@ -87,11 +123,17 @@ const MemoRelatedSettings = () => {
|
|||
<Input
|
||||
className="w-28 font-mono"
|
||||
type="number"
|
||||
min={0}
|
||||
value={memoRelatedSetting.contentLengthLimit}
|
||||
onChange={(event) => updatePartialSetting({ contentLengthLimit: Number(event.target.value) })}
|
||||
min={MIN_CONTENT_LENGTH_LIMIT}
|
||||
max={MAX_CONTENT_LENGTH_LIMIT}
|
||||
step={1}
|
||||
required
|
||||
aria-label={t("setting.memo.content-length-limit")}
|
||||
value={contentLengthLimitInput}
|
||||
onChange={(event) => setContentLengthLimitInput(event.target.value)}
|
||||
/>
|
||||
<span className="text-xs text-muted-foreground">{t("setting.memo.bytes-unit")}</span>
|
||||
<span className="text-xs text-muted-foreground">
|
||||
{t("setting.memo.content-length-limit-minimum", { min: MIN_CONTENT_LENGTH_LIMIT })}
|
||||
</span>
|
||||
</div>
|
||||
</SettingListItem>
|
||||
</SettingList>
|
||||
|
|
@ -142,7 +184,10 @@ const MemoRelatedSettings = () => {
|
|||
</SettingGroup>
|
||||
|
||||
<div className="w-full flex justify-end">
|
||||
<Button disabled={isEqual(memoRelatedSetting, originalSetting)} onClick={handleUpdateSetting}>
|
||||
<Button
|
||||
disabled={isEqual(memoRelatedSetting, originalSetting) && contentLengthLimitInput === String(originalSetting.contentLengthLimit)}
|
||||
onClick={handleUpdateSetting}
|
||||
>
|
||||
{t("common.save")}
|
||||
</Button>
|
||||
</div>
|
||||
|
|
|
|||
|
|
@ -651,6 +651,8 @@
|
|||
"configured-reactions": "Configured reactions",
|
||||
"content-length-limit": "Content length limit (Byte)",
|
||||
"content-length-limit-description": "Maximum memo body size accepted by the server.",
|
||||
"content-length-limit-error": "Content length limit must be a whole number between {{min}} and {{max}} bytes.",
|
||||
"content-length-limit-minimum": "Minimum: {{min}} bytes",
|
||||
"double-click-edit-description": "Allow users to open memo editing by double-clicking a memo.",
|
||||
"editing-description": "Control memo editing behavior and server-side content limits.",
|
||||
"editing-title": "Editing",
|
||||
|
|
|
|||
90
web/tests/memo-related-settings.test.tsx
Normal file
90
web/tests/memo-related-settings.test.tsx
Normal file
|
|
@ -0,0 +1,90 @@
|
|||
import { create } from "@bufbuild/protobuf";
|
||||
import { fireEvent, render, screen, waitFor } from "@testing-library/react";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import MemoRelatedSettings from "@/components/Settings/MemoRelatedSettings";
|
||||
import {
|
||||
type InstanceSetting,
|
||||
InstanceSetting_Key,
|
||||
InstanceSetting_MemoRelatedSettingSchema,
|
||||
} from "@/types/proto/api/v1/instance_service_pb";
|
||||
|
||||
const mocks = vi.hoisted(() => ({
|
||||
instance: {
|
||||
memoRelatedSetting: {},
|
||||
updateSetting: vi.fn(),
|
||||
fetchSetting: vi.fn(),
|
||||
},
|
||||
toastError: vi.fn(),
|
||||
toastSuccess: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("@/contexts/InstanceContext", () => ({
|
||||
useInstance: () => mocks.instance,
|
||||
}));
|
||||
|
||||
vi.mock("@/utils/i18n", () => ({
|
||||
useTranslate: () => (key: string) => key,
|
||||
}));
|
||||
|
||||
vi.mock("react-hot-toast", () => ({
|
||||
toast: {
|
||||
error: mocks.toastError,
|
||||
success: mocks.toastSuccess,
|
||||
},
|
||||
}));
|
||||
|
||||
describe("<MemoRelatedSettings> content length limit", () => {
|
||||
beforeEach(() => {
|
||||
mocks.instance.memoRelatedSetting = create(InstanceSetting_MemoRelatedSettingSchema, {
|
||||
contentLengthLimit: 32_768,
|
||||
enableDoubleClickEdit: false,
|
||||
reactions: ["thumbs-up"],
|
||||
});
|
||||
mocks.instance.updateSetting.mockReset().mockResolvedValue(undefined);
|
||||
mocks.instance.fetchSetting.mockReset().mockResolvedValue(undefined);
|
||||
mocks.toastError.mockReset();
|
||||
mocks.toastSuccess.mockReset();
|
||||
});
|
||||
|
||||
it.each([8_192, 16_384])("saves a valid %i-byte limit in the memo-related setting", async (contentLengthLimit) => {
|
||||
render(<MemoRelatedSettings />);
|
||||
|
||||
const input = screen.getByRole("spinbutton", { name: "setting.memo.content-length-limit" });
|
||||
expect(input).toHaveAttribute("min", "8192");
|
||||
expect(input).toHaveAttribute("max", "2147483647");
|
||||
expect(input).toHaveAttribute("step", "1");
|
||||
expect(screen.getByText("setting.memo.content-length-limit-minimum")).toBeVisible();
|
||||
|
||||
fireEvent.change(input, { target: { value: String(contentLengthLimit) } });
|
||||
fireEvent.click(screen.getByRole("button", { name: "common.save" }));
|
||||
|
||||
await waitFor(() => expect(mocks.instance.updateSetting).toHaveBeenCalledTimes(1));
|
||||
const setting = mocks.instance.updateSetting.mock.calls[0][0] as InstanceSetting;
|
||||
expect(setting.name).toBe("instance/settings/MEMO_RELATED");
|
||||
expect(setting.value.case).toBe("memoRelatedSetting");
|
||||
if (setting.value.case !== "memoRelatedSetting") {
|
||||
throw new Error("Expected memo-related setting payload");
|
||||
}
|
||||
expect(setting.value.value.contentLengthLimit).toBe(contentLengthLimit);
|
||||
expect(setting.value.value.reactions).toEqual(["thumbs-up"]);
|
||||
expect(mocks.instance.fetchSetting).toHaveBeenCalledWith(InstanceSetting_Key.MEMO_RELATED);
|
||||
expect(mocks.toastError).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it.each([
|
||||
["an empty value", ""],
|
||||
["a value below the minimum", "8191"],
|
||||
["a fractional value", "8192.5"],
|
||||
["a value above int32", "2147483648"],
|
||||
])("blocks %s before calling the update API", (_scenario, value) => {
|
||||
render(<MemoRelatedSettings />);
|
||||
|
||||
const input = screen.getByRole("spinbutton", { name: "setting.memo.content-length-limit" });
|
||||
fireEvent.change(input, { target: { value } });
|
||||
fireEvent.click(screen.getByRole("button", { name: "common.save" }));
|
||||
|
||||
expect(mocks.instance.updateSetting).not.toHaveBeenCalled();
|
||||
expect(mocks.instance.fetchSetting).not.toHaveBeenCalled();
|
||||
expect(mocks.toastError).toHaveBeenCalledWith("setting.memo.content-length-limit-error");
|
||||
});
|
||||
});
|
||||
Loading…
Reference in a new issue