veliovgroup/Meteor-Files

fails with `onBeforeUpload returned false [403]`, but my onBeforeUpload returns true

trusktr opened this issue · 7 comments

I'm having an issue:

  • Give an expressive description of what is went wrong:
    • Upload fails with transport http, says config.onBeforeUpload() returned false [403]
    • wish this was easier
  • Version of Meteor-Files you're experiencing this issue:
    • ostrio:files 2.2.1
  • Version of Meteor you're experiencing this issue
    • 2.7.1
  • Where this issue appears?
    • OS Linux, Chrome
  • Is it Client or Server issue?
    • client
  • Post Client and/or Server logs with enabled debug option, you can enable "debug" mode in Constructor
    • config.onBeforeUpload() returned false [403]

My code for FileCollection is basically copy/paste from GridFS guide:

// Based on https://github.com/veliovgroup/Meteor-Files/blob/master/docs/gridfs-bucket-integration.md#use-gridfs-with-gridfsbucket-as-a-storage

import {Meteor} from 'meteor/meteor'
import {FilesCollection} from 'meteor/ostrio:files'
import {createBucket} from './gridfs/createBucket'
import {createObjectId} from './gridfs/createObjectId'
import fs from 'fs'

type TODO = any

let tracksBucket: TODO

if (Meteor.isServer) {
	tracksBucket = createBucket('tracksBucket')
}

console.log('FilesCollection.schema', FilesCollection.schema)

export const Tracks = new FilesCollection({
	collectionName: 'tracks',
	allowClientCode: true,
	debug: Meteor.isServer && process.env.NODE_ENV === 'development',

	onBeforeUpload(file) {
		return true

		if (file.size <= 104_857_600 && /png|jpg|jpeg/i.test(file.extension)) {
			return true
		}

		return 'Please upload a file, with size equal or less than 100MB'
	},

	onAfterUpload(file) {
		// here we could manipulate our file and create a new version, for
		// example lower resolution version, etc...

		// then we read all versions we have got so far
		Object.keys(file.versions).forEach(versionName => {
			const metadata = {...file.meta, versionName, fileId: file._id}

			fs.createReadStream(file.versions[versionName].path)

				// upload the binary to the bucket
				.pipe(
					tracksBucket.openUploadStream(file.name, {
						contentType: file.type || 'binary/octet-stream',
						metadata,
					}),
				)

				// unlink the file from the fs on any error that occurred during
				// the upload to prevent zombie files
				.on('error', err => {
					console.error(err)
					this.unlink(this.collection.findOne(file._id), versionName) // Unlink files from FS
				})

				// once finished, attach the gridFS Object id on the
				// FilesCollection document's meta section and finally unlink
				// the upload file from the filesystem
				.on(
					'finish',
					Meteor.bindEnvironment(ver => {
						const property = `versions.${versionName}.meta.gridFsFileId`

						this.collection.update(file._id, {
							$set: {
								[property]: ver._id.toHexString(),
							},
						})

						this.unlink(this.collection.findOne(file._id), versionName) // Unlink files from FS
					}),
				)
		})
	},

	interceptDownload(http, file, versionName) {
		const {gridFsFileId} = file.versions[versionName].meta || {}

		if (gridFsFileId) {
			const gfsId = createObjectId({gridFsFileId})
			const readStream = tracksBucket.openDownloadStream(gfsId)

			readStream.on('data', data => {
				http.response.write(data)
			})

			readStream.on('end', () => {
				http.response.end('end')
			})

			readStream.on('error', () => {
				// not found probably
				// eslint-disable-next-line no-param-reassign
				http.response.statusCode = 404
				http.response.end('not found')
			})

			http.response.setHeader('Cache-Control', this.cacheControl)
			http.response.setHeader('Content-Disposition', `inline; filename="${file.name}"`)
		}

		return Boolean(gridFsFileId) // Serve file from either GridFS or FS if it wasn't uploaded yet
	},

	onAfterRemove(files) {
		files.forEach(file => {
			Object.keys(file.versions).forEach(versionName => {
				const gridFsFileId = (file.versions[versionName].meta || {}).gridFsFileId

				if (gridFsFileId) {
					const gfsId = createObjectId({gridFsFileId})

					tracksBucket.delete(gfsId, err => {
						if (err) console.error(err)
					})
				}
			})
		})
	},
})

if (Meteor.isServer) {
	Meteor.publish('tracks.all', function () {
		return Tracks.collection.find({})
	})
}

if (Meteor.isClient) Meteor.subscribe('tracks.all')

And here's the upload code:

	function upload(e: InputEvent) {
		e.preventDefault()

		const input = e.currentTarget as HTMLInputElement | null
		const file = input?.files?.[0]

		if (file) {
			// insert() docs: https://github.com/veliovgroup/Meteor-Files/blob/master/docs/insert.md
			let uploadInstance = Tracks.insert(
				{
					file,

					transport: 'http',

					// optional data to go with the file
					meta: {
						// userId: Meteor.userId(),  TODO
					},

					chunkSize: 'dynamic',
					allowWebWorkers: true, // If you see issues with uploads, change this to false

					onStart(_error, file) {
						console.log('start upload', file)
					},
					onAbort() {},
					onBeforeUpload() {},
					onError() {},
					onProgress(progress, file) {
						console.log('upload progress', progress, file)
					},
					onUploaded(error, file) {
						if (error) throw error
						console.log('done uploading', file)
					},
				},
				false,
			)

			uploadInstance.start()
		}
	}

Then when upload is called, the onUploaded method is passed that error.

If I remove transport: 'http' I get the same config.onBeforeUpload() returned false [403] error

Hello @trusktr ,

You have defined onBeforeUpload in .insert() method. Thought it's empty its return will be treated as undefined !== true;

Solution: remove onBeforeUpload hook or return true in the object passed to .insert() method

Thanks! I returned true from the onBeforeUpload on the client and that did the trick.

Does this example does it wrong?

https://github.com/Neobii/files-gridfs-autoform-example/blob/master/lib/images.js

Did the API change perhaps? Was that ok at some point?

Oh, onBeforeUpload works in both places? I imagine the server needs to validate, because a client can easily hack around the onBeforeUpload on the client.

Does this example does it wrong?

https://github.com/Neobii/files-gridfs-autoform-example/blob/master/lib/images.js

No, I don't see onBeforeUpload defined in object passed into .insert() method

Oh, onBeforeUpload works in both places?

@trusktr correct, check on client and server with option skip Client

Feel free to close it in case if the issue is solved on your end.

Hello @trusktr, was this one solved on your end?

Feel free to close it in case if the issue is solved on your end.