Skip to content

Commit

Permalink
Error out if source provides less data than expected (#606)
Browse files Browse the repository at this point in the history
* Adds end-to-end test that reproduces the issue

* Error out if the source provides less data than specified

* Remove impossible test

* Clean up tests

---------

Co-authored-by: Marius Kleidl <marius@transloadit.com>
  • Loading branch information
sdhull and Acconut authored Aug 2, 2023
1 parent 1a4b936 commit dc57adb
Show file tree
Hide file tree
Showing 7 changed files with 347 additions and 175 deletions.
3 changes: 2 additions & 1 deletion lib/browser/sources/FileSource.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ export default class FileSource {
}

const value = this._file.slice(start, end)
return Promise.resolve({ value })
const done = end >= this.size
return Promise.resolve({ value, done })
}

close() {
Expand Down
3 changes: 2 additions & 1 deletion lib/node/sources/BufferSource.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ export default class BufferSource {
slice(start, end) {
const value = this._buffer.slice(start, end)
value.size = value.length
return Promise.resolve({ value })
const done = end >= this.size
return Promise.resolve({ value, done })
}

close() {}
Expand Down
5 changes: 3 additions & 2 deletions lib/node/sources/FileSource.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,9 @@ class FileSource {
end: offset + end - 1,
autoClose: true,
})
stream.size = end - start
return Promise.resolve({ value: stream })
stream.size = Math.min(end - start, this.size)
const done = stream.size >= this.size
return Promise.resolve({ value: stream, done })
}

close() {
Expand Down
18 changes: 17 additions & 1 deletion lib/upload.js
Original file line number Diff line number Diff line change
Expand Up @@ -782,14 +782,30 @@ class BaseUpload {
}

return this._source.slice(start, end).then(({ value, done }) => {
const valueSize = value && value.size ? value.size : 0

// If the upload length is deferred, the upload size was not specified during
// upload creation. So, if the file reader is done reading, we know the total
// upload size and can tell the tus server.
if (this.options.uploadLengthDeferred && done) {
this._size = this._offset + (value && value.size ? value.size : 0)
this._size = this._offset + valueSize
req.setHeader('Upload-Length', this._size)
}

// The specified uploadSize might not match the actual amount of data that a source
// provides. In these cases, we cannot successfully complete the upload, so we
// rather error out and let the user know. If not, tus-js-client will be stuck
// in a loop of repeating empty PATCH requests.
// See https://community.transloadit.com/t/how-to-abort-hanging-companion-uploads/16488/13
const newSize = this._offset + valueSize
if (!this.options.uploadLengthDeferred && done && newSize !== this._size) {
return Promise.reject(
new Error(
`upload was configured with a size of ${this._size} bytes, but the source is done after ${newSize} bytes`
)
)
}

if (value === null) {
return this._sendRequest(req)
}
Expand Down
45 changes: 45 additions & 0 deletions test/spec/test-browser-specific.js
Original file line number Diff line number Diff line change
Expand Up @@ -658,6 +658,51 @@ describe('tus', () => {

await options.onSuccess.toBeCalled
})

it('should throw an error if the source provides less data than uploadSize', async () => {
const reader = makeReader('hello world')

const testStack = new TestHttpStack()
const options = {
httpStack: testStack,
uploadSize: 100,
chunkSize: 100,
endpoint: 'http://tus.io/uploads',
retryDelays: [],
onError: waitableFunction('onError'),
}

const upload = new tus.Upload(reader, options)
upload.start()
let req = await testStack.nextRequest()
expect(req.url).toBe('http://tus.io/uploads')
expect(req.method).toBe('POST')
expect(req.requestHeaders['Tus-Resumable']).toBe('1.0.0')

req.respondWith({
status: 204,
responseHeaders: {
Location: 'http://tus.io/uploads/foo',
},
})

req = await testStack.nextRequest()
expect(req.url).toBe('http://tus.io/uploads/foo')
expect(req.method).toBe('PATCH')

req.respondWith({
status: 204,
responseHeaders: {
'Upload-Offset': 11,
},
})

const err = await options.onError.toBeCalled

expect(err.message).toBe(
'tus: failed to upload chunk at offset 11, caused by Error: upload was configured with a size of 100 bytes, but the source is done after 11 bytes, originated from request (method: PATCH, url: http://tus.io/uploads/foo, response code: n/a, response text: n/a, request id: n/a)'
)
})
})

describe('resolving of URIs', () => {
Expand Down
32 changes: 32 additions & 0 deletions test/spec/test-common.js
Original file line number Diff line number Diff line change
Expand Up @@ -763,6 +763,38 @@ describe('tus', () => {
)
})

it('should throw an error if the source provides less data than uploadSize', async () => {
const testStack = new TestHttpStack()
const file = getBlob('hello world')
const options = {
httpStack: testStack,
uploadSize: 100,
endpoint: 'http://tus.io/uploads',
retryDelays: [],
onError: waitableFunction('onError'),
}

const upload = new tus.Upload(file, options)
upload.start()

const req = await testStack.nextRequest()
expect(req.url).toBe('http://tus.io/uploads')
expect(req.method).toBe('POST')
expect(req.requestHeaders['Tus-Resumable']).toBe('1.0.0')

req.respondWith({
status: 204,
responseHeaders: {
Location: 'http://tus.io/uploads/foo',
},
})

const err = await options.onError.toBeCalled
expect(err.message).toBe(
'tus: failed to upload chunk at offset 0, caused by Error: upload was configured with a size of 100 bytes, but the source is done after 11 bytes, originated from request (method: PATCH, url: http://tus.io/uploads/foo, response code: n/a, response text: n/a, request id: n/a)'
)
})

it('should throw if retryDelays is not an array', () => {
const file = getBlob('hello world')
const upload = new tus.Upload(file, {
Expand Down
Loading

0 comments on commit dc57adb

Please sign in to comment.