fix: allow files with # to be uploaded, show upload confirmation

client/components/FileUploadButton/FileUploadButton.tsx

@@ -42,6 +42,7 @@ class FileUploadButton extends Component<Props, State> {
 		};
 		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<Props, State> {
 		});
 	}
 
+	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,
 			});

client/components/FormattingBar/media/MediaAudio.tsx

@@ -19,17 +19,21 @@ class MediaAudio extends Component<Props, State> {
 		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<Props, State> {
 		});
 	}
 
+	onUploadError(err) {
+		this.setState({ isUploading: false, uploadError: err.message });
+	}
+
 	render() {
 		return (
 			<Dropzone onDrop={this.onDrop} accept="audio/mpeg, audio/ogg, audio/wav, audio/x-wav">
@@ -65,6 +73,9 @@ class MediaAudio extends Component<Props, State> {
 									<div className="drag-title">Drag & drop to upload Audio</div>
 									<div className="drag-details">Or click to browse files</div>
 									<div className="drag-details">.mp3, .wav, or .ogg</div>
+									{this.state.uploadError && (
+										<div className="drag-error">{this.state.uploadError}</div>
+									)}
 								</div>
 							)}
 							{this.state.isUploading && (

client/components/FormattingBar/media/MediaFile.tsx

@@ -22,17 +22,21 @@ class MediaFile extends Component<Props, State> {
 			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<Props, State> {
 		});
 	}
 
+	onUploadError(err) {
+		this.setState({ isUploading: false, uploadError: err.message });
+	}
+
 	render() {
 		return (
 			<Dropzone onDrop={this.onDrop}>
@@ -70,6 +78,9 @@ class MediaFile extends Component<Props, State> {
 									<Icon icon="circle-arrow-up" iconSize={50} />
 									<div className="drag-title">Drag & drop to upload a File</div>
 									<div className="drag-details">Or click to browse files</div>
+									{this.state.uploadError && (
+										<div className="drag-error">{this.state.uploadError}</div>
+									)}
 								</div>
 							)}
 							{this.state.isUploading && (

client/components/FormattingBar/media/MediaImage.tsx

@@ -19,17 +19,21 @@ class MediaImage extends Component<Props, State> {
 		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<Props, State> {
 		});
 	}
 
+	onUploadError(err) {
+		this.setState({ isUploading: false, uploadError: err.message });
+	}
+
 	render() {
 		return (
 			<Dropzone
@@ -70,6 +78,9 @@ class MediaImage extends Component<Props, State> {
 									<div className="drag-details">
 										.jpeg, .png, .svg, .webp, or .gif
 									</div>
+									{this.state.uploadError && (
+										<div className="drag-error">{this.state.uploadError}</div>
+									)}
 								</div>
 							)}
 							{this.state.isUploading && (

client/components/FormattingBar/media/MediaVideo.tsx

@@ -19,17 +19,21 @@ class MediaVideo extends Component<Props, State> {
 		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<Props, State> {
 		});
 	}
 
+	onUploadError(err) {
+		this.setState({ isUploading: false, uploadError: err.message });
+	}
+
 	render() {
 		return (
 			<Dropzone onDrop={this.onDrop} accept="video/mp4, video/webm">
@@ -65,6 +73,9 @@ class MediaVideo extends Component<Props, State> {
 									<div className="drag-title">Drag & drop to upload a Video</div>
 									<div className="drag-details">Or click to browse files</div>
 									<div className="drag-details">.mp4 or .webm</div>
+									{this.state.uploadError && (
+										<div className="drag-error">{this.state.uploadError}</div>
+									)}
 								</div>
 							)}
 							{this.state.isUploading && (

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;

client/containers/Pub/SpubHeader/DetailsTab.tsx

@@ -100,7 +100,7 @@ const DetailsTab = (props: Props) => {
 				<DownloadChooser
 					pubData={pub}
 					communityId={pub.communityId}
-					onSetDownloads={onUpdatePub}
+					onSetDownloads={(downloads) => onUpdatePub({ downloads })}
 					text="Upload new file"
 				/>
 			</SpubHeaderField>

client/utils/upload.ts

@@ -33,16 +33,28 @@ const checkForAsset = (url): Promise<void> => {
 		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 +66,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 +81,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 +125,45 @@ 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,
 	});