Skip to content

Commit 700cc56

Browse files
streamichstreamich
committed
fix: 🐛 encode change info in REMOVE and RENAME
1 parent a9444f8 commit 700cc56

File tree

4 files changed

+125
-138
lines changed

4 files changed

+125
-138
lines changed

‎src/nfs/v4/server/__demos__/tcp-server.ts‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,14 @@
11
import {Nfsv4OperationsNode} from '../operations/node/Nfsv4OperationsNode';
22
import {Nfsv4TcpServer} from '../Nfsv4TcpServer';
33
import {fs, vol} from 'memfs';
4+
// import * as fs from 'fs';
45

6+
// const dir = __dirname + '/mnt';
7+
// if (!fs.existsSync(dir)) fs.mkdirSync(dir);
8+
// if (!fs.existsSync(dir + '/export')) fs.mkdirSync(dir + '/export');
9+
// if (!fs.existsSync(dir + '/export/file.txt')) fs.writeFileSync(dir + '/export/file.txt', 'Hello, NFS v4!\n');
10+
11+
const dir = '/';
512
vol.fromJSON({
613
'/export': null,
714
'/export/file.txt': 'Hello, NFS v4!\n',
@@ -10,5 +17,5 @@ vol.fromJSON({
1017
// tslint:disable-next-line:no-console
1118
console.log(vol.toJSON());
1219

13-
const ops = new Nfsv4OperationsNode({fs: <any>fs, dir: '/'});
20+
const ops = new Nfsv4OperationsNode({fs: <any>fs, dir});
1421
Nfsv4TcpServer.start({ops, debug: true});

‎src/nfs/v4/server/operations/node/Nfsv4OperationsNode.ts‎

Lines changed: 18 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,13 @@ export class Nfsv4OperationsNode implements Nfsv4Operations {
9797
/** Map from lock-owner key to lock-owner state. */
9898
protected lockOwners: Map<string, LockOwnerState> = new Map();
9999

100+
/**
101+
* Server-wide monotonic change counter for directory change_info.
102+
* Incremented on every mutating operation (RENAME, REMOVE, CREATE, etc.).
103+
* Used to populate change_info4 before/after values for client cache validation.
104+
*/
105+
protected changeCounter: bigint = 0n;
106+
100107
constructor(opts: Nfsv4OperationsNodeOpts) {
101108
this.fs = opts.fs;
102109
this.promises = this.fs.promises;
@@ -1142,17 +1149,12 @@ export class Nfsv4OperationsNode implements Nfsv4Operations {
11421149
await this.promises.rmdir(targetPath);
11431150
} else {
11441151
await this.promises.unlink(targetPath);
1145-
const appleDoublePath = NodePath.join(NodePath.dirname(targetPath), '._' + NodePath.basename(targetPath));
1146-
try {
1147-
const appleDoubleStats = await this.promises.stat(appleDoublePath);
1148-
if (appleDoubleStats.isFile()) {
1149-
await this.promises.unlink(appleDoublePath);
1150-
}
1151-
} catch (err) {
1152-
if (!isErrCode('ENOENT', err)) throw err;
1153-
}
11541152
}
1155-
return new msg.Nfsv4RemoveResponse(Nfsv4Stat.NFS4_OK);
1153+
const before = this.changeCounter;
1154+
const after = this.changeCounter++;
1155+
const cinfo = new struct.Nfsv4ChangeInfo(true, before, after);
1156+
const resok = new msg.Nfsv4RemoveResOk(cinfo);
1157+
return new msg.Nfsv4RemoveResponse(Nfsv4Stat.NFS4_OK, resok);
11561158
} catch (err: unknown) {
11571159
const status = normalizeNodeFsError(err, ctx.connection.logger);
11581160
return new msg.Nfsv4RemoveResponse(status);
@@ -1181,18 +1183,12 @@ export class Nfsv4OperationsNode implements Nfsv4Operations {
11811183
try {
11821184
await this.promises.rename(oldPath, newPath);
11831185
this.fh.rename(oldPath, newPath);
1184-
const oldAppleDouble = NodePath.join(NodePath.dirname(oldPath), '._' + NodePath.basename(oldPath));
1185-
const newAppleDouble = NodePath.join(NodePath.dirname(newPath), '._' + NodePath.basename(newPath));
1186-
try {
1187-
const stats = await this.promises.stat(oldAppleDouble);
1188-
if (stats.isFile()) {
1189-
await this.promises.rename(oldAppleDouble, newAppleDouble);
1190-
this.fh.rename(oldAppleDouble, newAppleDouble);
1191-
}
1192-
} catch (err) {
1193-
if (!isErrCode('ENOENT', err)) throw err;
1194-
}
1195-
return new msg.Nfsv4RenameResponse(Nfsv4Stat.NFS4_OK);
1186+
const before = this.changeCounter;
1187+
const after = this.changeCounter++;
1188+
const sourceCinfo = new struct.Nfsv4ChangeInfo(true, before, after);
1189+
const targetCinfo = new struct.Nfsv4ChangeInfo(true, before, after);
1190+
const resok = new msg.Nfsv4RenameResOk(sourceCinfo, targetCinfo);
1191+
return new msg.Nfsv4RenameResponse(Nfsv4Stat.NFS4_OK, resok);
11961192
} catch (err: unknown) {
11971193
if (isErrCode('EXDEV', err)) return new msg.Nfsv4RenameResponse(Nfsv4Stat.NFS4ERR_XDEV);
11981194
const status = normalizeNodeFsError(err, ctx.connection.logger);

‎src/nfs/v4/server/operations/node/__tests__/REMOVE.spec.ts‎

Lines changed: 48 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import {setupNfsClientServerTestbed} from '../../../__tests__/setup';
22
import {nfs} from '../../../../builder';
33
import {Nfsv4Stat} from '../../../../constants';
4+
import * as msg from '../../../../messages';
45

56
describe('REMOVE operation', () => {
67
test('remove a file succeeds', async () => {
@@ -19,47 +20,65 @@ describe('REMOVE operation', () => {
1920
await stop();
2021
});
2122

22-
describe('AppleDouble file handling', () => {
23-
test('removes AppleDouble file when removing main file', async () => {
23+
describe('change_info semantics', () => {
24+
test('returns before < after on successful remove', async () => {
2425
const {client, stop, vol} = await setupNfsClientServerTestbed();
2526
vol.writeFileSync('/export/test.txt', 'data');
26-
vol.writeFileSync('/export/._test.txt', 'xattr-data');
2727
const res = await client.compound([nfs.PUTROOTFH(), nfs.REMOVE('test.txt')]);
2828
expect(res.status).toBe(Nfsv4Stat.NFS4_OK);
29-
expect(vol.existsSync('/export/test.txt')).toBe(false);
30-
expect(vol.existsSync('/export/._test.txt')).toBe(false);
29+
const removeRes = res.resarray[1] as msg.Nfsv4RemoveResponse;
30+
expect(removeRes.status).toBe(Nfsv4Stat.NFS4_OK);
31+
if (removeRes.status === Nfsv4Stat.NFS4_OK && removeRes.resok) {
32+
const cinfo = removeRes.resok.cinfo;
33+
expect(cinfo.atomic).toBe(true);
34+
expect(cinfo.after).toBeGreaterThan(cinfo.before);
35+
expect(cinfo.after - cinfo.before).toBe(1n);
36+
}
3137
await stop();
3238
});
3339

34-
test('succeeds even if AppleDouble file does not exist', async () => {
40+
test('change counter increments across multiple removes', async () => {
3541
const {client, stop, vol} = await setupNfsClientServerTestbed();
36-
vol.writeFileSync('/export/test.txt', 'data');
37-
const res = await client.compound([nfs.PUTROOTFH(), nfs.REMOVE('test.txt')]);
38-
expect(res.status).toBe(Nfsv4Stat.NFS4_OK);
39-
expect(vol.existsSync('/export/test.txt')).toBe(false);
42+
vol.writeFileSync('/export/file1.txt', 'data1');
43+
vol.writeFileSync('/export/file2.txt', 'data2');
44+
const res1 = await client.compound([nfs.PUTROOTFH(), nfs.REMOVE('file1.txt')]);
45+
expect(res1.status).toBe(Nfsv4Stat.NFS4_OK);
46+
const removeRes1 = res1.resarray[1] as msg.Nfsv4RemoveResponse;
47+
const res2 = await client.compound([nfs.PUTROOTFH(), nfs.REMOVE('file2.txt')]);
48+
expect(res2.status).toBe(Nfsv4Stat.NFS4_OK);
49+
const removeRes2 = res2.resarray[1] as msg.Nfsv4RemoveResponse;
50+
if (
51+
removeRes1.status === Nfsv4Stat.NFS4_OK &&
52+
removeRes1.resok &&
53+
removeRes2.status === Nfsv4Stat.NFS4_OK &&
54+
removeRes2.resok
55+
) {
56+
expect(removeRes2.resok.cinfo.after).toBeGreaterThan(removeRes1.resok.cinfo.after);
57+
expect(removeRes2.resok.cinfo.before).toBe(removeRes1.resok.cinfo.after);
58+
}
4059
await stop();
4160
});
4261

43-
test('does not remove AppleDouble if it is a directory', async () => {
62+
test('failed remove does not increment change counter', async () => {
4463
const {client, stop, vol} = await setupNfsClientServerTestbed();
45-
vol.writeFileSync('/export/test.txt', 'data');
46-
vol.mkdirSync('/export/._test.txt');
47-
const res = await client.compound([nfs.PUTROOTFH(), nfs.REMOVE('test.txt')]);
48-
expect(res.status).toBe(Nfsv4Stat.NFS4_OK);
49-
expect(vol.existsSync('/export/test.txt')).toBe(false);
50-
expect(vol.existsSync('/export/._test.txt')).toBe(true);
51-
expect(vol.statSync('/export/._test.txt').isDirectory()).toBe(true);
52-
await stop();
53-
});
54-
55-
test('removes directory without removing AppleDouble file', async () => {
56-
const {client, stop, vol} = await setupNfsClientServerTestbed();
57-
vol.mkdirSync('/export/testdir');
58-
vol.writeFileSync('/export/._testdir', 'xattr');
59-
const res = await client.compound([nfs.PUTROOTFH(), nfs.REMOVE('testdir')]);
60-
expect(res.status).toBe(Nfsv4Stat.NFS4_OK);
61-
expect(vol.existsSync('/export/testdir')).toBe(false);
62-
expect(vol.existsSync('/export/._testdir')).toBe(true);
64+
vol.writeFileSync('/export/existing.txt', 'data');
65+
const res1 = await client.compound([nfs.PUTROOTFH(), nfs.REMOVE('existing.txt')]);
66+
expect(res1.status).toBe(Nfsv4Stat.NFS4_OK);
67+
const removeRes1 = res1.resarray[1] as msg.Nfsv4RemoveResponse;
68+
const res2 = await client.compound([nfs.PUTROOTFH(), nfs.REMOVE('nonexistent.txt')]);
69+
expect(res2.status).not.toBe(Nfsv4Stat.NFS4_OK);
70+
vol.writeFileSync('/export/another.txt', 'data');
71+
const res3 = await client.compound([nfs.PUTROOTFH(), nfs.REMOVE('another.txt')]);
72+
expect(res3.status).toBe(Nfsv4Stat.NFS4_OK);
73+
const removeRes3 = res3.resarray[1] as msg.Nfsv4RemoveResponse;
74+
if (
75+
removeRes1.status === Nfsv4Stat.NFS4_OK &&
76+
removeRes1.resok &&
77+
removeRes3.status === Nfsv4Stat.NFS4_OK &&
78+
removeRes3.resok
79+
) {
80+
expect(removeRes3.resok.cinfo.after - removeRes1.resok.cinfo.after).toBe(1n);
81+
}
6382
await stop();
6483
});
6584
});

‎src/nfs/v4/server/operations/node/__tests__/RENAME.spec.ts‎

Lines changed: 51 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -121,103 +121,68 @@ describe('RENAME operation', () => {
121121
await stop();
122122
});
123123

124-
describe('AppleDouble file handling', () => {
125-
test('renames AppleDouble file when renaming main file', async () => {
124+
describe('change_info semantics', () => {
125+
test('returns before < after on successful rename', async () => {
126126
const {client, stop, vol} = await setupNfsClientServerTestbed();
127-
vol.writeFileSync('/export/test.txt', 'data');
128-
vol.writeFileSync('/export/._test.txt', 'xattr-data');
129-
const res = await client.compound([nfs.PUTROOTFH(), nfs.SAVEFH(), nfs.RENAME('test.txt', 'new.txt')]);
127+
vol.writeFileSync('/export/old.txt', 'data');
128+
const res = await client.compound([nfs.PUTROOTFH(), nfs.SAVEFH(), nfs.RENAME('old.txt', 'new.txt')]);
130129
expect(res.status).toBe(Nfsv4Stat.NFS4_OK);
131-
expect(vol.existsSync('/export/new.txt')).toBe(true);
132-
expect(vol.existsSync('/export/._new.txt')).toBe(true);
133-
expect(vol.existsSync('/export/test.txt')).toBe(false);
134-
expect(vol.existsSync('/export/._test.txt')).toBe(false);
135-
await stop();
136-
});
137-
138-
test('succeeds even if AppleDouble file does not exist', async () => {
139-
const {client, stop, vol} = await setupNfsClientServerTestbed();
140-
vol.writeFileSync('/export/test.txt', 'data');
141-
const res = await client.compound([nfs.PUTROOTFH(), nfs.SAVEFH(), nfs.RENAME('test.txt', 'new.txt')]);
142-
expect(res.status).toBe(Nfsv4Stat.NFS4_OK);
143-
expect(vol.existsSync('/export/new.txt')).toBe(true);
144-
expect(vol.existsSync('/export/._new.txt')).toBe(false);
145-
await stop();
146-
});
147-
148-
test('renames AppleDouble file for TextEdit-style save workflow', async () => {
149-
const {client, stop, vol} = await setupNfsClientServerTestbed();
150-
vol.writeFileSync('/export/file.txt', 'original content');
151-
vol.writeFileSync('/export/file.txt.sb-temp-u9VvVu', 'new content');
152-
vol.writeFileSync('/export/._file.txt.sb-temp-u9VvVu', 'new xattr');
153-
const res = await client.compound([
154-
nfs.PUTROOTFH(),
155-
nfs.SAVEFH(),
156-
nfs.RENAME('file.txt.sb-temp-u9VvVu', 'file.txt'),
157-
]);
158-
expect(res.status).toBe(Nfsv4Stat.NFS4_OK);
159-
expect(vol.existsSync('/export/file.txt')).toBe(true);
160-
expect(vol.existsSync('/export/._file.txt')).toBe(true);
161-
expect(vol.existsSync('/export/file.txt.sb-temp-u9VvVu')).toBe(false);
162-
expect(vol.existsSync('/export/._file.txt.sb-temp-u9VvVu')).toBe(false);
163-
expect(vol.readFileSync('/export/file.txt', 'utf8')).toBe('new content');
164-
expect(vol.readFileSync('/export/._file.txt', 'utf8')).toBe('new xattr');
165-
await stop();
166-
});
167-
168-
test('handles AppleDouble file with long filenames (ID-type FH)', async () => {
169-
const {client, stop, vol} = await setupNfsClientServerTestbed();
170-
const oldName = 'long_filename_' + 'x'.repeat(100) + '.txt';
171-
const newName = 'long_filename_' + 'y'.repeat(100) + '.txt';
172-
vol.writeFileSync('/export/' + oldName, 'data');
173-
vol.writeFileSync('/export/._' + oldName, 'xattr');
174-
const res = await client.compound([nfs.PUTROOTFH(), nfs.SAVEFH(), nfs.RENAME(oldName, newName)]);
175-
expect(res.status).toBe(Nfsv4Stat.NFS4_OK);
176-
expect(vol.existsSync('/export/' + newName)).toBe(true);
177-
expect(vol.existsSync('/export/._' + newName)).toBe(true);
178-
expect(vol.existsSync('/export/' + oldName)).toBe(false);
179-
expect(vol.existsSync('/export/._' + oldName)).toBe(false);
180-
await stop();
181-
});
182-
183-
test('AppleDouble file handle remains valid after rename', async () => {
184-
const {client, stop, vol} = await setupNfsClientServerTestbed();
185-
const oldName = 'original_' + 'a'.repeat(100) + '.txt';
186-
const newName = 'renamed_' + 'b'.repeat(100) + '.txt';
187-
vol.writeFileSync('/export/' + oldName, 'data');
188-
vol.writeFileSync('/export/._' + oldName, 'xattr');
189-
const lookupRes = await client.compound([nfs.PUTROOTFH(), nfs.LOOKUP('._' + oldName), nfs.GETFH()]);
190-
expect(lookupRes.status).toBe(Nfsv4Stat.NFS4_OK);
191-
const appleDoubleFh = (lookupRes.resarray[2] as msg.Nfsv4GetfhResponse).resok!.object;
192-
const renameRes = await client.compound([nfs.PUTROOTFH(), nfs.SAVEFH(), nfs.RENAME(oldName, newName)]);
130+
const renameRes = res.resarray[2] as msg.Nfsv4RenameResponse;
193131
expect(renameRes.status).toBe(Nfsv4Stat.NFS4_OK);
194-
const getAttrRes = await client.compound([nfs.PUTFH(appleDoubleFh), nfs.GETATTR([0x00000020])]);
195-
expect(getAttrRes.status).toBe(Nfsv4Stat.NFS4_OK);
132+
if (renameRes.status === Nfsv4Stat.NFS4_OK && renameRes.resok) {
133+
const sourceCinfo = renameRes.resok.sourceCinfo;
134+
const targetCinfo = renameRes.resok.targetCinfo;
135+
expect(sourceCinfo.atomic).toBe(true);
136+
expect(targetCinfo.atomic).toBe(true);
137+
expect(sourceCinfo.after).toBeGreaterThan(sourceCinfo.before);
138+
expect(targetCinfo.after).toBeGreaterThan(targetCinfo.before);
139+
expect(sourceCinfo.after - sourceCinfo.before).toBe(1n);
140+
}
196141
await stop();
197142
});
198143

199-
test('does not rename AppleDouble if it is a directory', async () => {
144+
test('change counter increments across multiple renames', async () => {
200145
const {client, stop, vol} = await setupNfsClientServerTestbed();
201-
vol.writeFileSync('/export/test.txt', 'data');
202-
vol.mkdirSync('/export/._test.txt');
203-
const res = await client.compound([nfs.PUTROOTFH(), nfs.SAVEFH(), nfs.RENAME('test.txt', 'new.txt')]);
204-
expect(res.status).toBe(Nfsv4Stat.NFS4_OK);
205-
expect(vol.existsSync('/export/new.txt')).toBe(true);
206-
expect(vol.existsSync('/export/._test.txt')).toBe(true);
207-
expect(vol.statSync('/export/._test.txt').isDirectory()).toBe(true);
146+
vol.writeFileSync('/export/file1.txt', 'data1');
147+
vol.writeFileSync('/export/file2.txt', 'data2');
148+
const res1 = await client.compound([nfs.PUTROOTFH(), nfs.SAVEFH(), nfs.RENAME('file1.txt', 'renamed1.txt')]);
149+
expect(res1.status).toBe(Nfsv4Stat.NFS4_OK);
150+
const renameRes1 = res1.resarray[2] as msg.Nfsv4RenameResponse;
151+
const res2 = await client.compound([nfs.PUTROOTFH(), nfs.SAVEFH(), nfs.RENAME('file2.txt', 'renamed2.txt')]);
152+
expect(res2.status).toBe(Nfsv4Stat.NFS4_OK);
153+
const renameRes2 = res2.resarray[2] as msg.Nfsv4RenameResponse;
154+
if (
155+
renameRes1.status === Nfsv4Stat.NFS4_OK &&
156+
renameRes1.resok &&
157+
renameRes2.status === Nfsv4Stat.NFS4_OK &&
158+
renameRes2.resok
159+
) {
160+
expect(renameRes2.resok.sourceCinfo.after).toBeGreaterThan(renameRes1.resok.sourceCinfo.after);
161+
expect(renameRes2.resok.sourceCinfo.before).toBe(renameRes1.resok.sourceCinfo.after);
162+
}
208163
await stop();
209164
});
210165

211-
test('handles case where main file starts with ._', async () => {
166+
test('failed rename does not increment change counter', async () => {
212167
const {client, stop, vol} = await setupNfsClientServerTestbed();
213-
vol.writeFileSync('/export/._special.txt', 'data');
214-
vol.writeFileSync('/export/._._special.txt', 'xattr');
215-
const res = await client.compound([nfs.PUTROOTFH(), nfs.SAVEFH(), nfs.RENAME('._special.txt', '._renamed.txt')]);
216-
expect(res.status).toBe(Nfsv4Stat.NFS4_OK);
217-
expect(vol.existsSync('/export/._renamed.txt')).toBe(true);
218-
expect(vol.existsSync('/export/._._renamed.txt')).toBe(true);
219-
expect(vol.existsSync('/export/._special.txt')).toBe(false);
220-
expect(vol.existsSync('/export/._._special.txt')).toBe(false);
168+
vol.writeFileSync('/export/existing.txt', 'data');
169+
const res1 = await client.compound([nfs.PUTROOTFH(), nfs.SAVEFH(), nfs.RENAME('existing.txt', 'renamed.txt')]);
170+
expect(res1.status).toBe(Nfsv4Stat.NFS4_OK);
171+
const renameRes1 = res1.resarray[2] as msg.Nfsv4RenameResponse;
172+
const res2 = await client.compound([nfs.PUTROOTFH(), nfs.SAVEFH(), nfs.RENAME('nonexistent.txt', 'fail.txt')]);
173+
expect(res2.status).not.toBe(Nfsv4Stat.NFS4_OK);
174+
vol.writeFileSync('/export/another.txt', 'data');
175+
const res3 = await client.compound([nfs.PUTROOTFH(), nfs.SAVEFH(), nfs.RENAME('another.txt', 'renamed3.txt')]);
176+
expect(res3.status).toBe(Nfsv4Stat.NFS4_OK);
177+
const renameRes3 = res3.resarray[2] as msg.Nfsv4RenameResponse;
178+
if (
179+
renameRes1.status === Nfsv4Stat.NFS4_OK &&
180+
renameRes1.resok &&
181+
renameRes3.status === Nfsv4Stat.NFS4_OK &&
182+
renameRes3.resok
183+
) {
184+
expect(renameRes3.resok.sourceCinfo.after - renameRes1.resok.sourceCinfo.after).toBe(1n);
185+
}
221186
await stop();
222187
});
223188
});

0 commit comments

Comments
 (0)