Skip to content

Commit

Permalink
Improve encoding of repository URLs
Browse files Browse the repository at this point in the history
- Encode `URLJoinPath` path components.
- Use the function overload of `replaceAll`.

I am hesitant to encode non-`URLJoinPath` template variables because it
could break as much as it fixes. The most popular git hosts all use
`URLJoinPath` anyway.

Closes #35.
  • Loading branch information
isker committed Jan 25, 2025
1 parent e73f0c4 commit dfbbdf6
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 7 deletions.
5 changes: 5 additions & 0 deletions .changeset/small-windows-beam.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"neogrok": patch
---

Improve URL encoding of repository URLs
34 changes: 34 additions & 0 deletions src/lib/url-templates.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,17 @@ describe("evaluateFileUrlTemplate", () => {
"ignored",
),
).toEqual("https://svn.future/r/12345/nopath");
expect(
evaluateFileUrlTemplate(
"https://github.com/hash/enjoyer/blob/{{.Version}}/{{.Path}}",
"m^ster",
"langs/a++/$$$/🚔/c#.md",
),
).toEqual(
// We cannot confidently apply any URL encodings to an old-style template
// because we do not know where in the URL the variables appear.
"https://github.com/hash/enjoyer/blob/m^ster/langs/a++/$$$/🚔/c#.md",
);
});

it("evaluates new templates", () => {
Expand All @@ -30,6 +41,15 @@ describe("evaluateFileUrlTemplate", () => {
"genversion.sh",
),
).toEqual("https://github.com/hanwen/go-fuse/blob/notify/genversion.sh");
expect(
evaluateFileUrlTemplate(
'{{URLJoinPath "https://github.com/hash/enjoyer/blob" .Version .Path}}',
"m^ster",
"langs/a++/$$$/🚔/c#.md",
),
).toEqual(
"https://github.com/hash/enjoyer/blob/m%5Ester/langs/a%2B%2B/%24%24%24/%F0%9F%9A%94/c%23.md",
);
expect(
evaluateFileUrlTemplate(
'{{ URLJoinPath "https://github.com/hanwen/go-fuse/blob" .Version .Path }}',
Expand Down Expand Up @@ -58,6 +78,14 @@ describe("evaluateCommitUrlTemplate", () => {
expect(
evaluateCommitUrlTemplate("https://svn.future/r/{{.Version}}", "12345"),
).toEqual("https://svn.future/r/12345");
expect(
evaluateCommitUrlTemplate(
"https://github.com/hash/enjoyer/commit/{{.Version}}",
"m^ster",
),
// We cannot confidently apply any URL encodings to an old-style template
// because we do not know where in the URL the variables appear.
).toEqual("https://github.com/hash/enjoyer/commit/m^ster");
});

it("evaluates new templates", () => {
Expand All @@ -79,5 +107,11 @@ describe("evaluateCommitUrlTemplate", () => {
"12345",
),
).toEqual("https://svn.future/r/12345");
expect(
evaluateCommitUrlTemplate(
'{{URLJoinPath "https://github.com/hash/enjoyer/commit" .Version}}',
"m^ster",
),
).toEqual("https://github.com/hash/enjoyer/commit/m%5Ester");
});
});
22 changes: 15 additions & 7 deletions src/lib/url-templates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,24 @@ export const evaluateFileUrlTemplate = (
.split(/\s+/)
.map((s) => {
if (s === ".Version") {
return version;
return version.split("/").map(encodeURIComponent).join("/");
} else if (s === ".Path") {
return path;
return path.split("/").map(encodeURIComponent).join("/");
} else {
// It's a quoted string: https://pkg.go.dev/strconv#Quote.
return JSON.parse(s);
}
})
.join("/");
} else {
return template
.replaceAll("{{.Version}}", version)
.replaceAll("{{.Path}}", path);
return (
template
// We use the function version of replaceAll because it interprets a
// variety of characters in strings specially. Only functions guarantee
// literal replacement.
.replaceAll("{{.Version}}", () => version)
.replaceAll("{{.Path}}", () => path)
);
}
};

Expand All @@ -44,14 +49,17 @@ export const evaluateCommitUrlTemplate = (
.split(/\s+/)
.map((s) => {
if (s === ".Version") {
return version;
return version.split("/").map(encodeURIComponent).join("/");
} else {
// It's a quoted string: https://pkg.go.dev/strconv#Quote.
return JSON.parse(s);
}
})
.join("/");
} else {
return template.replaceAll("{{.Version}}", version);
// We use the function version of replaceAll because it interprets a
// variety of characters in strings specially. Only functions guarantee
// literal replacement.
return template.replaceAll("{{.Version}}", () => version);
}
};

0 comments on commit dfbbdf6

Please sign in to comment.