Skip to content

Commit 88f3a8c

Browse files
author
Gyan Ranjan A
committed
address review: derive constant, add invariant docs, exact assertion, e2e test
- MAX_FILENAME_LENGTH now shows derivation (255 - 17) instead of magic 238 - Added invariant comment: \w without /u guarantees ASCII, so .length == bytes - Noted why we hash jobName not encoded (encoding isn't injective) - Changed toContain('Lw') to toBe('jobLwname') for exact assertion - Added tests/test-cases/parallel-matrix-long-name/ integration fixture that exercises the actual crash path end-to-end (regression guard for #1862)
1 parent e12efd4 commit 88f3a8c

5 files changed

Lines changed: 34 additions & 2 deletions

File tree

src/utils.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,13 @@ export class Utils {
5050
return url.replace(/^https:\/\//g, "").replace(/^http:\/\//g, "");
5151
}
5252

53-
static readonly MAX_FILENAME_LENGTH = 238; // 255 (NAME_MAX) - 17 (longest suffix: "gcl-" + "-" + jobId + "-build")
53+
// gcl-${safeJobName}-${jobId}-build → wrapper is 17 chars (jobId max 6 digits)
54+
static readonly MAX_FILENAME_LENGTH = 255 - 17; // NAME_MAX (bytes) - wrapper
5455

5556
static safeDockerString (jobName: string) {
57+
// INVARIANT: \w without /u is ASCII-only ([A-Za-z0-9_]), so `encoded` is pure ASCII
58+
// and .length === byte length. NAME_MAX is a byte limit — adding /u would break this.
59+
// We hash `jobName` (not `encoded`) because base64url encoding isn't injective.
5660
const encoded = jobName.replace(/[^\w-]+/g, (match) => {
5761
return base64url.encode(match);
5862
});
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
.gitlab-ci-local-*
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
build-job:
3+
stage: build
4+
script:
5+
- echo "APP='${APP}' $CI_NODE_INDEX/$CI_NODE_TOTAL"
6+
parallel:
7+
matrix:
8+
- APP:
9+
- "my-app-controller,My app controller to be used as reference for development teams,python311,controller,common,controller/setup.py,controller/setup_c.py,controller/setup_n.py,controller/tests/**/*,controller/coverage/*,controller/build/**/*,controller/coverage/coverage-unit.xml,75,true"
10+
- short
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import {WriteStreamsMock} from "../../../src/write-streams.js";
2+
import {handler} from "../../../src/handler.js";
3+
4+
test.concurrent("parallel-matrix-long-name - completes without ENAMETOOLONG", async () => {
5+
const writeStreams = new WriteStreamsMock();
6+
await handler({
7+
cwd: "tests/test-cases/parallel-matrix-long-name",
8+
shellIsolation: true,
9+
stateDir: ".gitlab-ci-local-parallel-matrix-long-name",
10+
}, writeStreams);
11+
12+
// Both matrix entries must complete — the long one would crash with ENAMETOOLONG before the fix
13+
const passing = writeStreams.stdoutLines.filter(l => l.includes(" PASS "));
14+
expect(passing.length).toBe(2);
15+
expect(writeStreams.stdoutLines.some(l => l.includes("build-job: [short]"))).toBe(true);
16+
expect(writeStreams.stdoutLines.some(l => l.includes("build-job: [my-app-controller,"))).toBe(true);
17+
});

tests/utils.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,7 @@ describe("safeDockerString", () => {
275275

276276
it("should encode non-alphanumeric characters", () => {
277277
const result = Utils.safeDockerString("job/name");
278-
expect(result).toContain("Lw"); // '/' encodes to base64url
278+
expect(result).toBe("jobLwname"); // '/' → 'Lw'
279279
});
280280

281281
it("should truncate and hash when encoded name exceeds MAX_FILENAME_LENGTH", () => {

0 commit comments

Comments
 (0)