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

Replace reserved chars in filenames #138

Merged
merged 3 commits into from
Mar 29, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## UNRELEASED

### Fixed

- ([#138](https://github.com/demos-europe/demosplan-ui/pull/138)) Uploading files whose names contained reserved characters no longer break certain endpoints. ([@spiess-demos](https://github.com/spiess-demos))

## v0.0.17 - 2023-03-23

- ([#133](https://github.com/demos-europe/demosplan-ui/pull/133)) Import `a11y-datepicker` not as es Module anymore, to make it resolveable
Expand Down
57 changes: 56 additions & 1 deletion src/components/core/DpUpload/DpUpload.vue
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export default {

/**
* Maximum number of files that can be uploaded.
* By default its a single file upload.
* By default, it's allowed to upload a single file.
*/
maxNumberOfFiles: {
type: Number,
Expand Down Expand Up @@ -95,6 +95,60 @@ export default {
return b ? b.pop() : ''
},

/**
* Fires immediately before a file is added to the Uppy store.
*
* The hook is used here to clean file names from characters that would
* break the tus endpoint. This could as well be implemented within the backend.
*
* @param currentFile
* @param files
* @return {(*&{meta: (*&{name: *}), name: *})|*}
* @see https://github.com/transloadit/uppy/blob/main/packages/@uppy/core/src/Uppy.js#L503
*/
handleOnBeforeFileAdded (currentFile, files) {
let fileName = currentFile.name

/*
* This is not an exhaustive list - the characters have been determined
* simply by trying out which break or do not break the tus endpoint.
*/
const reservedCharacters = [
'?',
'#',
'&',
'\'',
'"',
'/',
':',
'\\'
]

/*
* For reasons i could not figure out, the for loop was needed here
* to reliably access the item that is looped over. When using forEach,
* strangely it always held the value of currentFile.
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since its totally fine to use a for-loop, I suggest to drop the comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but in case somebody wants to optimize it, they'll be warned, if we leave the comment there.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I am against that. Then we have to argue all our code in comments....

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, removed in 5c6ee11

for (let i = 0; i < reservedCharacters.length; i++) {
if (fileName.includes(reservedCharacters[i])) {
fileName = fileName.replace(reservedCharacters[i], '_')
}
}

if (fileName !== currentFile.name) {
return {
...currentFile,
name: fileName,
meta: {
...currentFile.meta,
name: fileName
salisdemos marked this conversation as resolved.
Show resolved Hide resolved
}
}
} else {
return currentFile
}
},

initialize () {
const locale = { strings: { ...de().strings, ...this.translations } }
this.uppy = new Uppy({
Expand All @@ -106,6 +160,7 @@ export default {
maxFileSize: this.maxFileSize,
maxNumberOfFiles: this.maxNumberOfFiles
},
onBeforeFileAdded: this.handleOnBeforeFileAdded,
locale: locale
})

Expand Down
20 changes: 11 additions & 9 deletions src/components/core/DpUpload/DpUploadedFile.vue
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,10 @@
<span
:class="prefixClass('display--inline-block u-pl-0_5')"
style="width: calc(100% - 62px);">
{{ file.name }}
({{ file.size }})
<span class="overflow-word-break">
{{ file.name }}
</span>
({{ fileSize }})
<button
type="button"
:class="prefixClass('btn-icns u-m-0')"
Expand All @@ -31,7 +33,7 @@
</template>

<script>
import { getFileInfo } from '../../../lib'
import { convertSize } from '../../../lib'
import { prefixClassMixin } from '../../../mixins'

export default {
Expand All @@ -42,22 +44,22 @@ export default {
mixins: [prefixClassMixin],

props: {
fileString: {
type: String,
file: {
type: Object,
required: true
}
},

computed: {
file () {
return getFileInfo(this.fileString)
},

fileIcon () {
const icon = this.file.mimeType === 'txt' ? 'fa-file-text-o' : 'fa-folder-o'
return this.prefixClass('fa ' + icon)
},

fileSize () {
return convertSize('KB', this.file.size)
},

isImage () {
const imageTypes = ['png', 'jpg', 'gif', 'bmp', 'ico', 'tiff', 'svg']
return typeof imageTypes.find(type => type === this.file.mimeType) !== 'undefined'
Expand Down
14 changes: 2 additions & 12 deletions src/components/core/DpUpload/DpUploadedFileList.vue
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
</h4>
<ul :class="prefixClass('o-list space-stack-xs')">
<dp-uploaded-file
v-for="(fileString, idx) in fileStrings"
:file-string="fileString"
v-for="(file, idx) in files"
:file="file"
@file-remove="file => $emit('file-remove', file)"
:key="idx" />
</ul>
Expand All @@ -32,16 +32,6 @@ export default {
required: false,
default: () => ([])
}
},

computed: {
fileStrings () {
return this.files.map(file => {
let str = Object.values(file).toString()
str = str.replace(/,/g, ':')
return str
})
}
}
}
</script>