feat: validate uploaded image files (#3797)

* chore: upgrade react-dropzone to latest * feat: add basic image validation checks * feat: remove unused image input properties * feat: add compression to image input component * chore: remove unused compressor plugin from file input component * fix: remove on img clicked property from image converter list * test: add feature test --------- Co-authored-by: Ben Furber <[email protected]>
ONEARMY · Sep 2, 2024 · e0a1ba8 · e0a1ba8
1 parent ab4b347
commit e0a1ba8
Show file tree

Hide file tree

Showing 10 changed files with 161 additions and 46 deletions.
diff --git a/package.json b/package.json
@@ -86,6 +86,7 @@
     "@uppy/file-input": "^3.1.2",
     "@uppy/progress-bar": "^3.1.1",
     "@uppy/react": "^3.3.1",
+    "compressorjs": "^1.2.1",
     "countries-list": "^2.6.1",
     "date-fns": "^3.3.0",
     "debounce": "^1.2.0",
@@ -108,7 +109,7 @@
     "react": "18.3.1",
     "react-country-flag": "^3.1.0",
     "react-dom": "18.3.1",
-    "react-dropzone": "^10.1.10",
+    "react-dropzone": "^14.2.3",
     "react-final-form": "6.5.3",
     "react-final-form-arrays": "^3.1.3",
     "react-foco": "^1.3.1",

diff --git a/packages/cypress/src/fixtures/images/file.random b/packages/cypress/src/fixtures/images/file.random
diff --git a/packages/cypress/src/integration/settings.spec.ts b/packages/cypress/src/integration/settings.spec.ts
@@ -43,7 +43,7 @@ describe('[Settings]', () => {
       cy.get('[data-cy="Confirm.modal: Modal"]').should('be.visible')
     })
 
-    it('[Edit a new profile]', () => {
+    it.only('[Edit a new profile]', () => {
       const country = 'Brazil'
       const userImage = 'avatar'
       const displayName = 'settings_member_new'
@@ -79,8 +79,17 @@ describe('[Settings]', () => {
         description,
       })
 
-      cy.step('Can add avatar only')
+      cy.step('Errors if trying to upload invalid image')
+      cy.get(`[data-cy=userImage]`)
+        .find(':file')
+        .attachFile(`images/file.random`)
+      cy.get('[data-cy=ImageUploadError]').should('be.visible')
+      cy.get('[data-cy=ImageUploadError-Button]').click()
+
+      cy.step('Can add avatar')
       cy.setSettingImage(userImage, 'userImage')
+
+      cy.step("Can't add cover image")
       cy.get('[data-cy=coverImages]').should('not.exist')
 
       cy.setSettingAddContactLink({

diff --git a/src/common/Form/FileInput/FileInput.tsx b/src/common/Form/FileInput/FileInput.tsx
@@ -1,6 +1,5 @@
 import * as React from 'react'
 import { useEffect, useState } from 'react'
-import Compressor from '@uppy/compressor'
 import Uppy from '@uppy/core'
 import { DashboardModal } from '@uppy/react'
 import { Button, DownloadStaticFile } from 'oa-components'
@@ -25,10 +24,8 @@ interface IState {
 }
 export const FileInput = (props: IProps) => {
   const [state, setState] = useState<IState>({ open: false })
-  const [uppy] = useState(() =>
-    new Uppy({ ...UPPY_CONFIG, onBeforeUpload: () => uploadTriggered() }).use(
-      Compressor,
-    ),
+  const [uppy] = useState(
+    () => new Uppy({ ...UPPY_CONFIG, onBeforeUpload: () => uploadTriggered() }),
   )
 
   useEffect(() => {

diff --git a/src/common/Form/ImageInput/ImageConverter.tsx b/src/common/Form/ImageInput/ImageConverter.tsx
@@ -6,11 +6,9 @@ import type { IConvertedFileMeta } from 'src/types'
 interface IProps {
   file: File
   onImgConverted: (meta: IConvertedFileMeta) => void
-  onImgClicked: (meta: IConvertedFileMeta) => void
 }
 interface IState {
   convertedFile?: IConvertedFileMeta
-  openLightbox?: boolean
 }
 
 const _generateFileMeta = (c: File) => {
@@ -67,7 +65,6 @@ export const ImageConverter = (props: IProps) => {
         borderRadius: 1,
       }}
       id="preview"
-      onClick={() => props.onImgClicked(convertedFile)}
     />
   )
 }

diff --git a/src/common/Form/ImageInput/ImageConverterList.tsx b/src/common/Form/ImageInput/ImageConverterList.tsx
@@ -16,7 +16,6 @@ export const ImageConverterList = (props: IProps) => {
             key={file.name}
             file={file}
             onImgConverted={(meta) => handleConvertedFileChange(meta, index)}
-            onImgClicked={() => null}
           />
         )
       })}

diff --git a/src/common/Form/ImageInput/ImageInput.tsx b/src/common/Form/ImageInput/ImageInput.tsx
@@ -1,56 +1,73 @@
 import { useEffect, useRef, useState } from 'react'
 import Dropzone from 'react-dropzone'
-import { Button } from 'oa-components'
-import { Box, Image } from 'theme-ui'
+import { Button, Modal } from 'oa-components'
+import { logger } from 'src/logger'
+import { Box, Flex, Image, Text } from 'theme-ui'
 
+import { compressImage } from './compressImage'
 import { DeleteImage } from './DeleteImage'
 import { getPresentFiles } from './getPresentFiles'
 import { ImageConverterList } from './ImageConverterList'
 import { ImageInputWrapper } from './ImageInputWrapper'
+import { imageValid } from './imageValid'
 import { setSrc } from './setSrc'
 
 import type { IConvertedFileMeta } from 'src/types'
 import type { ThemeUIStyleObject } from 'theme-ui'
 import type { IInputValue, IMultipleInputValue, IValue } from './types'
 
-/*
-    This component takes multiple image using filepicker and resized clientside
-    Note, typings not available for client-compress so find full options here:
-    https://github.com/davejm/client-compress
-*/
 type IFileMeta = IConvertedFileMeta[] | IConvertedFileMeta | null
 
 interface IProps {
   onFilesChange: (fileMeta: IFileMeta) => void
   imageDisplaySx?: ThemeUIStyleObject | undefined
   value?: IValue
   hasText?: boolean
-  multiple?: boolean
   dataTestId?: string
 }
 
 export const ImageInput = (props: IProps) => {
   const fileInputRef = useRef<HTMLInputElement>(null)
   const prevPropsValue = useRef<IInputValue | IMultipleInputValue>()
 
-  const { dataTestId, imageDisplaySx, multiple, onFilesChange, value } = props
+  const { dataTestId, imageDisplaySx, onFilesChange, value } = props
+
   const [inputFiles, setInputFiles] = useState<File[]>([])
   const [convertedFiles, setConvertedFiles] = useState<IConvertedFileMeta[]>([])
   const [presentFiles, setPresentFiles] = useState<IMultipleInputValue>(
     getPresentFiles(value),
   )
-
-  const onDrop = (inputFiles) => {
-    setInputFiles(inputFiles)
+  const [isImageCorrupt, setIsImageCorrupt] = useState(false)
+  const [showErrorModal, setShowErrorModal] = useState(false)
+
+  const onDrop = async (selectedImage) => {
+    try {
+      await imageValid(selectedImage[0])
+      setIsImageCorrupt(false)
+
+      try {
+        const compressedImage = await compressImage(selectedImage[0])
+        selectedImage[0] = compressedImage
+      } catch (compressionError) {
+        logger.error(
+          'Image compression failed, using original image: ',
+          compressionError,
+        )
+      }
+
+      setInputFiles(selectedImage)
+    } catch (validationError) {
+      setIsImageCorrupt(true)
+      setShowErrorModal(true)
+    }
   }
 
   const handleConvertedFileChange = (newFile: IConvertedFileMeta, index) => {
     const nextFiles = convertedFiles
     nextFiles[index] = newFile
     setConvertedFiles(convertedFiles)
 
-    const value = props.multiple ? convertedFiles : convertedFiles[0]
-    props.onFilesChange(value)
+    props.onFilesChange(convertedFiles[0])
   }
 
   const handleImageDelete = (event: Event) => {
@@ -76,13 +93,18 @@ export const ImageInput = (props: IProps) => {
 
   return (
     <Box p={0} sx={imageDisplaySx ? imageDisplaySx : { height: '100%' }}>
-      <Dropzone accept="image/*" multiple={multiple} onDrop={onDrop}>
+      <Dropzone
+        accept={{
+          'image/*': ['.jpeg', '.jpg', '.png', '.gif', '.svg', '.webp'],
+        }}
+        multiple={false}
+        onDrop={onDrop}
+      >
         {({ getRootProps, getInputProps, rootRef }) => (
           <ImageInputWrapper
+            {...getRootProps()}
             ref={rootRef}
             hasUploadedImg={showUploadedImg}
-            sx={{ width: '100%', height: '100%' }}
-            {...getRootProps()}
           >
             <input
               ref={fileInputRef}
@@ -98,19 +120,50 @@ export const ImageInput = (props: IProps) => {
                 handleConvertedFileChange={handleConvertedFileChange}
               />
             )}
-
             {!hasImages && (
               <Button small variant="outline" icon="image" type="button">
                 Upload
               </Button>
             )}
-
             {hasImages && (
               <DeleteImage onClick={(event) => handleImageDelete(event)} />
             )}
           </ImageInputWrapper>
         )}
       </Dropzone>
+      <Modal
+        width={600}
+        isOpen={showErrorModal}
+        onDidDismiss={() => setShowErrorModal(false)}
+      >
+        {isImageCorrupt && (
+          <Flex
+            data-cy="ImageUploadError"
+            mt={[1, 1, 1]}
+            sx={{
+              flexDirection: 'column',
+              justifyContent: 'space-between',
+              gap: '20px',
+            }}
+          >
+            <Text>
+              The uploaded image appears to be corrupted or a type we don't
+              accept.
+            </Text>
+            <Text>
+              Check your image is valid and one of the following formats: jpeg,
+              jpg, png, gif, heic, svg or webp.
+            </Text>
+            <Button
+              data-cy="ImageUploadError-Button"
+              sx={{ marginTop: '20px', justifyContent: 'center' }}
+              onClick={() => setShowErrorModal(false)}
+            >
+              Try uploading something else
+            </Button>
+          </Flex>
+        )}
+      </Modal>
     </Box>
   )
 }
diff --git a/src/common/Form/ImageInput/compressImage.ts b/src/common/Form/ImageInput/compressImage.ts
@@ -0,0 +1,20 @@
+import Compressor from 'compressorjs'
+
+export const compressImage = (image: File): Promise<File> => {
+  return new Promise((resolve, reject) => {
+    if (!image) {
+      reject()
+      return
+    }
+
+    new Compressor(image, {
+      quality: 0.6, // 0 to 1
+      success: (compressed) => {
+        resolve(compressed as File)
+      },
+      error: (err) => {
+        reject(err)
+      },
+    })
+  })
+}
diff --git a/src/common/Form/ImageInput/imageValid.ts b/src/common/Form/ImageInput/imageValid.ts
@@ -0,0 +1,38 @@
+// Basic check using the filereader api to see whether we can create a displayable image
+// If this fails then there is a problem with the file
+
+export const imageValid = (file: File): Promise<void> => {
+  return new Promise((resolve, reject) => {
+    if (!file) {
+      reject()
+      return
+    }
+
+    const reader = new FileReader()
+
+    reader.onload = (e: ProgressEvent<FileReader>) => {
+      const img = document.createElement('img')
+
+      img.onload = () => {
+        // Image loaded successfully
+        img.remove()
+        resolve()
+      }
+
+      img.onerror = () => {
+        // Image failed to load (possibly corrupted)
+        img.remove()
+        reject()
+      }
+
+      img.src = e.target?.result as string
+    }
+
+    reader.onerror = () => {
+      // Error reading file. It might be corrupted.
+      reject()
+    }
+
+    reader.readAsDataURL(file)
+  })
+}
diff --git a/yarn.lock b/yarn.lock
@@ -10979,7 +10979,7 @@ __metadata:
   languageName: node
   linkType: hard
 
-"attr-accept@npm:^2.0.0":
+"attr-accept@npm:^2.2.2":
   version: 2.2.2
   resolution: "attr-accept@npm:2.2.2"
   checksum: 496f7249354ab53e522510c1dc8f67a1887382187adde4dc205507d2f014836a247073b05e9d9ea51e2e9c7f71b0d2aa21730af80efa9af2d68303e5f0565c4d
@@ -16002,12 +16002,12 @@ __metadata:
   languageName: node
   linkType: hard
 
-"file-selector@npm:^0.1.12":
-  version: 0.1.19
-  resolution: "file-selector@npm:0.1.19"
+"file-selector@npm:^0.6.0":
+  version: 0.6.0
+  resolution: "file-selector@npm:0.6.0"
   dependencies:
-    tslib: ^2.0.1
-  checksum: 5b105a3ede9139729ada72d6653ae3f4387a7bf2585e8700f9fa53f22457d1f88304fdde9ad7b43b694a5610d67058302257f448a75248fc2225880bca6df5df
+    tslib: ^2.4.0
+  checksum: 7d051b6e5d793f3c6e2ab287ba5e7c2c6a0971bccc9d56e044c8047ba483e18f60fc0b5771c951dc707c0d15f4f36ccb4f1f1aaf385d21ec8f7700dadf8325ba
   languageName: node
   linkType: hard
 
@@ -22831,6 +22831,7 @@ __metadata:
     all-contributors-cli: ^6.20.0
     chai-subset: ^1.6.0
     commitizen: ^4.2.4
+    compressorjs: ^1.2.1
     concurrently: ^6.2.0
     countries-list: ^2.6.1
     cross-env: ^7.0.3
@@ -22875,7 +22876,7 @@ __metadata:
     react-country-flag: ^3.1.0
     react-dev-utils: ^12.0.1
     react-dom: 18.3.1
-    react-dropzone: ^10.1.10
+    react-dropzone: ^14.2.3
     react-final-form: 6.5.3
     react-final-form-arrays: ^3.1.3
     react-foco: ^1.3.1
@@ -24804,16 +24805,16 @@ __metadata:
   languageName: node
   linkType: hard
 
-"react-dropzone@npm:^10.1.10":
-  version: 10.2.2
-  resolution: "react-dropzone@npm:10.2.2"
+"react-dropzone@npm:^14.2.3":
+  version: 14.2.3
+  resolution: "react-dropzone@npm:14.2.3"
   dependencies:
-    attr-accept: ^2.0.0
-    file-selector: ^0.1.12
-    prop-types: ^15.7.2
+    attr-accept: ^2.2.2
+    file-selector: ^0.6.0
+    prop-types: ^15.8.1
   peerDependencies:
-    react: ">= 16.8"
-  checksum: af08b78db753dd9c277c64364c153d6cb6d563df3cc4db1731458edcc203cfa41ed3d032c5249822e5eb4d2534232e514d663a025962d08f0e6ea65550411116
+    react: ">= 16.8 || 18.0.0"
+  checksum: 174b744d5ca898cf3d84ec1aeb6cef5211c446697e45dc8ece8287a03d291f8d07253206d5a1247ef156fd385d65e7de666d4d5c2986020b8543b8f2434e8b40
   languageName: node
   linkType: hard