Skip to content

Commit 7aa9cc4

Browse files
committedJan 30, 2025··
feat(ReadTask): improve SourceFile validation
1 parent 5715e63 commit 7aa9cc4

File tree

4 files changed

+97
-10
lines changed

4 files changed

+97
-10
lines changed
 

‎src/File.spec.ts

+72
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
import { writeFile } from "node:fs/promises";
2+
import { homedir } from "node:os";
3+
import { join } from "node:path";
4+
import { expect, tmpname } from "./_chai.spec";
5+
import { compareFilePaths, isFileEmpty, isPlatformCaseSensitive } from "./File";
6+
7+
describe("File", () => {
8+
describe("isFileEmpty()", () => {
9+
it("returns true for non-existent file", async () => {
10+
const path = tmpname();
11+
expect(await isFileEmpty(path)).to.eql(true);
12+
});
13+
14+
it("returns true for empty file", async () => {
15+
const path = tmpname();
16+
await writeFile(path, "");
17+
expect(await isFileEmpty(path)).to.eql(true);
18+
});
19+
20+
it("returns false for non-empty file", async () => {
21+
const path = tmpname();
22+
await writeFile(path, "content");
23+
expect(await isFileEmpty(path)).to.eql(false);
24+
});
25+
26+
it("throws error for blank path", async () => {
27+
await expect(isFileEmpty("")).to.be.rejectedWith(
28+
"isFileEmpty(): blank path",
29+
);
30+
});
31+
});
32+
33+
describe("compareFilePaths()", () => {
34+
it("returns true for identical paths", () => {
35+
const path = "/some/path/file.txt";
36+
expect(compareFilePaths(path, path)).to.eql(true);
37+
});
38+
39+
it("returns true for normalized paths", () => {
40+
expect(
41+
compareFilePaths(
42+
join(homedir(), "file"),
43+
join(homedir(), "dir", "..", "file"),
44+
),
45+
).to.eql(true);
46+
});
47+
48+
it("returns true for case-sensitive paths", () => {
49+
expect(compareFilePaths("/path/to/file", "/path/to/file")).to.eql(true);
50+
});
51+
52+
it("returns false for case-insensitive paths on Linux", () => {
53+
expect(compareFilePaths("/PATH/TO/FILE", "/path/to/file")).to.eql(
54+
isPlatformCaseSensitive(),
55+
);
56+
});
57+
58+
it("returns true for case-insensitive paths on non-Linux", () => {
59+
expect(compareFilePaths("/PATH/TO/FILE", "/path/to/file")).to.eql(true);
60+
});
61+
62+
it("handles different path separators", () => {
63+
expect(compareFilePaths("/path/to/file", "\\path\\to/file")).to.eql(true);
64+
});
65+
66+
it("returns false for different paths", () => {
67+
expect(compareFilePaths("/path/to/file1", "/path/to/file2")).to.eql(
68+
false,
69+
);
70+
});
71+
});
72+
});

‎src/File.ts

+14
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
import { stat, Stats } from "node:fs";
2+
import { normalize } from "node:path";
3+
import { lazy } from "./Lazy";
24
import { blank } from "./String";
35

46
export async function isFileEmpty(path: string): Promise<boolean> {
@@ -27,3 +29,15 @@ export async function isFileEmpty(path: string): Promise<boolean> {
2729
else throw err;
2830
}
2931
}
32+
33+
export const isPlatformCaseSensitive = lazy(
34+
() => process.platform !== "win32" && process.platform !== "darwin",
35+
);
36+
37+
export function compareFilePaths(a: string, b: string): boolean {
38+
const aNorm = normalize(a);
39+
const bNorm = normalize(b);
40+
return isPlatformCaseSensitive()
41+
? aNorm === bNorm
42+
: aNorm.localeCompare(bNorm, undefined, { sensitivity: "base" }) === 0;
43+
}

‎src/ReadTask.spec.ts

+8-6
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import {
1616
import { hms } from "./DateTime";
1717
import { ExifDateTime } from "./ExifDateTime";
1818
import { defaultVideosToUTC, ExifTime, ExifTool } from "./ExifTool";
19-
import { GeolocationTagNames } from "./GeolocationTags";
19+
import { GeolocationTagNames, GeolocationTags } from "./GeolocationTags";
2020
import { omit } from "./Object";
2121
import { pick } from "./Pick";
2222
import { ReadTask, ReadTaskOptions } from "./ReadTask";
@@ -26,15 +26,16 @@ import { isUTC } from "./Timezones";
2626
// eslint-disable-next-line @typescript-eslint/no-require-imports
2727
const gt = require("geo-tz");
2828

29+
const SourceFile = join(tmpdir(), "example.jpg");
30+
2931
function parse(
3032
args: {
3133
tags: object;
3234
error?: Error;
3335
SourceFile?: string;
3436
} & Partial<ReadTaskOptions>,
3537
): Tags {
36-
const SourceFile = args.SourceFile ?? "/tmp/example.jpg";
37-
const tt = ReadTask.for(SourceFile, {
38+
const tt = ReadTask.for(args.SourceFile ?? SourceFile, {
3839
defaultVideosToUTC: true,
3940
backfillTimezones: true,
4041
includeImageDataMD5: true,
@@ -212,6 +213,7 @@ describe("ReadTask", () => {
212213
expect(
213214
parse({
214215
tags: {
216+
SourceFile,
215217
GPSLatitude: 0,
216218
GPSLongitude: 0,
217219
GeolocationCity: "Takoradi",
@@ -234,13 +236,14 @@ describe("ReadTask", () => {
234236
geolocation: true,
235237
}),
236238
).to.eql({
237-
SourceFile: "/tmp/example.jpg",
239+
SourceFile,
238240
errors: [],
239241
warnings: ["Ignoring zero coordinates from GPSLatitude/GPSLongitude"],
240242
});
241243
});
242244
it("extracts GPS tags if valid lat/lon", () => {
243245
const tags = {
246+
SourceFile,
244247
GPSLatitude: "37 deg 48' 3.45\" N",
245248
GPSLongitude: "122 deg 23' 55.67\" W",
246249
GPSLatitudeRef: "North",
@@ -258,7 +261,7 @@ describe("ReadTask", () => {
258261
GeolocationPosition: "37.7966, -122.4086",
259262
GeolocationDistance: "1.01 km",
260263
GeolocationBearing: 246,
261-
};
264+
} satisfies Tags & Required<Omit<GeolocationTags, "GeolocationWarning">>;
262265
expect(
263266
parse({
264267
tags,
@@ -271,7 +274,6 @@ describe("ReadTask", () => {
271274
GPSLatitudeRef: "N",
272275
GPSLongitude: -122.398797,
273276
GPSLongitudeRef: "W",
274-
SourceFile: "/tmp/example.jpg",
275277
tz: "America/Los_Angeles",
276278
tzSource: "GeolocationTimeZone",
277279
errors: [],

‎src/ReadTask.ts

+3-4
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { ExifDateTime } from "./ExifDateTime";
1010
import { ExifTime } from "./ExifTime";
1111
import { ExifToolOptions, handleDeprecatedOptions } from "./ExifToolOptions";
1212
import { ExifToolTask } from "./ExifToolTask";
13+
import { compareFilePaths } from "./File";
1314
import { Utf8FilenameCharsetArgs } from "./FilenameCharsetArgs";
1415
import { parseGPSLocation } from "./GPS";
1516
import { lazy } from "./Lazy";
@@ -152,10 +153,8 @@ export class ReadTask extends ExifToolTask<Tags> {
152153
}
153154
// ExifTool does "humorous" things to paths, like flip path separators. resolve() undoes that.
154155
if (notBlank(this.#raw.SourceFile)) {
155-
const SourceFile = _path.resolve(this.#raw.SourceFile);
156-
// Sanity check that the result is for the file we want:
157-
if (SourceFile !== this.sourceFile) {
158-
// Throw an error rather than add an errors string because this is *really* bad:
156+
if (!compareFilePaths(this.#raw.SourceFile, this.sourceFile)) {
157+
// this would indicate a bug in batch-cluster:
159158
throw new Error(
160159
`Internal error: unexpected SourceFile of ${this.#raw.SourceFile} for file ${this.sourceFile}`,
161160
);

0 commit comments

Comments
 (0)
Please sign in to comment.