Fix path separator used by normalizeTarEntry() under Windows.

normalizeTarEntry() now also removes trailing slashes, reason why
the change-type is major.

Connects-to: balena-io/balena-cli#1133
Change-type: major
Signed-off-by: Paulo Castro <paulo@balena.io>

lib/index.ts

@@ -20,6 +20,20 @@ import * as tar from 'tar-stream';
 import { Readable, Writable } from 'stream';
 
 const noop = () => {};
+const pathSepRE = new RegExp(escapeRE(path.sep), 'g');
+const normalize: (p: string) => string =
+	path.sep === '/' ? path.normalize : normalizeNonPosix;
+
+/**
+ * Alternative path.normalize() function that uses '/' as a path separator in
+ * non-POSIX systems like Windows (which uses a backslash). This is useful for
+ * tar files/streams as the path separator should always be '/', regardless of
+ * platform/OS, as per tar spec:
+ * https://www.gnu.org/software/tar/manual/html_node/Standard.html
+ */
+function normalizeNonPosix(p: string): string {
+	return path.normalize(p).replace(pathSepRE, '/');
+}
 
 /**
  * normalizeTarEntry: Depending on how the tar archive was created,
@@ -29,13 +43,24 @@ const noop = () => {};
  *  * /Dockerfile -> Dockerfile
  *  * Dockerfile -> Dockerfile
  *  * ./a/b/Dockerfile -> a/b/Dockerfile
+ *  * foo/bar/ -> foo/bar/
+ *  * foo/bar/ -> foo/bar
+ * See additional input/output examples in tests.ts
  */
 export function normalizeTarEntry(name: string): string {
-	const normalized = path.normalize(name);
-	if (path.isAbsolute(normalized)) {
-		return normalized.substr(normalized.indexOf(path.sep) + 1);
+	if (!name) {
+		return '';
+	}
+	let normalized = normalize(name);
+	// remove any leading or trailing slashes
+	if (normalized.startsWith('/')) {
+		normalized = normalized.substr(1);
 	}
-	return normalized;
+	if (normalized.endsWith('/')) {
+		normalized = normalized.substr(0, normalized.length - 1);
+	}
+	// never return an empty string, unless the input was also empty
+	return normalized || '.';
 }
 
 /**
@@ -225,3 +250,12 @@ export function multicastStream(
 		);
 	});
 }
+
+/**
+ * Escape characters that have a special meaning in regular expressions.
+ * Borrowed from:
+ * https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions
+ */
+function escapeRE(s: string) {
+	return s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); // $& means the whole matched string
+}

tests/tests.ts

@@ -19,21 +19,47 @@ import { Readable, Writable, PassThrough } from 'stream';
 
 import * as Bluebird from 'bluebird';
 import * as fs from 'fs';
-import * as path from 'path';
 import * as tar from 'tar-stream';
 
 import * as TarUtils from '../lib';
 
 describe('Simple utils', () => {
 	it('should correctly normalize a tar entry', done => {
-		const fn = TarUtils.normalizeTarEntry;
-		expect(fn('Dockerfile')).to.equal('Dockerfile');
-		expect(fn('./Dockerfile')).to.equal('Dockerfile');
-		expect(fn('../Dockerfile')).to.equal(`..${path.sep}Dockerfile`);
-		expect(fn('/Dockerfile')).to.equal('Dockerfile');
-		expect(fn('./a/b/Dockerfile')).to.equal(
-			`a${path.sep}b${path.sep}Dockerfile`,
-		);
+		const testCases = [
+			// input, expected output
+			// a slash is also programmatically appended to every input test case
+			['', ''],
+			['.', '.'],
+			['..', '..'],
+			['/', '.'],
+			['/.', '.'],
+			['/..', '.'], // root's parent? hmm
+			['Dockerfile', 'Dockerfile'],
+			['./Dockerfile', 'Dockerfile'],
+			['../Dockerfile', `../Dockerfile`],
+			['/Dockerfile', 'Dockerfile'],
+			['./a/b/Dockerfile', `a/b/Dockerfile`],
+			['./a/../b/Dockerfile', `b/Dockerfile`],
+			['///a//b/Dockerfile', `a/b/Dockerfile`],
+			['a/./b/Dockerfile', `a/b/Dockerfile`],
+		];
+		for (let [input, expected] of testCases) {
+			let result = TarUtils.normalizeTarEntry(input);
+			expect(result).to.equal(
+				expected,
+				`normalizeTarEntry('${input}') returned '${result}', expected '${expected}'`,
+			);
+			if (input === '') {
+				continue;
+			}
+			// adding a trailing slash to the input should not alter the output
+			input += '/';
+			result = TarUtils.normalizeTarEntry(input);
+			expect(result).to.equal(
+				expected,
+				`normalizeTarEntry('${input}') returned '${result}' expected '${expected}'`,
+			);
+		}
 		done();
 	});
 
@@ -124,7 +150,7 @@ describe('multicastStream', function() {
 		const nStreams = 3;
 		for (let i = 0; i < nStreams; ++i) {
 			toStreams.push(
-				new MockWritable({ highWaterMark: hwm / (i + 1) }, i * 10),
+				new MockWritable({ highWaterMark: Math.ceil(hwm / (i + 1)) }, i * 10),
 			);
 		}