Skip to content

Commit

Permalink
fix(mimetype): Update mimetype detection function (#356)
Browse files Browse the repository at this point in the history
* fix(mimetype): Update mimetype detection function

* feat(s3errors): Parse s3 error responses

* fix(s3errors): add s3 retry policy to ii upload
  • Loading branch information
pcholuj authored May 13, 2020
1 parent 4927d9e commit bf4146d
Show file tree
Hide file tree
Showing 14 changed files with 291 additions and 76 deletions.
54 changes: 50 additions & 4 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,15 @@
"abab": "^2.0.3",
"debug": "^4.1.1",
"eventemitter3": "^4.0.0",
"fast-xml-parser": "^3.16.0",
"file-type": "^10.11.0",
"follow-redirects": "^1.10.0",
"isutf8": "^2.1.0",
"jsonschema": "^1.2.5",
"lodash.clonedeep": "^4.5.0",
"p-queue": "^4.0.0",
"spark-md5": "^3.0.0"
"spark-md5": "^3.0.0",
"ts-node": "^8.10.1"
},
"devDependencies": {
"@babel/core": "^7.8.4",
Expand Down
41 changes: 0 additions & 41 deletions src/lib/api/prefetch.spec.browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -225,45 +225,4 @@ describe('Prefetch', () => {

scope.done();
});

it('should send only events when config is already prefetched', async () => {
const sessionCopy = { ...testSession };
const mockPref = jest.fn() .mockImplementation(() => ({}));

scope.post('/prefetch').twice().reply(200, (_, data) => mockPref(data));

const prefetch = new Prefetch(sessionCopy);
const toSend = {
events: [PrefetchEvents.PICKER],
pickerOptions: {
uploadInBackground: true,
},
};

await prefetch.getConfig(toSend);

// add one more options request for second request
scope
.options(/.*/)
.reply(204);

await prefetch.getConfig(toSend);

expect(mockPref).toHaveBeenCalledTimes(2);
expect(mockPref).toHaveBeenCalledWith({
apikey: testApiKey,
events: [PrefetchEvents.PICKER],
settings: ['inapp_browser'],
picker_config: {
uploadInBackground: true,
},
});

expect(mockPref).toHaveBeenCalledWith({
apikey: testApiKey,
events: [PrefetchEvents.PICKER],
});

scope.done();
});
});
8 changes: 4 additions & 4 deletions src/lib/api/prefetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,17 +93,17 @@ export class Prefetch {
paramsToSend.security = { policy: this.session.policy, signature: this.session.signature };
}

if (this.session.prefetch && events) {
return FsRequest.post(`${this.session.urls.uploadApiUrl}/prefetch`, { ...paramsToSend, events }).then(() => this.session.prefetch);
}
// if (this.session.prefetch && events) {
// return FsRequest.post(`${this.session.urls.uploadApiUrl}/prefetch`, { ...paramsToSend, events }).then(() => this.session.prefetch);
// }

// we should always ask for this setting for picker
if (!settings) {
settings = ['inapp_browser'];
} else {
settings = settings.concat(['inapp_browser']);
// make arrray unique
settings = settings.filter((v, i, a) => settings.indexOf(v) === i);
settings = settings.filter((v, i) => settings.indexOf(v) === i);
}

let pickerOptionsToSend;
Expand Down
2 changes: 1 addition & 1 deletion src/lib/api/upload/file_tools.browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ export const getFile = (input: InputFile, sanitizeOptions?: SanitizeOptions): Pr
return readFile(file).then(
async res => {
let mime = file.type;
if (!file.type) {
if (!file.type || file.type.length === 0 || file.type === 'text/plain') {
mime = getMimetype(await res.slice(0, fileType.minimumBytes), filename);
}

Expand Down
28 changes: 28 additions & 0 deletions src/lib/api/upload/uploaders/s3.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,15 @@ const getSmallTestFile = () =>
name: 'test.txt',
});

const getNullSizeFile = () =>
new File({
slice: (start, end) => Promise.resolve(testBuff.slice(start, end)),
type: 'text/plain',
// @ts-ignore
size: 0,
name: 'test.txt',
});

const testApikey = 'testapikey';
const testHost = 'https://filestack-test.com';
const mockUploadId = '123132123';
Expand Down Expand Up @@ -230,6 +239,25 @@ describe('Api/Upload/Uploaders/S3', () => {
expect(mockPut).toHaveBeenCalled();
});

it('should throw error on file size 0', async () => {
mockStart.mockReturnValue({
uri: mockedUri,
region: mockRegion,
upload_id: mockUploadId,
location_url: testHost.replace('https://', ''),
});

const u = new S3Uploader({});
u.setUrl(testHost);
u.setApikey(testApikey);
u.addFile(getNullSizeFile());

const res = await u.execute();

expect(res[0].status).toEqual('Failed');
expect(mockStart).not.toHaveBeenCalled();
});

it('should throw error on wrong etag field', async (done) => {
mockStart.mockReturnValue({
uri: mockedUri,
Expand Down
45 changes: 44 additions & 1 deletion src/lib/api/upload/uploaders/s3.ts
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,11 @@ export class S3Uploader extends UploaderAbstract {
private startRequest(id: string): Promise<any> {
const payload = this.getPayloadById(id);

if (payload.file.size === 0) {
this.setPayloadStatus(id, FileState.FAILED);
return Promise.reject(new FilestackError(`Invalid file "${payload.file.name}" size - 0`, {}, FilestackErrorType.VALIDATION));
}

debug(`[${id}] Make start request`);
return FsRequest.post(
`${this.getUrl()}/multipart/start`,
Expand Down Expand Up @@ -435,7 +440,6 @@ export class S3Uploader extends UploaderAbstract {

const { data, headers } = await this.getS3PartMetadata(id, part);
debug(`[${id}] Received part ${partNumber} info body: \n%O\n headers: \n%O\n`, data, headers);

return FsRequest.put(data.url, part.buffer, {
cancelToken: this.cancelToken,
timeout: this.timeout,
Expand Down Expand Up @@ -465,6 +469,26 @@ export class S3Uploader extends UploaderAbstract {
return res;
})
.catch(err => {
const resp = err && err.response ? err.response : null;

/* istanbul ignore next */
if (resp && resp.status === 403) {
if (resp.data && resp.data.Error && resp.data.Error.code) {
let code = resp.data.Error.code;

if (Array.isArray(code)) {
code = code.pop();
}

switch (code) {
case 'RequestTimeTooSkewed':
return this.startPart(id, partNumber);
default:
return Promise.reject(new FilestackError('Cannot upload file', resp.data.Error, FilestackErrorType.REQUEST));
}
}
}

// release memory
part = null;

Expand Down Expand Up @@ -554,6 +578,25 @@ export class S3Uploader extends UploaderAbstract {
return this.uploadNextChunk(id, partNumber, chunkSize);
})
.catch(err => {
const resp = err && err.response ? err.response : null;
/* istanbul ignore next */
if (resp && resp.status === 403) {
if (resp.data && resp.data.Error && resp.data.Error.code) {
let code = resp.data.Error.code;

if (Array.isArray(code)) {
code = code.pop();
}

switch (code) {
case 'RequestTimeTooSkewed':
return this.startPart(id, partNumber);
default:
return Promise.reject(new FilestackError('Cannot upload file', resp.data.Error, FilestackErrorType.REQUEST));
}
}
}

// reset progress on failed upload
this.onProgressUpdate(id, partNumber, part.offset);
const nextChunkSize = chunkSize / 2;
Expand Down
4 changes: 2 additions & 2 deletions src/lib/request/adapters/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -231,12 +231,12 @@ export class HttpAdapter implements AdapterInterface {
return reject(new FsRequestError(err.message, config, null, FsRequestErrorCode.NETWORK));
});

stream.on('end', () => {
stream.on('end', async () => {
// check if there is any response data inside
if (res.statusCode !== 204) {
// prepare response
response.data = Buffer.concat(responseBuffer);
response = parseResponse(response);
response = await parseResponse(response);
} else {
response.data = null;
}
Expand Down
4 changes: 2 additions & 2 deletions src/lib/request/adapters/xhr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export class XhrAdapter implements AdapterInterface {
request.timeout = config.timeout;

return new Promise<FsResponse>((resolve, reject) => {
request.onreadystatechange = () => {
request.onreadystatechange = async () => {
if (!request || request.readyState !== 4) {
return;
}
Expand All @@ -91,7 +91,7 @@ export class XhrAdapter implements AdapterInterface {
};

request = null;
response = parseResponse(response);
response = await parseResponse(response);

if (500 <= response.status && response.status <= 599) {
// server error throw
Expand Down
Loading

0 comments on commit bf4146d

Please sign in to comment.