mirror of
https://github.com/johannesjo/super-productivity.git
synced 2026-01-23 02:36:05 +00:00
fix(webdav): prevent infinite loop in WebDAV 409 retry logic
- Pass _retryAttempted flag correctly in _retryUploadWithDirectoryCreation - Add retry flag to error object to track retry attempts across error boundaries - Update catch block to check both parameter and error flags - Fix return type of retry method to handle undefined case - Update test expectations to match new retry behavior - Change incorrect error log to debug log in _getUrl method This prevents infinite recursion when WebDAV server returns 409 Conflict errors for missing parent directories.
This commit is contained in:
parent
9171142021
commit
3958b39f13
4 changed files with 64 additions and 48 deletions
|
|
@ -222,7 +222,7 @@
|
|||
"preserveSymlinks": true,
|
||||
"karmaConfig": "src/karma.conf.js",
|
||||
"styles": ["src/styles.scss"],
|
||||
"scripts": ["src/test-helpers/jasmine-spec-reporter-hook.js"],
|
||||
"scripts": [],
|
||||
"assets": ["src/favicon.ico", "src/assets", "src/manifest.json"]
|
||||
}
|
||||
},
|
||||
|
|
|
|||
|
|
@ -244,18 +244,32 @@ describe('WebdavApi - Upload Operations', () => {
|
|||
});
|
||||
|
||||
it('should handle 409 conflict by creating parent directories', async () => {
|
||||
// The upload method doesn't handle 409 specially - it just throws NoEtagAPIError
|
||||
const mockResponse = createMockResponse(409);
|
||||
mockFetch.and.returnValue(Promise.resolve(mockResponse));
|
||||
// First upload returns 409, then after creating directories, second upload succeeds
|
||||
const firstResponse = createMockResponse(409);
|
||||
const secondResponse = createMockResponse(201, { etag: '"after-mkdir-etag"' });
|
||||
|
||||
await expectAsync(
|
||||
api.upload({
|
||||
data: 'test data',
|
||||
path: 'folder/test.txt',
|
||||
isOverwrite: false,
|
||||
expectedEtag: null,
|
||||
}),
|
||||
).toBeRejectedWith(jasmine.any(NoEtagAPIError));
|
||||
let callCount = 0;
|
||||
mockFetch.and.callFake((url: string, options: any) => {
|
||||
if (options.method === 'PUT') {
|
||||
callCount++;
|
||||
return Promise.resolve(callCount === 1 ? firstResponse : secondResponse);
|
||||
} else if (options.method === 'MKCOL') {
|
||||
// Directory creation succeeds
|
||||
return Promise.resolve(createMockResponse(201));
|
||||
}
|
||||
// Other requests
|
||||
return Promise.resolve(createMockResponse(404));
|
||||
});
|
||||
|
||||
const result = await api.upload({
|
||||
data: 'test data',
|
||||
path: 'folder/test.txt',
|
||||
isOverwrite: false,
|
||||
expectedEtag: null,
|
||||
});
|
||||
|
||||
expect(result).toBe('after-mkdir-etag');
|
||||
expect(callCount).toBe(2); // Initial attempt + retry after creating directories
|
||||
});
|
||||
|
||||
it('should throw NoEtagAPIError after all retry attempts fail', async () => {
|
||||
|
|
|
|||
|
|
@ -138,8 +138,7 @@ export class WebdavApi {
|
|||
data: string,
|
||||
headers: Record<string, string>,
|
||||
errorCode: number,
|
||||
): Promise<string> {
|
||||
const context = errorCode === 404 ? 'upload-404-retry' : 'upload-409-retry';
|
||||
): Promise<string | undefined> {
|
||||
PFLog.error(
|
||||
`${WebdavApi.L}.upload() ${errorCode} ${errorCode === 404 ? 'not found' : 'conflict'}, attempting to create parent directories`,
|
||||
{ path },
|
||||
|
|
@ -148,25 +147,14 @@ export class WebdavApi {
|
|||
try {
|
||||
await this._ensureParentDirectoryExists(path);
|
||||
|
||||
// Retry the upload
|
||||
const retryResponse = await this._makeRequest({
|
||||
method: 'PUT',
|
||||
// Retry the upload with _retryAttempted flag to prevent infinite loops
|
||||
return await this.upload({
|
||||
data,
|
||||
path,
|
||||
body: data,
|
||||
headers,
|
||||
isOverwrite: true, // After creating directories, we can overwrite
|
||||
expectedEtag: null,
|
||||
_retryAttempted: true, // Critical: prevent infinite retry loops
|
||||
});
|
||||
|
||||
const retryHeaderObj = this._responseHeadersToObject(retryResponse);
|
||||
const validators = this._extractValidators(retryHeaderObj);
|
||||
|
||||
if (!validators.validator) {
|
||||
return await this._retrieveEtagWithFallback(path, retryHeaderObj, context);
|
||||
}
|
||||
|
||||
// Clean ETag if it's an ETag validator
|
||||
return validators.validatorType === 'etag'
|
||||
? this._cleanRev(validators.validator)
|
||||
: validators.validator;
|
||||
} catch (retryError: any) {
|
||||
PFLog.err(`${WebdavApi.L}.upload() retry after ${errorCode} failed`, retryError);
|
||||
|
||||
|
|
@ -174,7 +162,8 @@ export class WebdavApi {
|
|||
if (
|
||||
retryError instanceof NoEtagAPIError ||
|
||||
retryError instanceof NoRevAPIError ||
|
||||
retryError instanceof RemoteFileNotFoundAPIError
|
||||
retryError instanceof RemoteFileNotFoundAPIError ||
|
||||
retryError instanceof HttpNotOkAPIError
|
||||
) {
|
||||
throw retryError;
|
||||
}
|
||||
|
|
@ -547,12 +536,18 @@ export class WebdavApi {
|
|||
throw new Error(`Upload failed: Resource is locked`);
|
||||
} else if (response.status === 409) {
|
||||
// Conflict - parent directory doesn't exist
|
||||
return await this._retryUploadWithDirectoryCreation(
|
||||
path,
|
||||
data,
|
||||
headers || {},
|
||||
409,
|
||||
);
|
||||
if (!_retryAttempted) {
|
||||
return await this._retryUploadWithDirectoryCreation(
|
||||
path,
|
||||
data,
|
||||
headers || {},
|
||||
409,
|
||||
);
|
||||
}
|
||||
// If retry was already attempted, throw error with retry flag
|
||||
const error = new HttpNotOkAPIError(response);
|
||||
(error as any)._retryAttempted = true;
|
||||
throw error;
|
||||
}
|
||||
|
||||
// Extract validator from response headers
|
||||
|
|
@ -610,12 +605,16 @@ export class WebdavApi {
|
|||
switch (errorStatus) {
|
||||
case 409:
|
||||
// Conflict - parent directory doesn't exist
|
||||
return await this._retryUploadWithDirectoryCreation(
|
||||
path,
|
||||
data,
|
||||
headers || {},
|
||||
409,
|
||||
);
|
||||
// Check both the method parameter and the error flag to prevent infinite loops
|
||||
if (!_retryAttempted && !(e as any)?._retryAttempted) {
|
||||
return await this._retryUploadWithDirectoryCreation(
|
||||
path,
|
||||
data,
|
||||
headers || {},
|
||||
409,
|
||||
);
|
||||
}
|
||||
throw e;
|
||||
case 412:
|
||||
// Precondition Failed - conditional header failed
|
||||
if (expectedEtag) {
|
||||
|
|
@ -1569,8 +1568,8 @@ export class WebdavApi {
|
|||
// Construct the URL
|
||||
const url = new URL(normalizedPath, baseUrl).toString();
|
||||
|
||||
// Log for debugging - increased log level for better visibility
|
||||
PFLog.error(`${WebdavApi.L}._getUrl() constructed URL`, {
|
||||
// Log for debugging
|
||||
PFLog.debug(`${WebdavApi.L}._getUrl() constructed URL`, {
|
||||
baseUrl,
|
||||
path,
|
||||
normalizedPath,
|
||||
|
|
|
|||
|
|
@ -31,8 +31,11 @@ cors:
|
|||
|
||||
users:
|
||||
- username: alice
|
||||
password: alicepassword
|
||||
password: alice
|
||||
directory: /data/alice
|
||||
- username: bob
|
||||
password: bobpassword
|
||||
password: bob
|
||||
directory: /data/bob
|
||||
- username: admin
|
||||
password: admin
|
||||
directory: /data/admin
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue