Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No content-type with multerS3.AUTO_CONTENT_TYPE and SVG #126

Closed
JustDoItSascha opened this issue Nov 13, 2019 · 4 comments
Closed

No content-type with multerS3.AUTO_CONTENT_TYPE and SVG #126

JustDoItSascha opened this issue Nov 13, 2019 · 4 comments

Comments

@JustDoItSascha
Copy link

Hey!

I got an error that was hard to debug, but I think I found the issue:

In your multerS3.AUTO_CONTENT_TYPE function, you do

file.stream.once('data' ...)

On production servers the stream is not completed when the once('data') is firing. Later on you put the firstChunk in the function isSvg(). But this function needs to have the whole content, because it's checking whether the SVG is properly closed. Since you give it just a part of the SVG, the check always fails and SVG images get the wrong content-type when uploading to S3.

Should I open a pull request for this?

JustDoItSascha added a commit to JustDoItSascha/multer-s3 that referenced this issue Nov 13, 2019
@lehni
Copy link

lehni commented Jun 28, 2021

I am seeing this problem too. The issue is that whenever firstChunk isn't actually the full SVG, is-svg returns false, since it's using an XML parser to look at the file. This happens for us with SVG files above a certain size (100kb or so).

@lehni
Copy link

lehni commented Jun 28, 2021

I ended up using a slightly different strategy than in JustDoItSascha@9e4673b

I trust file.mimetype if provided, and fall back on trying to read the type from the first chunk. Only if that fails, the code keeps collecting chunks unitl the end and then runs the detection on the full data.

const contentType = (req, file, cb) => {
  const { mimetype, stream } = file
  if (mimetype) {
    // 1. Trust file.mimetype if provided.
    cb(null, mimetype)
  } else {
    let data = null

    const done = type => {
      const outStream = new PassThrough()
      outStream.write(data)
      stream.pipe(outStream)
      cb(null, type, outStream)
    }

    const onData = chunk => {
      if (!data) {
        // 2. Try reading the mimetype from the first chunk.
        const type = FileType.fromBuffer(chunk)?.mime
        if (type) {
          stream.off('data', onData)
          done(type)
        } else {
          // 3. If that fails, keep collecting all chunks and determine
          //    the mimetype using the full data.
          stream.once('end', () => {
            const type = (
              FileType.fromBuffer(data)?.mime ||
              (isSvg(data) ? 'image/svg+xml' : 'application/octet-stream')
            )
            done(type)
          })
        }
      }
      data = data ? Buffer.concat([data, chunk]) : chunk
    }

    stream.on('data', onData)
  }
}

@LinusU
Copy link
Collaborator

LinusU commented Oct 14, 2021

Should be fixed in 2.9.1

@danny-does-stuff
Copy link

Fix is in this PR #103

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants