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

isFaultyImage returns false positive response when file.filepath is empty #20309

Open
dekhanov opened this issue May 14, 2024 · 2 comments
Open
Assignees
Labels
issue: bug Issue reporting a bug severity: low If the issue only affects a very niche base of users and an easily implemented workaround can solve source: core:upload Source is core/upload package status: pending reproduction Waiting for free time to reproduce the issue, or more information

Comments

@dekhanov
Copy link

Bug report

Required System information

  • Node.js version: 20.11
  • NPM version: 10.3.0
  • Strapi version: 4.23.1
  • Operating system: PostgreSQL
  • Is your project Javascript or Typescript: TS

Describe the bug

I believe there is a bug in the isFaultyImage function implementation on this line. Now, when the stats function returns correct result it will be passed to the resolve callback and will make the result of isFaultyImage positive, which is not the correct result.

The correct implementation should be something like this: pipeline.stats().then(() => resolve(false)).catch(() => resolve(true));

@joshuaellis
Copy link
Member

Can you please feel out reproduction steps.

@dekhanov
Copy link
Author

To reproduce the bug you can extend @strapi/plugin-upload. The bug is in the version 4.24.2 of the plugin.

src/extensions/upload/overrides.ts

/**
 * Overriding enhanceAndValidateFile method to validate available file extensions
 * node_modules/@strapi/plugin-upload/server/services/upload.js
 */
import { errors } from '@strapi/utils';
import fs from 'fs';

const { ApplicationError } = errors;
const allowedExtensions = [
  '.jpg',
  '.jpeg',
  '.png',
  '.webp',
  '.svg',
  '.pdf',
  '.mp4',
];

const isFileExtensionAllowed = (fileExtension) => {
  return allowedExtensions.includes(fileExtension);
};

export async function enhanceAndValidateFile(file, fileInfo = {}, metas = {}) {
  // @ts-expect-error this will be available on override
  const currentFile = await this.formatFileInfo(
    {
      filename: file.name,
      type: file.type,
      size: file.size,
    },
    fileInfo,
    {
      ...metas,
      tmpWorkingDirectory: file.tmpWorkingDirectory,
    }
  );
  if (!isFileExtensionAllowed(currentFile?.ext)) {
    throw new ApplicationError(
      `File type is not allowed. Allowed extentions: ${allowedExtensions.join(
        ','
      )}`
    );
  }

  currentFile.getStream = () => fs.createReadStream(file.path);

  const { optimize, isImage, isFaultyImage, isOptimizableImage } = strapi
    .plugin('upload')
    .service('image-manipulation');

  if (await isImage(currentFile)) {
    if (await isFaultyImage(currentFile)) {
      throw new ApplicationError('File is not a valid image');
    }
    if (await isOptimizableImage(currentFile)) {
      return optimize(currentFile);
    }
  }
  return currentFile;
}

src/index.ts

import { enhanceAndValidateFile } from './extensions/upload/overrides';

export default {
  register({ strapi }) {
    /**
     * Overriding enhanceAndValidateFile function to validate available file extensions
     */
    strapi.plugin('upload').service('upload').enhanceAndValidateFile =
      enhanceAndValidateFile;
  },
};

With this configuration, I get an error `throw new ApplicationError('File is not a valid image') ' when trying to upload an image to the media library. ' Everything worked with the plugin's 4.24.1 version.

@joshuaellis joshuaellis added issue: bug Issue reporting a bug severity: low If the issue only affects a very niche base of users and an easily implemented workaround can solve source: core:upload Source is core/upload package status: pending reproduction Waiting for free time to reproduce the issue, or more information labels May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
issue: bug Issue reporting a bug severity: low If the issue only affects a very niche base of users and an easily implemented workaround can solve source: core:upload Source is core/upload package status: pending reproduction Waiting for free time to reproduce the issue, or more information
Projects
Status: To be reviewed
Status: To review
Development

No branches or pull requests

3 participants