From 9232a5df318e7c883153a21d07987b1e474c79e6 Mon Sep 17 00:00:00 2001 From: Laura Beatris <48022589+LauraBeatris@users.noreply.github.com> Date: Thu, 18 Feb 2021 18:03:25 -0300 Subject: [PATCH 1/9] fix: do not allow to upload files with invalid formats --- src/__tests__/upload.js | 29 +++++++++++++++++++++++++++++ src/upload.js | 26 +++++++++++++++++++++----- 2 files changed, 50 insertions(+), 5 deletions(-) diff --git a/src/__tests__/upload.js b/src/__tests__/upload.js index f09a6ae2..bb6a64ee 100644 --- a/src/__tests__/upload.js +++ b/src/__tests__/upload.js @@ -163,3 +163,32 @@ test('should call onChange/input bubbling up the event when a file is selected', expect(onInputInput).toHaveBeenCalledTimes(1) expect(onInputForm).toHaveBeenCalledTimes(1) }) + +test('should not upload file with invalid unaccepted format', () => { + const file = new File(['hello'], 'hello.png', {type: 'image/png'}) + + const {element} = setup('') + + userEvent.upload(element, file) + + expect(element.files[0]).toBeUndefined() + expect(element.files.item(0)).toBeNull() + expect(element.files).toHaveLength(0) +}) + +test('should not upload multiple files with invalid unaccepted formats', () => { + const files = [ + new File(['hello'], 'hello.txt', {type: 'text/plain'}), + new File(['there'], 'there.pdf', {type: 'application/pdf'}), + ] + + const {element} = setup(` + + `) + + userEvent.upload(element, files) + + expect(element.files[0]).toBeUndefined() + expect(element.files.item(0)).toBeNull() + expect(element.files).toHaveLength(0) +}) diff --git a/src/upload.js b/src/upload.js index a252b438..db146407 100644 --- a/src/upload.js +++ b/src/upload.js @@ -4,16 +4,32 @@ import {blur} from './blur' import {focus} from './focus' function upload(element, fileOrFiles, init) { - if (element.disabled) return + const hasFileWithInvalidType = + !Array.isArray(fileOrFiles) && + element.accept && + !element.accept.includes(element.type) + + if (hasFileWithInvalidType || element.disabled) return click(element, init) const input = element.tagName === 'LABEL' ? element.control : element - const files = (Array.isArray(fileOrFiles) - ? fileOrFiles - : [fileOrFiles] - ).slice(0, input.multiple ? undefined : 1) + let files = [] + + if (Array.isArray(fileOrFiles)) { + files = element.accept + ? fileOrFiles.filter(file => element.accept.includes(file.type)) + : fileOrFiles + } else { + files = [fileOrFiles] + } + + const hasFilesWithInvalidTypes = files.length === 0 + + if (hasFilesWithInvalidTypes) return + + files = files.slice(0, input.multiple ? undefined : 1) // blur fires when the file selector pops up blur(element, init) From 831e922794f055c59b892a4d6e5a674e5a701482 Mon Sep 17 00:00:00 2001 From: Laura Beatris <48022589+LauraBeatris@users.noreply.github.com> Date: Thu, 18 Feb 2021 18:29:32 -0300 Subject: [PATCH 2/9] fix(upload): verify if accept attribute includes file type --- src/upload.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/upload.js b/src/upload.js index db146407..a0972f27 100644 --- a/src/upload.js +++ b/src/upload.js @@ -6,8 +6,8 @@ import {focus} from './focus' function upload(element, fileOrFiles, init) { const hasFileWithInvalidType = !Array.isArray(fileOrFiles) && - element.accept && - !element.accept.includes(element.type) + Boolean(element.accept) && + !element.accept.includes(fileOrFiles.type) if (hasFileWithInvalidType || element.disabled) return From a7c715a9f07800b6ac6b29fbe386a79f988b8e27 Mon Sep 17 00:00:00 2001 From: Laura Beatris <48022589+LauraBeatris@users.noreply.github.com> Date: Thu, 18 Feb 2021 19:55:46 -0300 Subject: [PATCH 3/9] perf(upload): handle higher range of mime types --- src/__tests__/upload.js | 42 ++++++++++++++++++++++++++++++++--------- src/upload.js | 18 +++++++++++++++--- 2 files changed, 48 insertions(+), 12 deletions(-) diff --git a/src/__tests__/upload.js b/src/__tests__/upload.js index bb6a64ee..f8879e1d 100644 --- a/src/__tests__/upload.js +++ b/src/__tests__/upload.js @@ -164,31 +164,55 @@ test('should call onChange/input bubbling up the event when a file is selected', expect(onInputForm).toHaveBeenCalledTimes(1) }) -test('should not upload file with invalid unaccepted format', () => { +test('should upload file with accepted format', () => { const file = new File(['hello'], 'hello.png', {type: 'image/png'}) + const {element} = setup('') + userEvent.upload(element, file) + + expect(element.files).toHaveLength(1) +}) + +test('should upload multiple files with accepted format', () => { + const files = [ + new File(['hello'], 'hello.png', {type: 'image/png'}), + new File(['there'], 'there.jpg', {type: 'audio/mp3'}), + new File(['there'], 'there.csv', {type: 'text/csv'}), + new File(['there'], 'there.jpg', {type: 'video/mp4'}), + ] + const {element} = setup(` + + `) + + userEvent.upload(element, files) + + expect(element.files).toHaveLength(3) +}) + +test('should not upload file with unaccepted format', () => { + const file = new File(['hello'], 'hello.png', {type: 'image/png'}) const {element} = setup('') userEvent.upload(element, file) - expect(element.files[0]).toBeUndefined() - expect(element.files.item(0)).toBeNull() expect(element.files).toHaveLength(0) }) -test('should not upload multiple files with invalid unaccepted formats', () => { +test('should not upload multiple files with unaccepted formats', () => { const files = [ new File(['hello'], 'hello.txt', {type: 'text/plain'}), new File(['there'], 'there.pdf', {type: 'application/pdf'}), + new File(['there'], 'there.png', {type: 'image/png'}), + new File(['there'], 'there.mp4', {type: 'video/mp4'}), ] - const {element} = setup(` - + `) userEvent.upload(element, files) - expect(element.files[0]).toBeUndefined() - expect(element.files.item(0)).toBeNull() - expect(element.files).toHaveLength(0) + expect(element.files).toHaveLength(1) }) diff --git a/src/upload.js b/src/upload.js index a0972f27..740743a8 100644 --- a/src/upload.js +++ b/src/upload.js @@ -3,11 +3,24 @@ import {click} from './click' import {blur} from './blur' import {focus} from './focus' +const isValidFileType = (acceptAttribute, type) => { + const acceptManyExtensions = /(video|audio|image)\/?\*/.test(acceptAttribute) + + if (!acceptManyExtensions) { + return acceptAttribute.includes(type) + } + + const fileMimeType = type.match(/\w+\/+/g) + const isValidMimeType = acceptAttribute.includes(fileMimeType) + + return isValidMimeType +} + function upload(element, fileOrFiles, init) { const hasFileWithInvalidType = !Array.isArray(fileOrFiles) && Boolean(element.accept) && - !element.accept.includes(fileOrFiles.type) + !isValidFileType(element.accept, fileOrFiles.type) if (hasFileWithInvalidType || element.disabled) return @@ -19,14 +32,13 @@ function upload(element, fileOrFiles, init) { if (Array.isArray(fileOrFiles)) { files = element.accept - ? fileOrFiles.filter(file => element.accept.includes(file.type)) + ? fileOrFiles.filter(file => isValidFileType(element.accept, file.type)) : fileOrFiles } else { files = [fileOrFiles] } const hasFilesWithInvalidTypes = files.length === 0 - if (hasFilesWithInvalidTypes) return files = files.slice(0, input.multiple ? undefined : 1) From 983eeacfaa63aff12110cce375b625776a9f209e Mon Sep 17 00:00:00 2001 From: Laura Beatris <48022589+LauraBeatris@users.noreply.github.com> Date: Thu, 18 Feb 2021 20:01:06 -0300 Subject: [PATCH 4/9] perf(upload): improve test coverate for files with unaccepted formats --- src/__tests__/upload.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/__tests__/upload.js b/src/__tests__/upload.js index f8879e1d..0e28b111 100644 --- a/src/__tests__/upload.js +++ b/src/__tests__/upload.js @@ -206,7 +206,6 @@ test('should not upload multiple files with unaccepted formats', () => { new File(['hello'], 'hello.txt', {type: 'text/plain'}), new File(['there'], 'there.pdf', {type: 'application/pdf'}), new File(['there'], 'there.png', {type: 'image/png'}), - new File(['there'], 'there.mp4', {type: 'video/mp4'}), ] const {element} = setup(` @@ -214,5 +213,5 @@ test('should not upload multiple files with unaccepted formats', () => { userEvent.upload(element, files) - expect(element.files).toHaveLength(1) + expect(element.files).toHaveLength(0) }) From bbc29c8957f1fbd0ee72a57f4abeff3ac19be42d Mon Sep 17 00:00:00 2001 From: Laura Beatris <48022589+LauraBeatris@users.noreply.github.com> Date: Thu, 18 Feb 2021 20:31:39 -0300 Subject: [PATCH 5/9] style(upload): improve semantic --- src/upload.js | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/upload.js b/src/upload.js index 740743a8..da54ab85 100644 --- a/src/upload.js +++ b/src/upload.js @@ -6,14 +6,16 @@ import {focus} from './focus' const isValidFileType = (acceptAttribute, type) => { const acceptManyExtensions = /(video|audio|image)\/?\*/.test(acceptAttribute) - if (!acceptManyExtensions) { - return acceptAttribute.includes(type) - } + let isValidType - const fileMimeType = type.match(/\w+\/+/g) - const isValidMimeType = acceptAttribute.includes(fileMimeType) + if (acceptManyExtensions) { + type = type.match(/\w+\/+/g) + isValidType = acceptAttribute.includes(type) + } else { + isValidType = acceptAttribute.includes(type) + } - return isValidMimeType + return isValidType } function upload(element, fileOrFiles, init) { From 580f60941ebd0ae7e8481df7b8a0837806860ed7 Mon Sep 17 00:00:00 2001 From: Laura Beatris <48022589+LauraBeatris@users.noreply.github.com> Date: Fri, 19 Feb 2021 07:13:14 -0300 Subject: [PATCH 6/9] style(upload): reduce logic and filter files directly --- src/upload.js | 29 +++++++++++------------------ 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/src/upload.js b/src/upload.js index da54ab85..98f0a0a3 100644 --- a/src/upload.js +++ b/src/upload.js @@ -3,18 +3,19 @@ import {click} from './click' import {blur} from './blur' import {focus} from './focus' -const isValidFileType = (acceptAttribute, type) => { - const acceptManyExtensions = /(video|audio|image)\/?\*/.test(acceptAttribute) +const hasValidFileType = (acceptAttribute, type) => { + if (!acceptAttribute) { + return true + } - let isValidType + const acceptManyExtensions = /(video|audio|image)\/?\*/.test(acceptAttribute) if (acceptManyExtensions) { type = type.match(/\w+\/+/g) - isValidType = acceptAttribute.includes(type) - } else { - isValidType = acceptAttribute.includes(type) } + const isValidType = acceptAttribute.includes(type) + return isValidType } @@ -22,7 +23,7 @@ function upload(element, fileOrFiles, init) { const hasFileWithInvalidType = !Array.isArray(fileOrFiles) && Boolean(element.accept) && - !isValidFileType(element.accept, fileOrFiles.type) + !hasValidFileType(element.accept, fileOrFiles.type) if (hasFileWithInvalidType || element.disabled) return @@ -30,21 +31,13 @@ function upload(element, fileOrFiles, init) { const input = element.tagName === 'LABEL' ? element.control : element - let files = [] - - if (Array.isArray(fileOrFiles)) { - files = element.accept - ? fileOrFiles.filter(file => isValidFileType(element.accept, file.type)) - : fileOrFiles - } else { - files = [fileOrFiles] - } + const files = (Array.isArray(fileOrFiles) ? fileOrFiles : [fileOrFiles]) + .filter(file => hasValidFileType(element.accept, file.type)) + .slice(0, input.multiple ? undefined : 1) const hasFilesWithInvalidTypes = files.length === 0 if (hasFilesWithInvalidTypes) return - files = files.slice(0, input.multiple ? undefined : 1) - // blur fires when the file selector pops up blur(element, init) // focus fires when they make their selection From 538b0d586994a65ec803d4c1bab589b413854bfc Mon Sep 17 00:00:00 2001 From: Philipp Fritsche Date: Fri, 19 Feb 2021 12:12:06 +0100 Subject: [PATCH 7/9] fix: apply file accept tokens --- src/upload.js | 50 ++++++++++++++++++++++++-------------------------- 1 file changed, 24 insertions(+), 26 deletions(-) diff --git a/src/upload.js b/src/upload.js index 98f0a0a3..c178e0f7 100644 --- a/src/upload.js +++ b/src/upload.js @@ -3,46 +3,27 @@ import {click} from './click' import {blur} from './blur' import {focus} from './focus' -const hasValidFileType = (acceptAttribute, type) => { - if (!acceptAttribute) { - return true - } - - const acceptManyExtensions = /(video|audio|image)\/?\*/.test(acceptAttribute) - - if (acceptManyExtensions) { - type = type.match(/\w+\/+/g) - } - - const isValidType = acceptAttribute.includes(type) - - return isValidType -} - function upload(element, fileOrFiles, init) { - const hasFileWithInvalidType = - !Array.isArray(fileOrFiles) && - Boolean(element.accept) && - !hasValidFileType(element.accept, fileOrFiles.type) - - if (hasFileWithInvalidType || element.disabled) return + if (element.disabled) return click(element, init) const input = element.tagName === 'LABEL' ? element.control : element const files = (Array.isArray(fileOrFiles) ? fileOrFiles : [fileOrFiles]) - .filter(file => hasValidFileType(element.accept, file.type)) + .filter(file => isAcceptableFile(file, element.accept)) .slice(0, input.multiple ? undefined : 1) - const hasFilesWithInvalidTypes = files.length === 0 - if (hasFilesWithInvalidTypes) return - // blur fires when the file selector pops up blur(element, init) // focus fires when they make their selection focus(element, init) + // treat empty array as if the user just closed the file upload dialog + if (files.length === 0) { + return + } + // the event fired in the browser isn't actually an "input" or "change" event // but a new Event with a type set to "input" and "change" // Kinda odd... @@ -69,4 +50,21 @@ function upload(element, fileOrFiles, init) { }) } +function isAcceptableFile(file, accept) { + if (!accept) { + return true + } + + const wildcards = ['audio/*', 'image/*', 'video/*'] + + return accept.split(',').some(acceptToken => { + if (accept[0] === '.') { + return file.name.endsWith(accept) + } else if (wildcards.includes(acceptToken)) { + return file.type.startsWith(acceptToken.substr(0, acceptToken.length - 1)) + } + return file.type === acceptToken + }) +} + export {upload} From cda583bb77c78bb8cc52bb15293dfbc37e4989fa Mon Sep 17 00:00:00 2001 From: Philipp Fritsche Date: Fri, 19 Feb 2021 12:31:37 +0100 Subject: [PATCH 8/9] fix: variable name --- src/upload.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/upload.js b/src/upload.js index c178e0f7..6b779976 100644 --- a/src/upload.js +++ b/src/upload.js @@ -58,7 +58,8 @@ function isAcceptableFile(file, accept) { const wildcards = ['audio/*', 'image/*', 'video/*'] return accept.split(',').some(acceptToken => { - if (accept[0] === '.') { + if (acceptToken[0] === '.') { + // tokens starting with a dot represent a file extension return file.name.endsWith(accept) } else if (wildcards.includes(acceptToken)) { return file.type.startsWith(acceptToken.substr(0, acceptToken.length - 1)) From c7b60b91ed1be9fc87de787af580ea15df4a3ccf Mon Sep 17 00:00:00 2001 From: Laura Beatris <48022589+LauraBeatris@users.noreply.github.com> Date: Fri, 19 Feb 2021 11:18:00 -0300 Subject: [PATCH 9/9] fix(upload): improve coverage by testing file extension --- src/__tests__/upload.js | 2 +- src/upload.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/__tests__/upload.js b/src/__tests__/upload.js index 0e28b111..fb6a2b7d 100644 --- a/src/__tests__/upload.js +++ b/src/__tests__/upload.js @@ -183,7 +183,7 @@ test('should upload multiple files with accepted format', () => { const {element} = setup(` `) diff --git a/src/upload.js b/src/upload.js index 6b779976..7a3b990a 100644 --- a/src/upload.js +++ b/src/upload.js @@ -60,7 +60,7 @@ function isAcceptableFile(file, accept) { return accept.split(',').some(acceptToken => { if (acceptToken[0] === '.') { // tokens starting with a dot represent a file extension - return file.name.endsWith(accept) + return file.name.endsWith(acceptToken) } else if (wildcards.includes(acceptToken)) { return file.type.startsWith(acceptToken.substr(0, acceptToken.length - 1)) }