From 4e904469c909cb2253623485709f6dec6db0f416 Mon Sep 17 00:00:00 2001 From: "Thomas F. K. Jorna" Date: Thu, 9 Jul 2026 22:32:31 +0200 Subject: [PATCH 1/2] fix: allow files with # to be uploaded, show upload confirmation --- .../FileUploadButton/FileUploadButton.tsx | 17 +++- .../FormattingBar/media/MediaAudio.tsx | 13 ++- .../FormattingBar/media/MediaFile.tsx | 13 ++- .../FormattingBar/media/MediaImage.tsx | 13 ++- .../FormattingBar/media/MediaVideo.tsx | 13 ++- .../components/FormattingBar/media/media.scss | 6 ++ .../containers/Pub/SpubHeader/DetailsTab.tsx | 2 +- client/utils/upload.ts | 79 +++++++++++++++---- 8 files changed, 135 insertions(+), 21 deletions(-) diff --git a/client/components/FileUploadButton/FileUploadButton.tsx b/client/components/FileUploadButton/FileUploadButton.tsx index 7ad67507c8..e38972e2ae 100644 --- a/client/components/FileUploadButton/FileUploadButton.tsx +++ b/client/components/FileUploadButton/FileUploadButton.tsx @@ -42,6 +42,7 @@ class FileUploadButton extends Component { }; this.randKey = Math.round(Math.random() * 99999).toString(); this.handleUploadFinish = this.handleUploadFinish.bind(this); + this.handleUploadError = this.handleUploadError.bind(this); this.handleFileSelect = this.handleFileSelect.bind(this); } @@ -52,9 +53,23 @@ class FileUploadButton extends Component { }); } + handleUploadError(err) { + // biome-ignore lint/suspicious/noAlert: simplest feedback in this button component + alert(`Upload failed: ${err.message}`); + this.setState({ + isUploading: false, + }); + } + handleFileSelect(evt) { if (evt.target.files.length) { - s3Upload(evt.target.files[0], () => {}, this.handleUploadFinish, 0); + s3Upload( + evt.target.files[0], + () => {}, + this.handleUploadFinish, + 0, + this.handleUploadError, + ); this.setState({ isUploading: true, }); diff --git a/client/components/FormattingBar/media/MediaAudio.tsx b/client/components/FormattingBar/media/MediaAudio.tsx index dccf245d9a..7e15fa1f7f 100644 --- a/client/components/FormattingBar/media/MediaAudio.tsx +++ b/client/components/FormattingBar/media/MediaAudio.tsx @@ -19,17 +19,21 @@ class MediaAudio extends Component { this.state = { isUploading: false, progress: 0, + uploadError: null, }; this.onDrop = this.onDrop.bind(this); this.onUploadFinish = this.onUploadFinish.bind(this); this.onUploadProgress = this.onUploadProgress.bind(this); + this.onUploadError = this.onUploadError.bind(this); } onDrop(files) { if (files.length) { - s3Upload(files[0], this.onUploadProgress, this.onUploadFinish, 0); + s3Upload(files[0], this.onUploadProgress, this.onUploadFinish, 0, this.onUploadError); this.setState({ isUploading: true, + progress: 0, + uploadError: null, }); } } @@ -47,6 +51,10 @@ class MediaAudio extends Component { }); } + onUploadError(err) { + this.setState({ isUploading: false, uploadError: err.message }); + } + render() { return ( @@ -65,6 +73,9 @@ class MediaAudio extends Component {
Drag & drop to upload Audio
Or click to browse files
.mp3, .wav, or .ogg
+ {this.state.uploadError && ( +
{this.state.uploadError}
+ )} )} {this.state.isUploading && ( diff --git a/client/components/FormattingBar/media/MediaFile.tsx b/client/components/FormattingBar/media/MediaFile.tsx index 53a8a74062..23881294d5 100644 --- a/client/components/FormattingBar/media/MediaFile.tsx +++ b/client/components/FormattingBar/media/MediaFile.tsx @@ -22,17 +22,21 @@ class MediaFile extends Component { progress: 0, loadingFileName: '', loadingFileSize: '', + uploadError: null, }; this.onDrop = this.onDrop.bind(this); this.onUploadFinish = this.onUploadFinish.bind(this); this.onUploadProgress = this.onUploadProgress.bind(this); + this.onUploadError = this.onUploadError.bind(this); } onDrop(files) { if (files.length) { - s3Upload(files[0], this.onUploadProgress, this.onUploadFinish, 0); + s3Upload(files[0], this.onUploadProgress, this.onUploadFinish, 0, this.onUploadError); this.setState({ isUploading: true, + progress: 0, + uploadError: null, loadingFileName: files[0].name, loadingFileSize: filesize(files[0].size, { round: 0 }), }); @@ -53,6 +57,10 @@ class MediaFile extends Component { }); } + onUploadError(err) { + this.setState({ isUploading: false, uploadError: err.message }); + } + render() { return ( @@ -70,6 +78,9 @@ class MediaFile extends Component {
Drag & drop to upload a File
Or click to browse files
+ {this.state.uploadError && ( +
{this.state.uploadError}
+ )} )} {this.state.isUploading && ( diff --git a/client/components/FormattingBar/media/MediaImage.tsx b/client/components/FormattingBar/media/MediaImage.tsx index 9fc556e6ce..955c0a7d87 100644 --- a/client/components/FormattingBar/media/MediaImage.tsx +++ b/client/components/FormattingBar/media/MediaImage.tsx @@ -19,17 +19,21 @@ class MediaImage extends Component { this.state = { isUploading: false, progress: 0, + uploadError: null, }; this.onDrop = this.onDrop.bind(this); this.onUploadFinish = this.onUploadFinish.bind(this); this.onUploadProgress = this.onUploadProgress.bind(this); + this.onUploadError = this.onUploadError.bind(this); } onDrop(files) { if (files.length) { - s3Upload(files[0], this.onUploadProgress, this.onUploadFinish, 0); + s3Upload(files[0], this.onUploadProgress, this.onUploadFinish, 0, this.onUploadError); this.setState({ isUploading: true, + progress: 0, + uploadError: null, }); } } @@ -47,6 +51,10 @@ class MediaImage extends Component { }); } + onUploadError(err) { + this.setState({ isUploading: false, uploadError: err.message }); + } + render() { return ( {
.jpeg, .png, .svg, .webp, or .gif
+ {this.state.uploadError && ( +
{this.state.uploadError}
+ )} )} {this.state.isUploading && ( diff --git a/client/components/FormattingBar/media/MediaVideo.tsx b/client/components/FormattingBar/media/MediaVideo.tsx index 2e08b86e23..9b78d1a615 100644 --- a/client/components/FormattingBar/media/MediaVideo.tsx +++ b/client/components/FormattingBar/media/MediaVideo.tsx @@ -19,17 +19,21 @@ class MediaVideo extends Component { this.state = { isUploading: false, progress: 0, + uploadError: null, }; this.onDrop = this.onDrop.bind(this); this.onUploadFinish = this.onUploadFinish.bind(this); this.onUploadProgress = this.onUploadProgress.bind(this); + this.onUploadError = this.onUploadError.bind(this); } onDrop(files) { if (files.length) { - s3Upload(files[0], this.onUploadProgress, this.onUploadFinish, 0); + s3Upload(files[0], this.onUploadProgress, this.onUploadFinish, 0, this.onUploadError); this.setState({ isUploading: true, + progress: 0, + uploadError: null, }); } } @@ -47,6 +51,10 @@ class MediaVideo extends Component { }); } + onUploadError(err) { + this.setState({ isUploading: false, uploadError: err.message }); + } + render() { return ( @@ -65,6 +73,9 @@ class MediaVideo extends Component {
Drag & drop to upload a Video
Or click to browse files
.mp4 or .webm
+ {this.state.uploadError && ( +
{this.state.uploadError}
+ )} )} {this.state.isUploading && ( diff --git a/client/components/FormattingBar/media/media.scss b/client/components/FormattingBar/media/media.scss index e902b38ad0..b62f635aac 100644 --- a/client/components/FormattingBar/media/media.scss +++ b/client/components/FormattingBar/media/media.scss @@ -38,6 +38,12 @@ $bp: vendor.$bp-namespace; padding-top: 1em; font-size: 16px; } + .drag-error { + padding-top: 1em; + font-size: 16px; + font-weight: 600; + color: #db3737; + } } .top-input { display: flex; diff --git a/client/containers/Pub/SpubHeader/DetailsTab.tsx b/client/containers/Pub/SpubHeader/DetailsTab.tsx index 6820c5b6e8..8ec74b62c0 100644 --- a/client/containers/Pub/SpubHeader/DetailsTab.tsx +++ b/client/containers/Pub/SpubHeader/DetailsTab.tsx @@ -100,7 +100,7 @@ const DetailsTab = (props: Props) => { onUpdatePub({ downloads })} text="Upload new file" /> diff --git a/client/utils/upload.ts b/client/utils/upload.ts index 1e469d54d5..eff75c9481 100644 --- a/client/utils/upload.ts +++ b/client/utils/upload.ts @@ -33,16 +33,26 @@ const checkForAsset = (url): Promise => { const checkUrl = () => { fetch(url, { method: 'HEAD', - }).then((response) => { - if (!response.ok) { + }) + .then((response) => { + if (!response.ok) { + if (checkCount < maxCheckCount) { + checkCount += 1; + return setTimeout(checkUrl, checkInterval); + } + return reject( + new Error(`Uploaded file could not be verified (status ${response.status})`), + ); + } + return resolve(); + }) + .catch((err) => { if (checkCount < maxCheckCount) { checkCount += 1; return setTimeout(checkUrl, checkInterval); } - return reject(); - } - return resolve(); - }); + return reject(err); + }); }; checkUrl(); }); @@ -54,7 +64,11 @@ const getFileNameForUpload = (file: File) => { const { communityId, userId, pubId } = getUploadContext(); const [rawFileName = 'unknown', fileExtension = 'jpg'] = file.name?.split(/(.*)\.(.*)/).filter(Boolean) ?? []; - const fileName = rawFileName.replace(/\s+/g, '_'); + // Replace whitespace and any character that isn't URL/S3-key safe (e.g. `#`, `?`, + // `%`, `&`) with an underscore. Such characters otherwise end up in the S3 key and + // break both the upload verification (`#` is parsed as a URL fragment, so the HEAD + // check hits the wrong object and S3 returns a 403) and the eventual download URL. + const fileName = rawFileName.replace(/[^a-zA-Z0-9\-_.]+/g, '_'); const random = Math.floor(Math.random() * 8); const now = new Date().getTime(); const pubSegment = pubId ? `/p${pubId}` : ''; @@ -65,16 +79,34 @@ import { MAX_UPLOAD_SIZE_BYTES } from 'utils/uploadConsts'; const getBaseUrlForBucket = (bucket) => `https://s3-external-1.amazonaws.com/${bucket}`; -export const s3Upload = (file: File, onProgress, onFinish, index?: number) => { +const defaultOnError = (err: Error) => { + // biome-ignore lint/suspicious/noAlert: simplest feedback for a non-React utility + alert(`Upload failed: ${err.message}`); +}; + +export const s3Upload = ( + file: File, + onProgress, + onFinish, + index?: number, + onError: (err: Error) => void = defaultOnError, +) => { if (file.size > MAX_UPLOAD_SIZE_BYTES) { const sizeMB = (file.size / (1024 * 1024)).toFixed(1); - // biome-ignore lint/suspicious/noAlert: simplest feedback for a non-React utility - alert(`File "${file.name}" is ${sizeMB} MB, which exceeds the 100 MB upload limit.`); + onError( + new Error( + `File "${file.name}" is ${sizeMB} MB, which exceeds the 100 MB upload limit.`, + ), + ); return; } const fileName = getFileNameForUpload(file); const fileType = file.type !== undefined ? file.type : 'image/jpeg'; function beginUpload(this: any) { + if (this.status < 200 || this.status >= 300) { + onError(new Error(`Could not get an upload policy (status ${this.status})`)); + return; + } const { policy, signature, acl, awsAccessKeyId, bucket } = JSON.parse(this.responseText); const formData = new FormData(); formData.append('key', fileName); @@ -91,19 +123,36 @@ export const s3Upload = (file: File, onProgress, onFinish, index?: number) => { const sendFile = new XMLHttpRequest(); const baseUrl = getBaseUrlForBucket(bucket); sendFile.upload.addEventListener('progress', (evt) => onProgress(evt, index), false); - sendFile.upload.addEventListener( + // Listen on the request (not `sendFile.upload`) so we can inspect S3's response + // status. A rejected upload (e.g. a 403 for a malformed key) still fires the + // upload `load` event, so checking the response status here is what lets us + // surface the failure instead of spinning forever. + sendFile.addEventListener( 'load', - (evt) => - checkForAsset(`${baseUrl}/${fileName}`).then(() => - onFinish(evt, index, file.type, fileName, file.name), - ), + (evt) => { + if (sendFile.status < 200 || sendFile.status >= 300) { + onError(new Error(`Upload was rejected by the server (status ${sendFile.status})`)); + return; + } + checkForAsset(`${baseUrl}/${fileName}`).then( + () => onFinish(evt, index, file.type, fileName, file.name), + (err) => onError(err instanceof Error ? err : new Error('Upload verification failed')), + ); + }, false, ); + sendFile.addEventListener('error', () => onError(new Error('Network error during upload')), false); + sendFile.addEventListener('abort', () => onError(new Error('Upload was aborted')), false); sendFile.open('POST', baseUrl, true); sendFile.send(formData); } const getPolicy = new XMLHttpRequest(); getPolicy.addEventListener('load', beginUpload); + getPolicy.addEventListener( + 'error', + () => onError(new Error('Network error while requesting upload policy')), + false, + ); const policyParams = new URLSearchParams({ contentType: file.type, }); From 9b529746b8a396b5dfcdb4fd37ef86b1d404ce38 Mon Sep 17 00:00:00 2001 From: "Thomas F. K. Jorna" Date: Thu, 9 Jul 2026 22:36:15 +0200 Subject: [PATCH 2/2] chore: lint --- client/utils/upload.ts | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/client/utils/upload.ts b/client/utils/upload.ts index eff75c9481..3f2bd848fc 100644 --- a/client/utils/upload.ts +++ b/client/utils/upload.ts @@ -41,7 +41,9 @@ const checkForAsset = (url): Promise => { return setTimeout(checkUrl, checkInterval); } return reject( - new Error(`Uploaded file could not be verified (status ${response.status})`), + new Error( + `Uploaded file could not be verified (status ${response.status})`, + ), ); } return resolve(); @@ -131,17 +133,26 @@ export const s3Upload = ( 'load', (evt) => { if (sendFile.status < 200 || sendFile.status >= 300) { - onError(new Error(`Upload was rejected by the server (status ${sendFile.status})`)); + onError( + new Error(`Upload was rejected by the server (status ${sendFile.status})`), + ); return; } checkForAsset(`${baseUrl}/${fileName}`).then( () => onFinish(evt, index, file.type, fileName, file.name), - (err) => onError(err instanceof Error ? err : new Error('Upload verification failed')), + (err) => + onError( + err instanceof Error ? err : new Error('Upload verification failed'), + ), ); }, false, ); - sendFile.addEventListener('error', () => onError(new Error('Network error during upload')), false); + sendFile.addEventListener( + 'error', + () => onError(new Error('Network error during upload')), + false, + ); sendFile.addEventListener('abort', () => onError(new Error('Upload was aborted')), false); sendFile.open('POST', baseUrl, true); sendFile.send(formData);