Images uploaded with wrong extension trigger ActiveStorage::IntegrityError

未关闭
#597 1 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看

还没有人认领这个 Issue。

评估

难度
4/5
预计耗时
3-5 天
新手友好度
55/100
Issue 类型
缺陷
描述清晰度
基本清楚
活跃度
活跃
技术栈
rails, ruby
领域
backend, cloud

调研方向

从 ActiveStorage::Service::CloudinaryService#ext_for_file 开始,将其扩展名选择与报告中的 monkey patch 进行比较。使用一个文件名带有 .png 扩展名的 JPEG 重现一次 Rails Direct Upload,然后跟踪 Cloudinary URL、blob checksum 以及 ActiveStorage::IntegrityError 路径。完成标准是:原始上传的图像可以在没有 checksum 不匹配或意外格式转换的情况下下载并分析。

由索引模型根据 Issue 内容生成。

描述

Describe the bug in a sentence or two.

When using Rails Direct Uploads, we can't analyze a file before it reaches Cloudinary. If a user uploads an image with the wrong extension (say a JPEG named image.png), then Cloudinary will return an URL ending with .png, which requests a transformation of the image to PNG, resulting in a checksum that does not match the one stored in the associated ActiveStorage::Blob, which triggers ActiveStorage::IntegrityError.

Issue Type (Can be multiple)

  • Build - Cannot install or import the SDK
  • Performance - Performance issues
  • Behaviour - Functions are not working as expected (such as generate URL)
  • Documentation - Inconsistency between the docs and behaviour
  • Other (Specify)

Operating System

  • Linux
  • Windows
  • macOS
  • All

Environment and Libraries (fill in the version numbers)

  • Cloudinary Ruby SDK version - 2.4.4
  • Ruby Version - 3.4.10
  • Rails Version - 8.1.3

Current workaround

In our codebase, we monkey patched ActiveStorage::Service::CloudinaryService#ext_for_file like this as a workaround:

# The original ext_for_file method takes the extension from the filename and only falls back to the
# content type when the filename carries none. What this patch does is let Rails determine the real
# content type of the file, and if it matches the filename extension return this extension, otherwise
# return an extension that really matches the content_type.
#
# This logic does not apply to "raw" resource types, as these are never transcoded by cloudinary.
def ext_for_file(key, filename = nil, content_type = nil)
  attributes = key.is_a?(ActiveStorage::BlobKey) ? key.attributes : {}
  content_type = content_type.presence || attributes[:content_type]
  return super if content_type_to_resource_type(content_type).eql?('raw')

  # { 'identified' => true } in metadata means Rails has analyzed the file and determined its true
  # content type. It does that by downloading the file from the provided url, at the moment of
  # attaching the blob to the record. Before that, the content type is what the client said it was,
  # which can be wrong. Returning nil before identification forces no extension and therefore no
  # conversion on the Cloudinary side. That allows Rails to download and analyze the original file
  # intead of a converted one.
  return nil unless attributes.dig(:metadata, 'identified')

  filename = ActiveStorage::Filename.wrap(filename.presence || attributes[:filename])
  filename_extension = filename.extension_without_delimiter
  content_type_extensions = Marcel::TYPE_EXTS[content_type]
  return filename_extension if content_type_extensions.blank?

  content_type_extensions.find { it == filename_extension } || content_type_extensions.first
end
主要语言
Ruby
星标
420
派生
282
PR 合并指标
30 天内没有已合并 PR

环境准备

从这里开始

  1. 先读完整个 Issue,再读项目的贡献指南。
  2. 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
  3. Fork 仓库,在一个分支上完成修改。
  4. 提交 Pull Request,并在描述里引用这个 Issue 编号。

cloudinary/cloudinary_gem 的其他 Issue

查看 cloudinary/cloudinary_gem 的全部 Issue

相似的 Issue

更多 Ruby Issue

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。