Skip to content

Commit ab361fc

Browse files
zxdong262zxdong262
committed
Fix binary trzsz transfer
1 parent c3d0083 commit ab361fc

2 files changed

Lines changed: 256 additions & 2 deletions

File tree

‎src/app/server/trzsz.js‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,20 @@ class FileWriter {
118118

119119
async writeFile (buf) {
120120
this._ensureStream()
121-
const canContinue = this.writeStream.write(buf)
121+
// Never hand the caller's buffer straight to the stream.
122+
//
123+
// In binary mode (tsz -b) the remote announces `escape_chars: []`, so
124+
// TrzszBuffer.readBinary() returns a view over its own reusable arrBuf
125+
// and unescapeData() passes that view through untouched. A WriteStream
126+
// keeps the reference until the data actually reaches the file, so the
127+
// very next protocol read (the next `#DATA:<len>\n` header, or the
128+
// trailing `#MD5:` line) rewrites arrBuf offset 0 while this chunk is
129+
// still queued — and the file on disk comes out with protocol text
130+
// spliced into it. Copy so the bytes we queued are ours.
131+
const chunk = Buffer.from(
132+
buf instanceof ArrayBuffer ? new Uint8Array(buf) : buf
133+
)
134+
const canContinue = this.writeStream.write(chunk)
122135
if (!canContinue) {
123136
if (!this._drainPromise) {
124137
this._drainPromise = new Promise((resolve) => {
@@ -736,4 +749,9 @@ class TrzszManager {
736749
}
737750
}
738751
const trzszManager = new TrzszManager()
739-
module.exports = { trzszManager }
752+
module.exports = {
753+
TrzszSession,
754+
TrzszManager,
755+
trzszManager,
756+
TRZSZ_STATE
757+
}

‎src/test/unit-ci/trzsz.spec.js‎

Lines changed: 236 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,236 @@
1+
/**
2+
* Unit tests for the server-side trzsz handler (src/app/server/trzsz.js)
3+
*
4+
* The TrzszSession is driven end-to-end against a real trzsz2 TrzszTransfer
5+
* acting as the remote peer, wired back-to-back over an in-memory "wire"
6+
* (the wire copies, exactly like a pty or a socket does). The peer speaks
7+
* the real protocol (ACT/CFG/NUM/NAME/SIZE/DATA/MD5), so the tests exercise
8+
* the actual state machines rather than mocks of them.
9+
*
10+
* The download tests exist because of a silent-corruption bug: in binary
11+
* mode (tsz -b) the remote announces `binary: true, escape_chars: []`, and
12+
* then the chain
13+
*
14+
* TrzszBuffer.readBinary() -> view over the *reusable* this.arrBuf
15+
* unescapeData(data, []) -> returns that view unchanged
16+
* FileWriter.writeFile() -> hands the view to fs.WriteStream.write()
17+
*
18+
* means the very next protocol read (the next `#DATA:<len>\n` header, or the
19+
* trailing `#MD5:` line) writes into arrBuf offset 0 while the previous
20+
* chunk is still queued for disk, so the file on disk ends up with header
21+
* text spliced into it. The MD5 handshake still passes, because trzsz2 hashes
22+
* the view before the next read mutates it. Hence: "download says complete,
23+
* file is corrupt".
24+
*/
25+
26+
process.env.NODE_ENV = 'development'
27+
28+
const { test, describe, beforeEach, afterEach } = require('node:test')
29+
const assert = require('node:assert/strict')
30+
const fs = require('node:fs')
31+
const os = require('node:os')
32+
const path = require('node:path')
33+
const { TrzszTransfer } = require('trzsz2')
34+
35+
const { TrzszSession } = require('../../../src/app/server/trzsz')
36+
37+
// ─── wire harness ───────────────────────────────────────────────────────────
38+
39+
// ::TRZSZ:TRANSFER:<direction>:<version>:<unique id>:<port>
40+
// 'S' (0x53) = receive, 'R' (0x52) = send
41+
const TRZSZ_RECEIVE_MAGIC = Buffer.from('::TRZSZ:TRANSFER:S:1.1.7:testunique:34567\r\n')
42+
43+
/**
44+
* The wire copies bytes, like a pty read or a socket write does. That copy
45+
* is what makes the receiver-side aliasing bug reachable: the socket is not
46+
* the thing that pins the buffer, the pending fs write is.
47+
*/
48+
function toU8 (data) {
49+
if (typeof data === 'string') return Buffer.from(data, 'latin1')
50+
return Buffer.from(data)
51+
}
52+
53+
/**
54+
* Connect a real TrzszSession (receiver role) to a real TrzszTransfer
55+
* (sender role) over an in-memory wire.
56+
*
57+
* session.term.write -> peer.buffer (our protocol replies)
58+
* peer.writer -> session.handleData (remote protocol messages)
59+
*/
60+
function makePair () {
61+
const events = []
62+
const holder = { peer: null }
63+
const term = {
64+
write: (data) => holder.peer.addReceivedData(toU8(data))
65+
}
66+
const ws = {
67+
s: (msg) => events.push(msg),
68+
send: () => {}
69+
}
70+
const session = new TrzszSession(term, ws)
71+
const peer = new TrzszTransfer(
72+
(data) => session.handleData(toU8(data)),
73+
false
74+
)
75+
holder.peer = peer
76+
return { session, peer, events }
77+
}
78+
79+
/**
80+
* A stand-in for the remote's file source. Allocates a fresh buffer on every
81+
* read on purpose: the peer must not alias its own source, otherwise a
82+
* sender-side aliasing bug would be confused with the receiver-side one
83+
* these tests are about.
84+
*/
85+
function makeRemoteReader (name, data) {
86+
let offset = 0
87+
return {
88+
getRelPath: () => [name],
89+
getPathId: () => 0,
90+
isDir: () => false,
91+
getSize: () => data.length,
92+
readFile: async (buffer) => {
93+
const size = Math.min(buffer.byteLength, data.length - offset)
94+
const out = new Uint8Array(size)
95+
out.set(data.subarray(offset, offset + size))
96+
offset += size
97+
return out
98+
},
99+
closeFile: async () => {}
100+
}
101+
}
102+
103+
/**
104+
* Deterministic, printable payload (0x20..0x7e) so a failure diff is
105+
* readable and so stray control bytes never confuse the protocol framing.
106+
*/
107+
function makePattern (size) {
108+
const buf = Buffer.allocUnsafe(size)
109+
for (let i = 0; i < size; i++) {
110+
buf[i] = 0x20 + (i % 0x5f)
111+
}
112+
return buf
113+
}
114+
115+
/** Wait until `cond()` is true, or fail loudly. */
116+
async function waitFor (cond, label, timeoutMs = 4000) {
117+
const deadline = Date.now() + timeoutMs
118+
while (Date.now() < deadline) {
119+
if (cond()) return
120+
await new Promise((resolve) => setTimeout(resolve, 5))
121+
}
122+
throw new Error(`timed out waiting for ${label}`)
123+
}
124+
125+
/** Wait until every FileWriter has flushed and closed its stream. */
126+
async function waitForFlush (session) {
127+
await waitFor(
128+
() => session.fileWriters.every((w) => w.writeStream === null),
129+
'file writers to flush'
130+
)
131+
}
132+
133+
/**
134+
* Byte comparison with a compact, useful failure message: how many bytes
135+
* differ, where the first one is, and what is actually on disk there.
136+
*/
137+
function assertByteExact (actual, expected, label) {
138+
assert.equal(actual.length, expected.length, `${label}: size mismatch`)
139+
let count = 0
140+
let first = -1
141+
for (let i = 0; i < actual.length; i++) {
142+
if (actual[i] !== expected[i]) {
143+
if (first < 0) first = i
144+
count++
145+
}
146+
}
147+
if (first < 0) return
148+
const around = actual.subarray(first, first + 24).toString('latin1')
149+
assert.fail(
150+
`${label}: ${count} of ${actual.length} bytes differ; ` +
151+
`first at offset ${first}, got ${JSON.stringify(around)}`
152+
)
153+
}
154+
155+
/**
156+
* Drive the remote sender: consume our ACT, announce the transfer config,
157+
* then push one file.
158+
*/
159+
async function runPeer (peer, reader, config) {
160+
await peer.recvAction()
161+
await peer.sendConfig(config, [], undefined, 0)
162+
return peer.sendFiles([reader])
163+
}
164+
165+
/** Receive one file and return the bytes that landed on disk. */
166+
async function receiveFile (session, peer, events, { name, src, config }) {
167+
const dest = path.join(tmpDir, name)
168+
session.setSavePath(tmpDir)
169+
const done = runPeer(peer, makeRemoteReader(name, src), config)
170+
session.handleData(TRZSZ_RECEIVE_MAGIC)
171+
await done
172+
await waitForFlush(session)
173+
// The real remote prints "Success" once it has saved the file; that text is
174+
// what makes the session deliver session-complete and tear itself down.
175+
await waitFor(() => session._pendingComplete !== null, 'pending completion')
176+
session.handleData(Buffer.from('Success'))
177+
assert.ok(events.some((e) => e.event === 'session-complete'), 'session-complete')
178+
return fs.readFileSync(dest)
179+
}
180+
181+
// ─── environment ────────────────────────────────────────────────────────────
182+
183+
let tmpDir
184+
185+
describe('trzsz session', () => {
186+
beforeEach(() => {
187+
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'trzsz-test-'))
188+
})
189+
190+
afterEach(() => {
191+
fs.rmSync(tmpDir, { recursive: true, force: true })
192+
})
193+
194+
describe('download', () => {
195+
test('binary mode (tsz -b, empty escape_chars) writes a byte-exact file', async () => {
196+
const { session, peer, events } = makePair()
197+
// bufsize small so the sender's chunk size stops growing quickly, which
198+
// is the regime a real multi-MB download spends all its time in: the
199+
// sender reuses one buffer, and so does TrzszBuffer.arrBuf.
200+
const config = { binary: true, overwrite: true, bufsize: 4096, timeout: 5 }
201+
const src = makePattern(64 * 1024)
202+
const actual = await receiveFile(session, peer, events, {
203+
name: 'payload.bin',
204+
src,
205+
config
206+
})
207+
assertByteExact(actual, src, 'binary-mode download')
208+
})
209+
210+
test('base64 mode writes a byte-exact file', async () => {
211+
const { session, peer, events } = makePair()
212+
const config = { overwrite: true, bufsize: 4096, timeout: 5 }
213+
const src = makePattern(64 * 1024)
214+
const actual = await receiveFile(session, peer, events, {
215+
name: 'payload.txt',
216+
src,
217+
config
218+
})
219+
assertByteExact(actual, src, 'base64-mode download')
220+
})
221+
222+
test('binary mode survives a payload full of protocol-looking bytes', async () => {
223+
const { session, peer, events } = makePair()
224+
const config = { binary: true, overwrite: true, bufsize: 4096, timeout: 5 }
225+
// '#DATA:2048\n' repeated: in binary mode with no escape chars this is
226+
// just payload, and it must come out the other end untouched.
227+
const src = Buffer.from('#DATA:2048\n'.repeat(6000))
228+
const actual = await receiveFile(session, peer, events, {
229+
name: 'tricky.bin',
230+
src,
231+
config
232+
})
233+
assertByteExact(actual, src, 'binary-mode download (protocol-like payload)')
234+
})
235+
})
236+
})

0 commit comments

Comments
 (0)