From 8a51dd7a8ff1dc19ad32edd78f7cb2203d921263 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=A4=A9=E6=B5=B7=20=E5=8E=9F?= Date: Tue, 9 Jun 2026 00:04:32 +0900 Subject: [PATCH 1/2] =?UTF-8?q?fix(api):=20=E7=BB=9F=E4=B8=80=E5=9B=BE?= =?UTF-8?q?=E7=89=87=E4=B8=8A=E4=BC=A0=E9=99=90=E5=88=B6=E5=B9=B6=E8=BD=AC?= =?UTF-8?q?=E6=8D=A2=20TIFF=20=E4=B8=BA=20PNG?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit API 上传大小限制改为配置驱动，multipart 413 返回标准 JSON，并将 TIFF 上传规范化保存为 PNG。 --- config/config.exs | 1 + config/runtime.exs | 13 ++ docs/features/public-rest-api.md | 18 +- docs/features/upload-images.md | 4 +- .../2026-06-08-api-tiff-upload-rejected.md | 28 +++ ...026-06-08-api-upload-too-large-response.md | 27 +++ lib/small_sdk/image_magick.ex | 27 +++ lib/vmemo/memo/image_storage.ex | 54 ++++- lib/vmemo_web/api/v1/image_controller.ex | 84 +++++++- lib/vmemo_web/controllers/file_controller.ex | 4 +- lib/vmemo_web/endpoint.ex | 56 ++++- lib/vmemo_web/live/components/search_box.ex | 4 +- lib/vmemo_web/live/components/upload_form.ex | 4 +- lib/vmemo_web/multipart_parser.ex | 97 +++++++++ test/vmemo/memo/image_storage_test.exs | 20 ++ .../api/v1/image_controller_test.exs | 193 ++++++++++++++++++ .../controllers/file_controller_test.exs | 14 ++ 17 files changed, 627 insertions(+), 21 deletions(-) create mode 100644 docs/postmortem/2026-06-08-api-tiff-upload-rejected.md create mode 100644 docs/postmortem/2026-06-08-api-upload-too-large-response.md create mode 100644 lib/vmemo_web/multipart_parser.ex diff --git a/config/config.exs b/config/config.exs index aafb5258..4003bc9b 100644 --- a/config/config.exs +++ b/config/config.exs @@ -57,6 +57,7 @@ config :vmemo, ecto_repos: [Vmemo.Repo], generators: [timestamp_type: :utc_datetime], ash_domains: [Vmemo.Admin, Vmemo.Account, Vmemo.Memo, Vmemo.Ai, Vmemo.Chat, Vmemo.Jobs], + image_upload_max_file_size: 50_000_000, user_data_import_typesense_chunk_size: 50, user_data_import_typesense_chunk_pause_ms: 50 diff --git a/config/runtime.exs b/config/runtime.exs index 94d0794b..25c308ed 100644 --- a/config/runtime.exs +++ b/config/runtime.exs @@ -21,6 +21,19 @@ config :vmemo, config :vmemo, Vmemo.Repo, url: database_url +if image_upload_max_file_size = System.get_env("IMAGE_UPLOAD_MAX_FILE_SIZE") do + case Integer.parse(image_upload_max_file_size) do + {value, ""} when value > 0 -> + config :vmemo, image_upload_max_file_size: value + + _ -> + raise """ + environment variable IMAGE_UPLOAD_MAX_FILE_SIZE is invalid. + It must be a positive integer. + """ + end +end + if config_env() in [:dev, :test] do openrouter_api_key = System.get_env("OPENROUTER_API_KEY") diff --git a/docs/features/public-rest-api.md b/docs/features/public-rest-api.md index fec27a76..7485ace3 100644 --- a/docs/features/public-rest-api.md +++ b/docs/features/public-rest-api.md @@ -40,7 +40,7 @@ Request form fields: | Name | Type | Required | Description | |---|---|---|---| -| `file` | File | Yes | Image file (`PNG`, `JPG`, `JPEG`, `GIF`, `WEBP`) | +| `file` | File | Yes | Image file (`PNG`, `JPG`, `JPEG`, `GIF`, `WEBP`, `TIF`, `TIFF`) | | `note` | String | No | User note for the image | Example: @@ -125,6 +125,17 @@ All API errors follow this shape: Notes: - `statusCode` is the HTTP status code. - `statusMessage` is the standard HTTP reason phrase. +- `413` upload errors include the uploaded file or request body size, the app/API limit, and a retry suggestion. + +## API Logging + +API requests log a request summary and a response summary for `/api/*` routes. + +Logged fields include: +- Request: method, path, content type, content length +- Response: method, path, HTTP status, duration, response content type, user id when available + +Sensitive request data such as bearer tokens and file contents are not logged. ### Common Errors @@ -132,14 +143,17 @@ Notes: |---|---|---| | `400` | `No file provided` / `Invalid image format` | Invalid upload payload or file content | | `401` | `Invalid or missing API token` | Missing, invalid, disabled, or expired token | +| `413` | `Uploaded image is ...` / `Uploaded request body is ...` | Uploaded file or request body exceeds the API upload limit | | `404` | `Image not found` | Image not found or not owned by token user | | `500` | `Failed to create image` | Failed to create image record | | `500` | `Failed to delete image` | Failed to delete image | ## File Constraints -- Allowed formats: `PNG`, `JPG`, `JPEG`, `GIF`, `WEBP` +- Allowed formats: `PNG`, `JPG`, `JPEG`, `GIF`, `WEBP`, `TIF`, `TIFF` +- Max file size: `50 MB` - Validation: both file extension and file header (magic bytes) are checked +- TIFF uploads are converted to PNG before storage so image pages and thumbnails work in browsers. - Files with non-image content (e.g. a `.png` extension on a PDF) are rejected ## Usage Examples diff --git a/docs/features/upload-images.md b/docs/features/upload-images.md index a2ef7729..502f33a2 100644 --- a/docs/features/upload-images.md +++ b/docs/features/upload-images.md @@ -2,7 +2,7 @@ ## 概要 用户通过 `UploadForm` 组件上传图片。每次提交生成一个 `upload_batch_id`，同批上传的图片共享该 ID。上传后图片进入 search embedding（Typesense）和 vision embedding（OpenRouter caption）异步处理流程。 -系统会保留上传原图到 storage；仅在调用外部 vision 服务前对大图执行一次预处理，以降低请求体积。 +系统会保留上传原图到 storage；TIFF 上传会先规范化保存为 PNG，以保证浏览器可展示。仅在调用外部 vision 服务前对大图执行一次预处理，以降低请求体积。 ## 架构 @@ -13,7 +13,7 @@ 4. 创建成功的图片自动触发 Oban jobs：`sync_typesense` + `generate_caption` ### Vision 调用前图片预处理 -- 存储策略：`storage/v1/...` 始终保留原图，不写回压缩图。 +- 存储策略：`storage/v1/...` 保留上传图片；TIFF 会在入库前转换为 PNG，其他格式不写回压缩图。 - 调用策略：仅在外部 vision 请求前处理图片，处理结果只用于本次请求。 - 处理规则： - 小图（< 500KB）跳过预处理。 diff --git a/docs/postmortem/2026-06-08-api-tiff-upload-rejected.md b/docs/postmortem/2026-06-08-api-tiff-upload-rejected.md new file mode 100644 index 00000000..b50af4f9 --- /dev/null +++ b/docs/postmortem/2026-06-08-api-tiff-upload-rejected.md @@ -0,0 +1,28 @@ +# API TIFF Upload Rejected And Not Web Displayable + +## What happened + +`POST /api/v1/images` returned `400` for a clipboard upload whose multipart file had `content_type: "image/tiff"` and a `.tiff` filename. + +The request was parsed successfully and was under the configured upload size limit, but the API rejected it during image type validation. + +After accepting TIFF, uploaded TIFF files were still stored with a `.tiff` extension. Image detail pages then referenced TIFF storage URLs or TIFF thumbnails, which are not reliably displayable in browsers. + +## Root cause + +The API image allowlist and magic-byte detection only covered PNG, JPEG, GIF, and WEBP. TIFF files from clipboard or screenshot tools were therefore treated as unsupported image uploads. + +TIFF is also a poor web display format: most browsers do not support it natively, so keeping TIFF as the stored display asset made image pages fragile. + +## Fix applied + +- Added TIFF to the API image MIME allowlist. +- Added TIFF magic-byte detection for little-endian and big-endian TIFF headers. +- Converted TIFF uploads to PNG in `ImageStorage.cp_file/3` before storage. +- Updated UI upload accept lists and REST API docs to include TIFF and document PNG normalization. +- Added a file response MIME mapping for stored `.tif` and `.tiff` files. +- Added regression tests for API TIFF upload, TIFF-to-PNG storage, and TIFF file response content type. + +## What we learned + +Clipboard image uploads can use image formats beyond the formats selected in the UI. API validation should accept the same practical image formats users can produce from common screenshot and clipboard workflows, but storage should normalize non-web-friendly formats into browser-friendly assets. diff --git a/docs/postmortem/2026-06-08-api-upload-too-large-response.md b/docs/postmortem/2026-06-08-api-upload-too-large-response.md new file mode 100644 index 00000000..8052487b --- /dev/null +++ b/docs/postmortem/2026-06-08-api-upload-too-large-response.md @@ -0,0 +1,27 @@ +# API Upload Too Large Response + +## What happened + +`POST /api/v1/images` rejected multipart uploads above Plug's default parser body limit before the request reached API authentication or `ImageController`. + +This made API uploads fail for files that the web UI accepts, and the too-large failure did not use the REST API JSON error shape. + +## Root cause + +The web UI used `allow_upload(:images, max_file_size: 12_000_000)`, while the Endpoint `Plug.Parsers` configuration did not set an explicit `:length`. + +`Plug.Parsers.RequestTooLargeError` is raised at the Endpoint parser layer, before the API controller can return its standard `%{statusCode, statusMessage, message}` response. + +## Fix applied + +- Added a shared `:image_upload_max_file_size` application config, overridable with `IMAGE_UPLOAD_MAX_FILE_SIZE`. +- Set the multipart parser body limit high enough to carry the shared image limit plus form overhead. +- Added multipart parser handling for oversized upload bodies so `/api/*` returns HTTP `413` with JSON. +- Added controller validation for actual multipart file size so API files over the UI limit return HTTP `413` with JSON. +- Improved `413` messages to include the uploaded size, configured limits, and retry guidance. +- Added API request/response summary logs without logging bearer tokens or file contents. +- Updated REST API documentation with the image file limit and `413` error. + +## What we learned + +Endpoint parser limits and LiveView upload limits are separate controls. API upload behavior should share the same file-size source of truth as UI uploads, and parser-layer failures need explicit API JSON handling because they happen before route pipelines and controllers. diff --git a/lib/small_sdk/image_magick.ex b/lib/small_sdk/image_magick.ex index 77339673..77e60b48 100644 --- a/lib/small_sdk/image_magick.ex +++ b/lib/small_sdk/image_magick.ex @@ -67,6 +67,18 @@ defmodule SmallSdk.ImageMagick do end end + def convert_to_png!(input_path, output_path) + when is_binary(input_path) and is_binary(output_path) do + {tool, use_magick_entrypoint?} = pick_tool!() + args = build_convert_to_png_args(use_magick_entrypoint?, input_path, output_path) + {_output, status} = System.cmd(tool, args, stderr_to_stdout: true) + + case status do + 0 -> :ok + _ -> raise "ImageMagick PNG conversion failed with exit status #{status}." + end + end + defp pick_tool! do cond do executable?("magick") -> {"magick", true} @@ -111,6 +123,21 @@ defmodule SmallSdk.ImageMagick do [in_path, "-auto-orient", "-resize", "#{max_side}x#{max_side}>", "-strip", out_path] end + defp build_convert_to_png_args(true, in_path, out_path) do + ["convert" | build_convert_to_png_args(false, in_path, out_path)] + end + + defp build_convert_to_png_args(false, in_path, out_path) do + [ + "#{in_path}[0]", + "-auto-orient", + "-strip", + "-define", + "png:compression-level=9", + out_path + ] + end + defp build_convert_args(in_path, out_path, mime_type, max_side, quality) do common = [in_path, "-auto-orient", "-resize", "#{max_side}x#{max_side}>", "-strip"] diff --git a/lib/vmemo/memo/image_storage.ex b/lib/vmemo/memo/image_storage.ex index 12303cba..a929dea5 100644 --- a/lib/vmemo/memo/image_storage.ex +++ b/lib/vmemo/memo/image_storage.ex @@ -6,7 +6,13 @@ defmodule Vmemo.Memo.ImageStorage do @thumb_sizes %{s: 320, m: 1280} def cp_file(src, user_id, filename) do - dest = FileSystem.cp!(src, gen_dest(user_id, filename)) + dest = + if tiff_upload?(src, filename) do + convert_tiff_to_png!(src, gen_dest(user_id, png_filename(filename))) + else + FileSystem.cp!(src, gen_dest(user_id, filename)) + end + {:ok, dest} end @@ -65,4 +71,50 @@ defmodule Vmemo.Memo.ImageStorage do Path.join([user_id, "images", timestamp <> "_" <> normalized_filename]) end + + defp convert_tiff_to_png!(src, dest) do + storage_dest = Path.join(["storage/v1", dest]) + storage_dest |> Path.dirname() |> File.mkdir_p!() + ImageMagick.convert_to_png!(src, storage_dest) + storage_dest + end + + defp png_filename(filename) do + filename + |> to_string() + |> Path.rootname() + |> Kernel.<>(".png") + end + + defp tiff_upload?(src, filename) do + tiff_extension?(filename) or tiff_header?(src) + end + + defp tiff_extension?(filename) do + filename + |> to_string() + |> Path.extname() + |> String.downcase() + |> then(&(&1 in [".tif", ".tiff"])) + end + + defp tiff_header?(path) when is_binary(path) do + case File.open(path, [:read, :binary]) do + {:ok, io} -> + try do + case :file.read(io, 4) do + {:ok, <<"II", marker, 0>>} when marker in [42, 43] -> true + {:ok, <<"MM", 0, marker>>} when marker in [42, 43] -> true + _ -> false + end + after + File.close(io) + end + + {:error, _reason} -> + false + end + end + + defp tiff_header?(_path), do: false end diff --git a/lib/vmemo_web/api/v1/image_controller.ex b/lib/vmemo_web/api/v1/image_controller.ex index 61fa4df3..e1f28298 100644 --- a/lib/vmemo_web/api/v1/image_controller.ex +++ b/lib/vmemo_web/api/v1/image_controller.ex @@ -103,16 +103,26 @@ defmodule VmemoWeb.Api.V1.ImageController do end defp validate_and_process_upload(%Plug.Upload{} = upload) do + with :ok <- validate_upload_file_size(upload.path) do + process_sized_upload(upload) + end + end + + defp process_sized_upload(%Plug.Upload{} = upload) do if clipboard_html_upload?(upload) do validate_and_process_clipboard_html(upload.path) else - with {:ok, mime_type} <- validate_image_content(upload.path), - :ok <- validate_upload_content_type(upload.content_type, mime_type) do - filename = generate_filename(upload.filename, mime_type) - {:ok, %{path: upload.path, filename: filename}} - else - {:error, reason} -> {:error, reason} - end + validate_and_process_image_upload(upload) + end + end + + defp validate_and_process_image_upload(%Plug.Upload{} = upload) do + with {:ok, mime_type} <- validate_image_content(upload.path), + :ok <- validate_upload_content_type(upload.content_type, mime_type) do + filename = generate_filename(upload.filename, mime_type) + {:ok, %{path: upload.path, filename: filename}} + else + {:error, reason} -> {:error, reason} end end @@ -373,7 +383,8 @@ defmodule VmemoWeb.Api.V1.ImageController do defp validate_upload_content_type(_content_type, _detected_mime_type), do: {:error, "Invalid file type. Only image files are allowed"} - defp supported_mime_types, do: ~w(image/png image/jpeg image/jpg image/gif image/webp) + defp supported_mime_types, + do: ~w(image/png image/jpeg image/jpg image/gif image/webp image/tiff) defp normalize_content_type(content_type) when is_binary(content_type) do content_type @@ -395,6 +406,20 @@ defmodule VmemoWeb.Api.V1.ImageController do defp validate_non_empty_binary(binary) when is_binary(binary) and byte_size(binary) > 0, do: :ok defp validate_non_empty_binary(_), do: {:error, "No file provided"} + defp validate_upload_file_size(path) do + case File.stat(path) do + {:ok, %{size: size}} -> + if size <= image_upload_max_file_size() do + :ok + else + {:error, {:image_too_large, size}} + end + + {:error, reason} -> + {:error, "Failed to read file: #{reason}"} + end + end + defp safe_upload_path?(path) when is_binary(path) do normalized_upload_path = normalize_private_tmp_path(Path.expand(path)) normalized_tmp_root = normalize_private_tmp_path(Path.expand(System.tmp_dir!())) @@ -437,7 +462,7 @@ defmodule VmemoWeb.Api.V1.ImageController do defp pick_upload_extension(original_filename, mime_type) do case Path.extname(original_filename) |> String.downcase() do - ext when ext in ~w(.png .jpg .jpeg .gif .webp) -> ext + ext when ext in ~w(.png .jpg .jpeg .gif .webp .tif .tiff) -> ext _ -> mime_type_to_extension(mime_type) end end @@ -446,6 +471,7 @@ defmodule VmemoWeb.Api.V1.ImageController do defp mime_type_to_extension("image/jpeg"), do: ".jpg" defp mime_type_to_extension("image/gif"), do: ".gif" defp mime_type_to_extension("image/webp"), do: ".webp" + defp mime_type_to_extension("image/tiff"), do: ".tiff" defp mime_type_to_extension(_), do: ".jpg" defp detect_mime_type_from_binary(<<0xFF, 0xD8, 0xFF, _::binary>>), do: "image/jpeg" @@ -461,6 +487,9 @@ defmodule VmemoWeb.Api.V1.ImageController do defp detect_mime_type_from_binary(<<"RIFF", _::binary-size(4), "WEBP", _::binary>>), do: "image/webp" + defp detect_mime_type_from_binary(<<"II", 42, 0, _::binary>>), do: "image/tiff" + defp detect_mime_type_from_binary(<<"MM", 0, 42, _::binary>>), do: "image/tiff" + defp detect_mime_type_from_binary(_), do: nil defp process_image_upload(conn, path, filename, params, current_user, upload_meta) do @@ -506,7 +535,9 @@ defmodule VmemoWeb.Api.V1.ImageController do } end - defp error_response(conn, status_code, message) do + defp error_response(conn, status_code, reason) do + {status_code, message} = error_status_and_message(reason, status_code) + conn |> put_status(status_code) |> json(%{ @@ -515,4 +546,37 @@ defmodule VmemoWeb.Api.V1.ImageController do message: message }) end + + defp error_status_and_message({:image_too_large, size}, _status_code) do + {413, image_too_large_message(size)} + end + + defp error_status_and_message(message, status_code), do: {status_code, message} + + defp image_upload_max_file_size do + Application.fetch_env!(:vmemo, :image_upload_max_file_size) + end + + defp image_too_large_message(size) do + "Uploaded image is #{format_size(size)}, which exceeds the app limit of #{format_size(image_upload_max_file_size())}. Compress the image or upload a smaller file, then try again." + end + + defp format_size(bytes) do + mb = + bytes + |> Kernel./(1_000_000) + |> :erlang.float_to_binary(decimals: 2) + + "#{format_integer(bytes)} bytes (#{mb} MB)" + end + + defp format_integer(integer) do + integer + |> Integer.to_string() + |> String.graphemes() + |> Enum.reverse() + |> Enum.chunk_every(3) + |> Enum.map_join(",", &Enum.join/1) + |> String.reverse() + end end diff --git a/lib/vmemo_web/controllers/file_controller.ex b/lib/vmemo_web/controllers/file_controller.ex index 508695ae..536330b0 100644 --- a/lib/vmemo_web/controllers/file_controller.ex +++ b/lib/vmemo_web/controllers/file_controller.ex @@ -7,7 +7,9 @@ defmodule VmemoWeb.FileController do ".jpg" => "image/jpeg", ".jpeg" => "image/jpeg", ".gif" => "image/gif", - ".webp" => "image/webp" + ".webp" => "image/webp", + ".tif" => "image/tiff", + ".tiff" => "image/tiff" } def show(conn, %{"user_id" => user_id, "filename" => filename}) do diff --git a/lib/vmemo_web/endpoint.ex b/lib/vmemo_web/endpoint.ex index d6daea74..17c3ab97 100644 --- a/lib/vmemo_web/endpoint.ex +++ b/lib/vmemo_web/endpoint.ex @@ -1,6 +1,7 @@ defmodule VmemoWeb.Endpoint do use Sentry.PlugCapture use Phoenix.Endpoint, otp_app: :vmemo + require Logger # The session will be stored in the cookie and signed, # this means its contents can be read but not tampered with. @@ -51,9 +52,10 @@ defmodule VmemoWeb.Endpoint do plug Plug.RequestId plug Plug.Telemetry, event_prefix: [:phoenix, :endpoint] + plug :log_api_request plug Plug.Parsers, - parsers: [:urlencoded, :multipart, :json], + parsers: [:urlencoded, VmemoWeb.MultipartParser, :json], pass: ["*/*"], json_decoder: Phoenix.json_library() @@ -62,4 +64,56 @@ defmodule VmemoWeb.Endpoint do plug Plug.Session, @session_options plug Sentry.PlugContext plug VmemoWeb.Router + + defp log_api_request(%Plug.Conn{request_path: "/api/" <> _} = conn, _opts) do + start_time = System.monotonic_time() + + Logger.info( + "api_request start method=#{conn.method} path=#{conn.request_path} request_content_type=#{request_content_type(conn)} request_content_length=#{request_content_length(conn)}" + ) + + Plug.Conn.register_before_send(conn, fn conn -> + duration_ms = + System.monotonic_time() + |> Kernel.-(start_time) + |> System.convert_time_unit(:native, :millisecond) + + Logger.info( + "api_response sent method=#{conn.method} path=#{conn.request_path} status=#{conn.status} duration_ms=#{duration_ms} response_content_type=#{response_content_type(conn)} user_id=#{user_id(conn)}" + ) + + conn + end) + end + + defp log_api_request(conn, _opts), do: conn + + defp request_content_type(conn) do + conn + |> Plug.Conn.get_req_header("content-type") + |> List.first() + |> empty_to_unknown() + end + + defp request_content_length(conn) do + conn + |> Plug.Conn.get_req_header("content-length") + |> List.first() + |> empty_to_unknown() + end + + defp response_content_type(conn) do + conn.resp_headers + |> List.keyfind("content-type", 0) + |> case do + {_key, value} -> empty_to_unknown(value) + nil -> "unknown" + end + end + + defp user_id(%Plug.Conn{assigns: %{current_user: %{id: id}}}) when not is_nil(id), do: id + defp user_id(_conn), do: "none" + + defp empty_to_unknown(value) when is_binary(value) and value != "", do: value + defp empty_to_unknown(_value), do: "unknown" end diff --git a/lib/vmemo_web/live/components/search_box.ex b/lib/vmemo_web/live/components/search_box.ex index 33327703..ef84f9bd 100644 --- a/lib/vmemo_web/live/components/search_box.ex +++ b/lib/vmemo_web/live/components/search_box.ex @@ -13,9 +13,9 @@ defmodule VmemoWeb.LiveComponents.SearchBox do |> assign(:q, "") |> assign(:submit_error, nil) |> allow_upload(:image, - accept: ~w(.png .jpg .jpeg .gif .webp), + accept: ~w(.png .jpg .jpeg .gif .webp .tif .tiff), max_entries: 100, - max_file_size: 12_000_000 + max_file_size: Application.fetch_env!(:vmemo, :image_upload_max_file_size) )} end diff --git a/lib/vmemo_web/live/components/upload_form.ex b/lib/vmemo_web/live/components/upload_form.ex index 3a498f75..7b6d92c9 100644 --- a/lib/vmemo_web/live/components/upload_form.ex +++ b/lib/vmemo_web/live/components/upload_form.ex @@ -19,9 +19,9 @@ defmodule VmemoWeb.LiveComponents.UploadForm do socket = socket |> allow_upload(:images, - accept: ~w(.png .jpg .jpeg .gif .webp), + accept: ~w(.png .jpg .jpeg .gif .webp .tif .tiff), max_entries: 100, - max_file_size: 12_000_000 + max_file_size: Application.fetch_env!(:vmemo, :image_upload_max_file_size) ) {:ok, socket} diff --git a/lib/vmemo_web/multipart_parser.ex b/lib/vmemo_web/multipart_parser.ex new file mode 100644 index 00000000..bacef166 --- /dev/null +++ b/lib/vmemo_web/multipart_parser.ex @@ -0,0 +1,97 @@ +defmodule VmemoWeb.MultipartParser do + @moduledoc false + + import Plug.Conn + + alias Plug.Conn.Status + + @behaviour Plug.Parsers + @api_body_limit_buffer 1_000_000 + @multipart Plug.Parsers.MULTIPART + + @impl true + def init(opts), do: opts + + @impl true + def parse(conn, "multipart", subtype, headers, opts) do + opts = + opts + |> Keyword.put(:length, api_multipart_body_limit()) + |> @multipart.init() + + case @multipart.parse(conn, "multipart", subtype, headers, opts) do + {:error, :too_large, conn} -> + {:ok, %{}, too_large_response(conn)} + + result -> + result + end + end + + def parse(conn, _type, _subtype, _headers, _opts), do: {:next, conn} + + defp too_large_response(conn) do + status_code = 413 + status_message = Status.reason_phrase(status_code) + message = api_body_too_large_message(request_content_length(conn)) + + body = + Phoenix.json_library().encode!(%{ + statusCode: status_code, + statusMessage: status_message, + message: message + }) + + conn + |> put_resp_content_type("application/json") + |> send_resp(status_code, body) + |> halt() + end + + defp request_content_length(conn) do + conn + |> get_req_header("content-length") + |> List.first() + |> parse_positive_integer() + end + + defp parse_positive_integer(value) when is_binary(value) do + case Integer.parse(value) do + {integer, ""} when integer >= 0 -> integer + _other -> nil + end + end + + defp parse_positive_integer(_value), do: nil + + defp api_multipart_body_limit do + Application.fetch_env!(:vmemo, :image_upload_max_file_size) + @api_body_limit_buffer + end + + defp api_body_too_large_message(size) when is_integer(size) and size >= 0 do + "Uploaded request body is #{format_size(size)}, which exceeds the API body limit of #{format_size(api_multipart_body_limit())} and the app image limit of #{format_size(Application.fetch_env!(:vmemo, :image_upload_max_file_size))}. Compress the image or upload a smaller file, then try again." + end + + defp api_body_too_large_message(_size) do + "Uploaded request body is too large. The API body limit is #{format_size(api_multipart_body_limit())} and the app image limit is #{format_size(Application.fetch_env!(:vmemo, :image_upload_max_file_size))}. Compress the image or upload a smaller file, then try again." + end + + defp format_size(bytes) do + mb = + bytes + |> Kernel./(1_000_000) + |> :erlang.float_to_binary(decimals: 2) + + "#{format_integer(bytes)} bytes (#{mb} MB)" + end + + defp format_integer(integer) do + integer + |> Integer.to_string() + |> String.graphemes() + |> Enum.reverse() + |> Enum.chunk_every(3) + |> Enum.map_join(",", &Enum.join/1) + |> String.reverse() + end +end diff --git a/test/vmemo/memo/image_storage_test.exs b/test/vmemo/memo/image_storage_test.exs index 6c0bfba7..1866e7ab 100644 --- a/test/vmemo/memo/image_storage_test.exs +++ b/test/vmemo/memo/image_storage_test.exs @@ -46,10 +46,30 @@ defmodule Vmemo.Memo.ImageStorageTest do ImageStorage.storage_path_from_url("https://cdn.example.com/demo.jpg", user_id) end + test "cp_file/3 converts tiff uploads to png for browser display" do + user_id = "u-#{System.unique_integer([:positive])}" + src = Path.join(System.tmp_dir!(), "vmemo-test-#{System.unique_integer([:positive])}.tiff") + File.write!(src, tiff_binary()) + + on_exit(fn -> + File.rm(src) + File.rm_rf!(Path.join([@storage_prefix, user_id])) + end) + + assert {:ok, dest} = ImageStorage.cp_file(src, user_id, "clipboard.tiff") + assert Path.extname(dest) == ".png" + assert <<0x89, 0x50, 0x4E, 0x47, _::binary>> = File.read!(dest) + end + test "storage_path_from_url/2 returns invalid_url for invalid params" do assert {:error, :invalid_url} = ImageStorage.storage_path_from_url(nil, "u1") assert {:error, :invalid_url} = ImageStorage.storage_path_from_url("/storage/v1/u1/images/a.png", nil) end + + defp tiff_binary do + "SUkqAAoAAAD//w8AAAEDAAEAAAABAAAAAQEDAAEAAAABAAAAAgEDAAEAAAAQAAAAAwEDAAEAAAABAAAABgEDAAEAAAABAAAACgEDAAEAAAABAAAAEQEEAAEAAAAIAAAAEgEDAAEAAAABAAAAFQEDAAEAAAABAAAAFgEDAAEAAAABAAAAFwEEAAEAAAACAAAAHAEDAAEAAAABAAAAKQEDAAIAAAAAAAEAPgEFAAIAAAD0AAAAPwEFAAYAAADEAAAAAAAAAIXrUQAAAIAAw/WoAAAAAALNzEwAAAAAAc3MTAAAAIAAzcxMAAAAAAKPwvUAAAAAEDcaoAAAAAACK4cKAAAAIAA=" + |> Base.decode64!() + end end diff --git a/test/vmemo_web/api/v1/image_controller_test.exs b/test/vmemo_web/api/v1/image_controller_test.exs index ac21913c..315697ec 100644 --- a/test/vmemo_web/api/v1/image_controller_test.exs +++ b/test/vmemo_web/api/v1/image_controller_test.exs @@ -11,9 +11,12 @@ defmodule VmemoWeb.Api.V1.ImageControllerTest do alias Vmemo.Memo.ImageNote alias Vmemo.Memo.Note + import ExUnit.CaptureLog import Vmemo.AccountFixtures import VmemoWeb.ApiFixtures @fixture_image Path.expand("test/support/fixtures/images/wall-e.png") + @ui_image_max_file_size Application.compile_env!(:vmemo, :image_upload_max_file_size) + @api_multipart_body_limit @ui_image_max_file_size + 1_000_000 describe "POST /api/v1/images - Create image" do setup %{conn: conn} do @@ -136,6 +139,33 @@ defmodule VmemoWeb.Api.V1.ImageControllerTest do assert conn.status == 200 end + test "accepts tiff clipboard upload from API clients", %{ + conn: conn, + raw_token: raw_token, + user: user + } do + test_image_path = create_test_tiff() + + conn = + conn + |> put_req_header("authorization", "Bearer #{raw_token}") + |> post(~p"/api/v1/images", %{ + "file" => %Plug.Upload{ + path: test_image_path, + filename: "Clipboard Jun 8, 2026 at 20.07.tiff", + content_type: "image/tiff" + } + }) + + assert conn.status == 200 + response = json_response(conn, 200) + assert is_binary(response["id"]) + + assert {:ok, image} = Image.get(response["id"], actor: user) + assert String.ends_with?(image.url, ".png") + refute String.ends_with?(image.url, ".tiff") + end + test "accepts content_type with parameters", %{conn: conn, raw_token: raw_token} do test_image_path = create_test_image() @@ -191,6 +221,110 @@ defmodule VmemoWeb.Api.V1.ImageControllerTest do assert conn.status == 200 end + test "accepts multipart image within UI size limit above Plug parser default", %{ + conn: conn, + raw_token: raw_token + } do + file_binary = large_png_binary(20_000_000) + {body, boundary} = multipart_body(file_binary) + + conn = + conn + |> put_req_header("authorization", "Bearer #{raw_token}") + |> put_req_header("accept", "application/json") + |> put_req_header("content-length", Integer.to_string(byte_size(body))) + |> put_req_header("content-type", "multipart/form-data; boundary=#{boundary}") + |> post(~p"/api/v1/images", body) + + assert conn.status == 200 + assert is_binary(json_response(conn, 200)["id"]) + end + + test "returns 413 standard JSON when multipart image exceeds UI size limit", %{ + conn: conn, + raw_token: raw_token + } do + file_binary = large_png_binary(@ui_image_max_file_size + 1) + {body, boundary} = multipart_body(file_binary) + + conn = + conn + |> put_req_header("authorization", "Bearer #{raw_token}") + |> put_req_header("accept", "application/json") + |> put_req_header("content-length", Integer.to_string(byte_size(body))) + |> put_req_header("content-type", "multipart/form-data; boundary=#{boundary}") + |> post(~p"/api/v1/images", body) + + assert json_response(conn, 413) == %{ + "statusCode" => 413, + "statusMessage" => "Request Entity Too Large", + "message" => + "Uploaded image is #{format_test_size(@ui_image_max_file_size + 1)}, which exceeds the app limit of #{format_test_size(@ui_image_max_file_size)}. Compress the image or upload a smaller file, then try again." + } + end + + test "returns 413 standard JSON when multipart request exceeds API body limit", %{ + conn: conn, + raw_token: raw_token + } do + file_binary = large_png_binary(@api_multipart_body_limit + 1) + {body, boundary} = multipart_body(file_binary) + + conn = + conn + |> put_req_header("authorization", "Bearer #{raw_token}") + |> put_req_header("accept", "application/json") + |> put_req_header("content-length", Integer.to_string(byte_size(body))) + |> put_req_header("content-type", "multipart/form-data; boundary=#{boundary}") + |> post(~p"/api/v1/images", body) + + response = json_response(conn, 413) + + assert response["statusCode"] == 413 + assert response["statusMessage"] == "Request Entity Too Large" + + assert response["message"] =~ + "Uploaded request body is #{format_test_size(byte_size(body))}" + + assert response["message"] =~ + "API body limit of #{format_test_size(@api_multipart_body_limit)}" + + assert response["message"] =~ + "app image limit of #{format_test_size(@ui_image_max_file_size)}" + + assert response["message"] =~ "Compress the image or upload a smaller file, then try again." + end + + test "logs API request and response summaries without sensitive payload", %{ + conn: conn, + raw_token: raw_token + } do + binary = File.read!(@fixture_image) + + previous_level = Logger.level() + + log = + try do + Logger.configure(level: :info) + + capture_log([level: :info], fn -> + conn + |> put_req_header("authorization", "Bearer #{raw_token}") + |> put_req_header("content-length", Integer.to_string(byte_size(binary))) + |> put_req_header("content-type", "image/png") + |> post(~p"/api/v1/images", binary) + end) + after + Logger.configure(level: previous_level) + end + + assert log =~ "api_request start method=POST path=/api/v1/images" + assert log =~ "api_response sent method=POST path=/api/v1/images status=200" + assert log =~ "request_content_length=#{byte_size(binary)}" + refute log =~ raw_token + refute log =~ binary + end + test "accepts data url payload in file field", %{conn: conn, raw_token: raw_token} do base64 = @fixture_image @@ -462,6 +596,65 @@ defmodule VmemoWeb.Api.V1.ImageControllerTest do temp_file end + defp create_test_tiff do + temp_file = Path.join(System.tmp_dir!(), "test_image_#{:rand.uniform(100_000)}.tiff") + File.write!(temp_file, tiff_binary()) + temp_file + end + + defp tiff_binary do + "SUkqAAoAAAD//w8AAAEDAAEAAAABAAAAAQEDAAEAAAABAAAAAgEDAAEAAAAQAAAAAwEDAAEAAAABAAAABgEDAAEAAAABAAAACgEDAAEAAAABAAAAEQEEAAEAAAAIAAAAEgEDAAEAAAABAAAAFQEDAAEAAAABAAAAFgEDAAEAAAABAAAAFwEEAAEAAAACAAAAHAEDAAEAAAABAAAAKQEDAAIAAAAAAAEAPgEFAAIAAAD0AAAAPwEFAAYAAADEAAAAAAAAAIXrUQAAAIAAw/WoAAAAAALNzEwAAAAAAc3MTAAAAIAAzcxMAAAAAAKPwvUAAAAAEDcaoAAAAAACK4cKAAAAIAA=" + |> Base.decode64!() + end + + defp large_png_binary(size) when size >= 12 do + png_header = <<0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A, 0, 0, 0, 0>> + png_header <> :binary.copy(<<0>>, size - byte_size(png_header)) + end + + defp multipart_body(file_binary) do + boundary = "vmemo-test-boundary-#{System.unique_integer([:positive])}" + + body = + IO.iodata_to_binary([ + "--", + boundary, + "\r\n", + "Content-Disposition: form-data; name=\"file\"; filename=\"large.png\"\r\n", + "Content-Type: image/png\r\n\r\n", + file_binary, + "\r\n--", + boundary, + "\r\n", + "Content-Disposition: form-data; name=\"note\"\r\n\r\n", + "large image", + "\r\n--", + boundary, + "--\r\n" + ]) + + {body, boundary} + end + + defp format_test_size(bytes) do + mb = + bytes + |> Kernel./(1_000_000) + |> :erlang.float_to_binary(decimals: 2) + + "#{format_test_integer(bytes)} bytes (#{mb} MB)" + end + + defp format_test_integer(integer) do + integer + |> Integer.to_string() + |> String.graphemes() + |> Enum.reverse() + |> Enum.chunk_every(3) + |> Enum.map_join(",", &Enum.join/1) + |> String.reverse() + end + defp create_image!(attrs) do ensure_fixture_image!(attrs) attrs = Map.put_new(attrs, :inner_purpose, nil) diff --git a/test/vmemo_web/controllers/file_controller_test.exs b/test/vmemo_web/controllers/file_controller_test.exs index 6d97a46d..56feaf46 100644 --- a/test/vmemo_web/controllers/file_controller_test.exs +++ b/test/vmemo_web/controllers/file_controller_test.exs @@ -47,6 +47,20 @@ defmodule VmemoWeb.FileControllerTest do assert get_resp_header(conn, "content-type") == ["image/png"] end + test "serves tiff images with image/tiff content type", %{ + conn: conn, + user_id: user_id, + image_dir: image_dir + } do + tiff = Path.join(image_dir, "sample.tiff") + File.write!(tiff, "tiff-data") + + conn = get(conn, ~p"/storage/v1/#{user_id}/images/sample.tiff") + + assert response(conn, 200) == "tiff-data" + assert get_resp_header(conn, "content-type") == ["image/tiff"] + end + test "returns 404 with no-store when both thumbnail and original are missing", %{ conn: conn, user_id: user_id From e2e54cb1b540261ca1177c75ea2f45824aae69bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=A4=A9=E6=B5=B7=20=E5=8E=9F?= Date: Tue, 9 Jun 2026 00:21:22 +0900 Subject: [PATCH 2/2] =?UTF-8?q?fix(upload):=20=E5=89=8D=E7=BD=AE=20TIFF=20?= =?UTF-8?q?=E8=BD=AC=E6=8D=A2=E9=81=BF=E5=85=8D=E6=B1=A1=E6=9F=93=20copy?= =?UTF-8?q?=20=E8=AF=AD=E4=B9=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 将 TIFF 转 PNG 移到 ImageUpload.store，恢复 ImageStorage.cp_file 为纯复制，并更新上传调用点和测试。 --- docs/features/upload-images.md | 2 +- .../2026-06-08-api-tiff-upload-rejected.md | 2 +- lib/vmemo/memo/image.ex | 12 +-- lib/vmemo/memo/image_storage.ex | 54 +------------ lib/vmemo/memo/image_upload.ex | 76 +++++++++++++++++++ lib/vmemo_web/api/v1/image_controller.ex | 6 +- lib/vmemo_web/live/components/search_box.ex | 12 +-- lib/vmemo_web/live/components/upload_form.ex | 7 +- test/vmemo/memo/image_storage_test.exs | 21 ++++- 9 files changed, 120 insertions(+), 72 deletions(-) create mode 100644 lib/vmemo/memo/image_upload.ex diff --git a/docs/features/upload-images.md b/docs/features/upload-images.md index 502f33a2..8e18346d 100644 --- a/docs/features/upload-images.md +++ b/docs/features/upload-images.md @@ -9,7 +9,7 @@ ### 上传流程 1. 用户选择图片 → LiveView `allow_upload(:images, ...)` 管理暂存 2. 提交时生成 `upload_batch_id`（UUID） -3. 逐个 `consume_uploaded_entry` → `ImageStorage.cp_file` → `Image.create_with_sync` +3. 逐个 `consume_uploaded_entry` → `ImageUpload.store` → `Image.create_with_sync` 4. 创建成功的图片自动触发 Oban jobs：`sync_typesense` + `generate_caption` ### Vision 调用前图片预处理 diff --git a/docs/postmortem/2026-06-08-api-tiff-upload-rejected.md b/docs/postmortem/2026-06-08-api-tiff-upload-rejected.md index b50af4f9..b867fe39 100644 --- a/docs/postmortem/2026-06-08-api-tiff-upload-rejected.md +++ b/docs/postmortem/2026-06-08-api-tiff-upload-rejected.md @@ -18,7 +18,7 @@ TIFF is also a poor web display format: most browsers do not support it natively - Added TIFF to the API image MIME allowlist. - Added TIFF magic-byte detection for little-endian and big-endian TIFF headers. -- Converted TIFF uploads to PNG in `ImageStorage.cp_file/3` before storage. +- Converted TIFF uploads to PNG in `ImageUpload.store/3` before copying into storage. - Updated UI upload accept lists and REST API docs to include TIFF and document PNG normalization. - Added a file response MIME mapping for stored `.tif` and `.tiff` files. - Added regression tests for API TIFF upload, TIFF-to-PNG storage, and TIFF file response content type. diff --git a/lib/vmemo/memo/image.ex b/lib/vmemo/memo/image.ex index 90a3eb2a..743ff39e 100644 --- a/lib/vmemo/memo/image.ex +++ b/lib/vmemo/memo/image.ex @@ -25,6 +25,7 @@ defmodule Vmemo.Memo.Image do alias Vmemo.Ai.Caption alias Vmemo.Memo.Changes.SyncImageTags alias Vmemo.Jobs.Job + alias Vmemo.Memo.ImageUpload alias Vmemo.Memo.ImageStorage alias Vmemo.SearchEngine.TsImage @@ -386,14 +387,14 @@ defmodule Vmemo.Memo.Image do storage_file_id = Ash.ActionInput.get_argument(input, :storage_file_id) user_id = actor.id - case ImageStorage.cp_file(temp_path, user_id, storage_file_id) do - {:ok, dest} -> + case ImageUpload.store(temp_path, user_id, storage_file_id) do + {:ok, %{dest: dest, filename: stored_filename}} -> case Ash.create( __MODULE__, %{ note: "", url: Path.join("/", dest), - file_id: storage_file_id, + file_id: stored_filename, user_id: user_id, inner_purpose: "search" }, @@ -1053,9 +1054,10 @@ defmodule Vmemo.Memo.Image do tmp_path <- Path.join("tmp/mcp_uploads", "#{System.system_time(:microsecond)}-#{final_filename}"), :ok <- File.write(tmp_path, binary), - {:ok, dest} <- ImageStorage.cp_file(tmp_path, user_id, final_filename) do + {:ok, %{dest: dest, filename: stored_filename}} <- + ImageUpload.store(tmp_path, user_id, final_filename) do _ = File.rm(tmp_path) - {:ok, %{url: Path.join("/", dest), file_id: final_filename}} + {:ok, %{url: Path.join("/", dest), file_id: stored_filename}} else :error -> {:error, "Invalid file payload"} diff --git a/lib/vmemo/memo/image_storage.ex b/lib/vmemo/memo/image_storage.ex index a929dea5..12303cba 100644 --- a/lib/vmemo/memo/image_storage.ex +++ b/lib/vmemo/memo/image_storage.ex @@ -6,13 +6,7 @@ defmodule Vmemo.Memo.ImageStorage do @thumb_sizes %{s: 320, m: 1280} def cp_file(src, user_id, filename) do - dest = - if tiff_upload?(src, filename) do - convert_tiff_to_png!(src, gen_dest(user_id, png_filename(filename))) - else - FileSystem.cp!(src, gen_dest(user_id, filename)) - end - + dest = FileSystem.cp!(src, gen_dest(user_id, filename)) {:ok, dest} end @@ -71,50 +65,4 @@ defmodule Vmemo.Memo.ImageStorage do Path.join([user_id, "images", timestamp <> "_" <> normalized_filename]) end - - defp convert_tiff_to_png!(src, dest) do - storage_dest = Path.join(["storage/v1", dest]) - storage_dest |> Path.dirname() |> File.mkdir_p!() - ImageMagick.convert_to_png!(src, storage_dest) - storage_dest - end - - defp png_filename(filename) do - filename - |> to_string() - |> Path.rootname() - |> Kernel.<>(".png") - end - - defp tiff_upload?(src, filename) do - tiff_extension?(filename) or tiff_header?(src) - end - - defp tiff_extension?(filename) do - filename - |> to_string() - |> Path.extname() - |> String.downcase() - |> then(&(&1 in [".tif", ".tiff"])) - end - - defp tiff_header?(path) when is_binary(path) do - case File.open(path, [:read, :binary]) do - {:ok, io} -> - try do - case :file.read(io, 4) do - {:ok, <<"II", marker, 0>>} when marker in [42, 43] -> true - {:ok, <<"MM", 0, marker>>} when marker in [42, 43] -> true - _ -> false - end - after - File.close(io) - end - - {:error, _reason} -> - false - end - end - - defp tiff_header?(_path), do: false end diff --git a/lib/vmemo/memo/image_upload.ex b/lib/vmemo/memo/image_upload.ex new file mode 100644 index 00000000..d3c0ef79 --- /dev/null +++ b/lib/vmemo/memo/image_upload.ex @@ -0,0 +1,76 @@ +defmodule Vmemo.Memo.ImageUpload do + @moduledoc false + + alias SmallSdk.ImageMagick + alias Vmemo.Memo.ImageStorage + + def store(src, user_id, filename) do + prepared = prepare_for_storage!(src, filename) + + try do + with {:ok, dest} <- ImageStorage.cp_file(prepared.path, user_id, prepared.filename) do + {:ok, %{dest: dest, filename: prepared.filename}} + end + after + if prepared.cleanup? do + _ = File.rm(prepared.path) + end + end + rescue + error -> {:error, error} + end + + defp prepare_for_storage!(src, filename) do + if tiff_upload?(src, filename) do + png_path = temp_png_path() + ImageMagick.convert_to_png!(src, png_path) + + %{path: png_path, filename: png_filename(filename), cleanup?: true} + else + %{path: src, filename: filename, cleanup?: false} + end + end + + defp temp_png_path do + Path.join(System.tmp_dir!(), "vmemo-upload-#{System.unique_integer([:positive])}.png") + end + + defp png_filename(filename) do + filename + |> to_string() + |> Path.rootname() + |> Kernel.<>(".png") + end + + defp tiff_upload?(src, filename) do + tiff_extension?(filename) or tiff_header?(src) + end + + defp tiff_extension?(filename) do + filename + |> to_string() + |> Path.extname() + |> String.downcase() + |> then(&(&1 in [".tif", ".tiff"])) + end + + defp tiff_header?(path) when is_binary(path) do + case File.open(path, [:read, :binary]) do + {:ok, io} -> + try do + case :file.read(io, 4) do + {:ok, <<"II", marker, 0>>} when marker in [42, 43] -> true + {:ok, <<"MM", 0, marker>>} when marker in [42, 43] -> true + _ -> false + end + after + File.close(io) + end + + {:error, _reason} -> + false + end + end + + defp tiff_header?(_path), do: false +end diff --git a/lib/vmemo_web/api/v1/image_controller.ex b/lib/vmemo_web/api/v1/image_controller.ex index e1f28298..2dbea4c6 100644 --- a/lib/vmemo_web/api/v1/image_controller.ex +++ b/lib/vmemo_web/api/v1/image_controller.ex @@ -9,7 +9,7 @@ defmodule VmemoWeb.Api.V1.ImageController do alias Plug.Conn.Status alias Vmemo.Memo.Image - alias Vmemo.Memo.ImageStorage + alias Vmemo.Memo.ImageUpload require Logger @@ -496,13 +496,13 @@ defmodule VmemoWeb.Api.V1.ImageController do user_id = to_string(current_user.id) try do - {:ok, dest} = ImageStorage.cp_file(path, user_id, filename) + {:ok, %{dest: dest, filename: stored_filename}} = ImageUpload.store(path, user_id, filename) note = Map.get(params, "note", "") attrs = %{ note: note, url: Path.join("/", dest), - file_id: filename, + file_id: stored_filename, user_id: user_id, inner_purpose: nil } diff --git a/lib/vmemo_web/live/components/search_box.ex b/lib/vmemo_web/live/components/search_box.ex index ef84f9bd..992e6cfe 100644 --- a/lib/vmemo_web/live/components/search_box.ex +++ b/lib/vmemo_web/live/components/search_box.ex @@ -4,7 +4,7 @@ defmodule VmemoWeb.LiveComponents.SearchBox do use Gettext, backend: VmemoWeb.Gettext alias Vmemo.Memo.Image - alias Vmemo.Memo.ImageStorage + alias Vmemo.Memo.ImageUpload @impl true def mount(socket) do @@ -158,13 +158,14 @@ defmodule VmemoWeb.LiveComponents.SearchBox do consume_uploaded_entry(socket, entry, fn %{path: path} -> filename = entry.uuid <> Path.extname(entry.client_name) - with {:ok, dest} <- ImageStorage.cp_file(path, current_user.id, filename), + with {:ok, %{dest: dest, filename: stored_filename}} <- + ImageUpload.store(path, current_user.id, filename), {:ok, image} <- Image.create_with_sync( %{ note: "", url: Path.join("/", dest), - file_id: filename, + file_id: stored_filename, user_id: current_user.id, upload_batch_id: Ecto.UUID.generate(), inner_purpose: nil @@ -246,13 +247,14 @@ defmodule VmemoWeb.LiveComponents.SearchBox do consume_uploaded_entry(socket, entry, fn %{path: path} -> filename = entry.uuid <> Path.extname(entry.client_name) - with {:ok, dest} <- ImageStorage.cp_file(path, current_user.id, filename), + with {:ok, %{dest: dest, filename: stored_filename}} <- + ImageUpload.store(path, current_user.id, filename), {:ok, image} <- Image.create_with_sync( %{ note: "", url: Path.join("/", dest), - file_id: filename, + file_id: stored_filename, user_id: current_user.id, upload_batch_id: Ecto.UUID.generate(), inner_purpose: nil diff --git a/lib/vmemo_web/live/components/upload_form.ex b/lib/vmemo_web/live/components/upload_form.ex index 7b6d92c9..96e7746f 100644 --- a/lib/vmemo_web/live/components/upload_form.ex +++ b/lib/vmemo_web/live/components/upload_form.ex @@ -10,8 +10,8 @@ defmodule VmemoWeb.LiveComponents.UploadForm do alias VmemoWeb.LiveComponents.Waterfall alias Vmemo.Memo.Image + alias Vmemo.Memo.ImageUpload alias Vmemo.Memo.ImageNote - alias Vmemo.Memo.ImageStorage alias Vmemo.Memo.Note @impl true @@ -373,13 +373,14 @@ defmodule VmemoWeb.LiveComponents.UploadForm do user_id = current_user.id filename = entry.uuid <> Path.extname(entry.client_name) - with {:ok, dest} <- ImageStorage.cp_file(path, user_id, filename), + with {:ok, %{dest: dest, filename: stored_filename}} <- + ImageUpload.store(path, user_id, filename), {:ok, image} <- Image.create_with_sync( %{ note: note_text, url: Path.join("/", dest), - file_id: filename, + file_id: stored_filename, user_id: user_id, upload_batch_id: upload_batch_id, inner_purpose: nil diff --git a/test/vmemo/memo/image_storage_test.exs b/test/vmemo/memo/image_storage_test.exs index 1866e7ab..a8d442d6 100644 --- a/test/vmemo/memo/image_storage_test.exs +++ b/test/vmemo/memo/image_storage_test.exs @@ -1,6 +1,7 @@ defmodule Vmemo.Memo.ImageStorageTest do use ExUnit.Case, async: true + alias Vmemo.Memo.ImageUpload alias Vmemo.Memo.ImageStorage alias Vmemo.Storage @@ -46,7 +47,7 @@ defmodule Vmemo.Memo.ImageStorageTest do ImageStorage.storage_path_from_url("https://cdn.example.com/demo.jpg", user_id) end - test "cp_file/3 converts tiff uploads to png for browser display" do + test "cp_file/3 copies files without converting image format" do user_id = "u-#{System.unique_integer([:positive])}" src = Path.join(System.tmp_dir!(), "vmemo-test-#{System.unique_integer([:positive])}.tiff") File.write!(src, tiff_binary()) @@ -57,6 +58,24 @@ defmodule Vmemo.Memo.ImageStorageTest do end) assert {:ok, dest} = ImageStorage.cp_file(src, user_id, "clipboard.tiff") + assert Path.extname(dest) == ".tiff" + assert File.read!(dest) == tiff_binary() + end + + test "ImageUpload.store/3 converts tiff uploads to png for browser display" do + user_id = "u-#{System.unique_integer([:positive])}" + src = Path.join(System.tmp_dir!(), "vmemo-test-#{System.unique_integer([:positive])}.tiff") + File.write!(src, tiff_binary()) + + on_exit(fn -> + File.rm(src) + File.rm_rf!(Path.join([@storage_prefix, user_id])) + end) + + assert {:ok, %{dest: dest, filename: filename}} = + ImageUpload.store(src, user_id, "clipboard.tiff") + + assert filename == "clipboard.png" assert Path.extname(dest) == ".png" assert <<0x89, 0x50, 0x4E, 0x47, _::binary>> = File.read!(dest) end