Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(Node): Do not drop readonly attributes but only forbid updating them #967

Merged
merged 2 commits into from
May 29, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions __tests__/files/node.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -532,6 +532,8 @@ describe('Attributes update', () => {

file.update({
etag: '5678',
'owner-display-name': undefined,
'owner-id': undefined,
})

expect(file.attributes?.etag).toBe('5678')
Expand All @@ -546,16 +548,19 @@ describe('Attributes update', () => {
owner: 'emma',
attributes: {
etag: '1234',
size: 9999,
},
})

file.update({
etag: '5678',
owner: 'admin',
fileid: 1234,
size: undefined,
})

expect(file.attributes?.etag).toBe('5678')
expect(file.attributes?.size).toBe(9999)
expect(file.attributes?.owner).toBeUndefined()
expect(file.attributes?.fileid).toBeUndefined()
})
Expand Down
56 changes: 29 additions & 27 deletions lib/files/node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,22 +44,30 @@
private _attributes: Attribute
private _knownDavService = /(remote|public)\.php\/(web)?dav/i

private readonlyAttributes = Object.entries(Object.getOwnPropertyDescriptors(Node.prototype))
.filter(e => typeof e[1].get === 'function' && e[0] !== '__proto__')
.map(e => e[0])

private handler = {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
set: (target: Attribute, prop: string, value: any): any => {
set: (target: Attribute, prop: string, value: unknown): boolean => {
if (this.readonlyAttributes.includes(prop)) {
return false
}
// Edit modification time
this.updateMtime()
// Apply original changes
return Reflect.set(target, prop, value)
},
deleteProperty: (target: Attribute, prop: string) => {
deleteProperty: (target: Attribute, prop: string): boolean => {
if (this.readonlyAttributes.includes(prop)) {
return false
}
// Edit modification time
this.updateMtime()
// Apply original changes
return Reflect.deleteProperty(target, prop)
},
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} as ProxyHandler<any>
} as ProxyHandler<Attribute>

constructor(data: NodeData, davService?: RegExp) {
// Validate data
Expand All @@ -68,8 +76,7 @@
this._data = data

// Proxy the attributes to update the mtime on change
// eslint-disable-next-line @typescript-eslint/no-explicit-any
this._attributes = new Proxy(this.cleanAttributes(data.attributes || {}) as any, this.handler)
this._attributes = new Proxy(data.attributes || {}, this.handler)
delete this._data.attributes

if (davService) {
Expand Down Expand Up @@ -334,28 +341,23 @@
* @param attributes The new attributes to update
*/
update(attributes: Attribute) {
// Proxy the attributes to update the mtime on change
// eslint-disable-next-line @typescript-eslint/no-explicit-any
this._attributes = new Proxy(this.cleanAttributes(attributes) as any, this.handler)
}

private cleanAttributes(attributes: Attribute): Attribute {
const getters = Object.entries(Object.getOwnPropertyDescriptors(Node.prototype))
.filter(e => typeof e[1].get === 'function' && e[0] !== '__proto__')
.map(e => e[0])

// Remove protected getters from the attributes
// you cannot update them
const clean: Attribute = {}
for (const key in attributes) {
if (getters.includes(key)) {
logger.debug(`Discarding protected attribute ${key}`, { node: this, attributes })
continue
for (const [name, value] of Object.entries(attributes)) {
try {
if (value === undefined) {
delete this.attributes[name]
} else {
this.attributes[name] = value
}
} catch (e) {
// Ignore but warn on readonly attributes
if (e instanceof TypeError) {
logger.warn(`Cannot update readonly attribute ${name} on Node`)
} else {
// Throw all other exceptions
throw e

Check warning on line 357 in lib/files/node.ts

View check run for this annotation

Codecov / codecov/patch

lib/files/node.ts#L357

Added line #L357 was not covered by tests
}
}
clean[key] = attributes[key]
}

return clean
}

}
Loading