mirror of
https://github.com/status-im/sourcecred.git
synced 2025-01-27 04:46:13 +00:00
Fix crash on repos with underscores and dots (#738)
The GitHub regex in urlIdParse.js incorrectly disallowed repo names with underscores and dots. Fixes #721. To mitigate errors like this in the future, code which uses regexes to find owners and repos has been modified to all depend on the same regex pattern. Test plan: Unit tests have been updated to include the failure case (they correctly failed), and then code was updated so that the tests pass again. Also, I manually verified that loading ipfs/js.ipfs.io no longer fails. Paired with @wchargin
This commit is contained in:
Dandelion Mané
GitHub
parent
436cad0326
commit
8009e20e5b
@ -5,9 +5,12 @@ export opaque type Repo: {|+name: string, +owner: string|} = {|
|
||||
+owner: string,
|
||||
|};
|
||||
|
||||
export const githubOwnerPattern = "[A-Za-z0-9-]+";
|
||||
export const githubRepoPattern = "[A-Za-z0-9-._]+";
|
||||
|
||||
export function makeRepo(owner: string, name: string): Repo {
|
||||
const validOwner = /^[A-Za-z0-9-]+$/;
|
||||
const validRepo = /^[A-Za-z0-9-._]+$/;
|
||||
const validOwner = new RegExp(`^${githubOwnerPattern}$`);
|
||||
const validRepo = new RegExp(`^${githubRepoPattern}$`);
|
||||
if (!owner.match(validOwner)) {
|
||||
throw new Error(`Invalid repository owner: ${JSON.stringify(owner)}`);
|
||||
}
|
||||
|
||||
@ -1,6 +1,7 @@
|
||||
// @flow
|
||||
|
||||
import {textBlocks} from "./parseMarkdown";
|
||||
import {githubOwnerPattern, githubRepoPattern} from "../../core/repo";
|
||||
|
||||
export type ParsedReference = {|
|
||||
// "@user" or "#123" or "https://github.com/owner/name/..."
|
||||
@ -32,10 +33,11 @@ function parseReferencesFromRawString(textBlock: string): ParsedReference[] {
|
||||
}
|
||||
|
||||
function findRepoNumericReferences(textBlock: string): ParsedReference[] {
|
||||
return findAllMatches(
|
||||
/(?:\W|^)([a-zA-Z0-9-]+\/[a-zA-Z0-9-]+#\d+)(?=\W|$)/gm,
|
||||
textBlock
|
||||
).map((x) => ({
|
||||
const re = new RegExp(
|
||||
`(?:\\W|^)((?:${githubOwnerPattern})/(?:${githubRepoPattern})#\\d+)(?=\\W|$)`,
|
||||
"gm"
|
||||
);
|
||||
return findAllMatches(re, textBlock).map((x) => ({
|
||||
refType: "BASIC",
|
||||
ref: x[1],
|
||||
}));
|
||||
@ -49,12 +51,16 @@ function findNumericReferences(textBlock: string): ParsedReference[] {
|
||||
}
|
||||
|
||||
function findUsernameReferences(textBlock: string): ParsedReference[] {
|
||||
const pairedWithPattern =
|
||||
"(?:\\W|^)(?:P|p)aired(?:-| )(?:w|W)ith:? " +
|
||||
`(@(?:${githubOwnerPattern}))(?=\\W|$)`;
|
||||
const basicPattern = `(?:\\W|^)(@(?:${githubOwnerPattern}))(?=\\W|$)`;
|
||||
const pairedWithRefs = findAllMatches(
|
||||
/(?:\W|^)(?:P|p)aired(?:-| )(?:w|W)ith:? (@[a-zA-Z0-9-]+)(?=\W|$)/gm,
|
||||
new RegExp(pairedWithPattern, "gm"),
|
||||
textBlock
|
||||
).map((x) => ({ref: x[1], refType: "PAIRED_WITH"}));
|
||||
const basicRefs = findAllMatches(
|
||||
/(?:\W|^)(@[a-zA-Z0-9-]+)(?=\W|$)/gm,
|
||||
new RegExp(basicPattern, "gm"),
|
||||
textBlock
|
||||
).map((x) => ({ref: x[1], refType: "BASIC"}));
|
||||
for (const {ref} of pairedWithRefs) {
|
||||
@ -68,16 +74,15 @@ function findUsernameReferences(textBlock: string): ParsedReference[] {
|
||||
}
|
||||
|
||||
function findGithubUrlReferences(textBlock: string): ParsedReference[] {
|
||||
const githubNamePart = /(?:[a-zA-Z0-9_-]+)/.source;
|
||||
const urlRegex = new RegExp(
|
||||
"" +
|
||||
/(?:\W|^)/.source +
|
||||
"(" +
|
||||
/http(?:s)?:\/\/github.com\//.source +
|
||||
githubNamePart +
|
||||
`(?:${githubOwnerPattern})` +
|
||||
"(?:" +
|
||||
/\//.source +
|
||||
githubNamePart +
|
||||
`(?:${githubRepoPattern})` +
|
||||
/\/(?:issues|pull)\//.source +
|
||||
/(?:\d+)/.source +
|
||||
/(?:#(?:issue|issuecomment|pullrequestreview|discussion_r)-?(?:\d+))?/
|
||||
|
||||
@ -65,90 +65,90 @@ describe("parseReferences", () => {
|
||||
});
|
||||
|
||||
describe("cross-repo links", () => {
|
||||
const repoRef = "sourcecred/sourcecred#12";
|
||||
const repoRef = "sourcecred/example_.repo#12";
|
||||
it("a bare link", () => {
|
||||
expect(parseReferences(repoRef)).toEqual([
|
||||
{refType: "BASIC", ref: repoRef},
|
||||
]);
|
||||
});
|
||||
it("a link with surrounding context", () => {
|
||||
expect(parseReferences("please see sourcecred/sourcecred#12")).toEqual([
|
||||
{refType: "BASIC", ref: repoRef},
|
||||
]);
|
||||
expect(parseReferences("please see sourcecred/example_.repo#12")).toEqual(
|
||||
[{refType: "BASIC", ref: repoRef}]
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
it("finds a trivial url reference", () => {
|
||||
expect(
|
||||
parseReferences("https://github.com/sourcecred/sourcecred/issues/86")
|
||||
parseReferences("https://github.com/sourcecred/example_.repo/issues/86")
|
||||
).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("finds url references", () => {
|
||||
const example = `
|
||||
A directly linked issue:
|
||||
https://github.com/sourcecred/example-github/issues/1
|
||||
https://github.com/sourcecred/exa_mple-git.hub/issues/1
|
||||
|
||||
A directly linked issue with fragment:
|
||||
https://github.com/sourcecred/example-github/issues/1#issue-300934818
|
||||
https://github.com/sourcecred/exa_mple-git.hub/issues/1#issue-300934818
|
||||
|
||||
A directly linked pull request:
|
||||
https://github.com/sourcecred/example-github/pull/3
|
||||
https://github.com/sourcecred/exa_mple-git.hub/pull/3
|
||||
|
||||
A directly linked pull request with fragment:
|
||||
https://github.com/sourcecred/example-github/pull/3#issue-171887741
|
||||
https://github.com/sourcecred/exa_mple-git.hub/pull/3#issue-171887741
|
||||
|
||||
A directly linked issue comment:
|
||||
https://github.com/sourcecred/example-github/issues/6#issuecomment-373768442
|
||||
https://github.com/sourcecred/exa_mple-git.hub/issues/6#issuecomment-373768442
|
||||
|
||||
A directly linked pull request review:
|
||||
https://github.com/sourcecred/example-github/pull/5#pullrequestreview-100313899
|
||||
https://github.com/sourcecred/exa_mple-git.hub/pull/5#pullrequestreview-100313899
|
||||
|
||||
A directly linked pull request review comment:
|
||||
https://github.com/sourcecred/example-github/pull/5#discussion_r171460198
|
||||
https://github.com/sourcecred/exa_mple-git.hub/pull/5#discussion_r171460198
|
||||
|
||||
A directly linked pull request comment:
|
||||
https://github.com/sourcecred/example-github/pull/3#issuecomment-369162222
|
||||
https://github.com/sourcecred/exa_mple-git.hub/pull/3#issuecomment-369162222
|
||||
`;
|
||||
|
||||
const expected = [
|
||||
{
|
||||
refType: "BASIC",
|
||||
ref: "https://github.com/sourcecred/example-github/issues/1",
|
||||
ref: "https://github.com/sourcecred/exa_mple-git.hub/issues/1",
|
||||
},
|
||||
{
|
||||
refType: "BASIC",
|
||||
ref:
|
||||
"https://github.com/sourcecred/example-github/issues/1#issue-300934818",
|
||||
"https://github.com/sourcecred/exa_mple-git.hub/issues/1#issue-300934818",
|
||||
},
|
||||
{
|
||||
refType: "BASIC",
|
||||
ref: "https://github.com/sourcecred/example-github/pull/3",
|
||||
ref: "https://github.com/sourcecred/exa_mple-git.hub/pull/3",
|
||||
},
|
||||
{
|
||||
refType: "BASIC",
|
||||
ref:
|
||||
"https://github.com/sourcecred/example-github/pull/3#issue-171887741",
|
||||
"https://github.com/sourcecred/exa_mple-git.hub/pull/3#issue-171887741",
|
||||
},
|
||||
{
|
||||
refType: "BASIC",
|
||||
ref:
|
||||
"https://github.com/sourcecred/example-github/issues/6#issuecomment-373768442",
|
||||
"https://github.com/sourcecred/exa_mple-git.hub/issues/6#issuecomment-373768442",
|
||||
},
|
||||
{
|
||||
refType: "BASIC",
|
||||
ref:
|
||||
"https://github.com/sourcecred/example-github/pull/5#pullrequestreview-100313899",
|
||||
"https://github.com/sourcecred/exa_mple-git.hub/pull/5#pullrequestreview-100313899",
|
||||
},
|
||||
{
|
||||
refType: "BASIC",
|
||||
ref:
|
||||
"https://github.com/sourcecred/example-github/pull/5#discussion_r171460198",
|
||||
"https://github.com/sourcecred/exa_mple-git.hub/pull/5#discussion_r171460198",
|
||||
},
|
||||
{
|
||||
refType: "BASIC",
|
||||
ref:
|
||||
"https://github.com/sourcecred/example-github/pull/3#issuecomment-369162222",
|
||||
"https://github.com/sourcecred/exa_mple-git.hub/pull/3#issuecomment-369162222",
|
||||
},
|
||||
];
|
||||
|
||||
@ -157,20 +157,20 @@ https://github.com/sourcecred/example-github/pull/3#issuecomment-369162222
|
||||
|
||||
it("doesn't find urls mangled with word characters", () => {
|
||||
expect(
|
||||
parseReferences("foohttps://github.com/sourcecred/sourcecred/pull/94")
|
||||
parseReferences("foohttps://github.com/sourcecred/example_.repo/pull/94")
|
||||
).toHaveLength(0);
|
||||
|
||||
expect(
|
||||
parseReferences("https://github.com/sourcecred/sourcecred/pull/94foo")
|
||||
parseReferences("https://github.com/sourcecred/example_.repo/pull/94foo")
|
||||
).toHaveLength(0);
|
||||
|
||||
expect(
|
||||
parseReferences("(https://github.com/sourcecred/sourcecred/pull/94)")
|
||||
parseReferences("(https://github.com/sourcecred/example_.repo/pull/94)")
|
||||
).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("allows but excludes leading and trailing punctuation", () => {
|
||||
const base = "https://github.com/sourcecred/sourcecred/pull/94";
|
||||
const base = "https://github.com/sourcecred/example_.repo/pull/94";
|
||||
expect(parseReferences(`!${base}`)).toEqual([
|
||||
{refType: "BASIC", ref: base},
|
||||
]);
|
||||
@ -207,14 +207,14 @@ https://github.com/sourcecred/example-github/pull/3#issuecomment-369162222
|
||||
it("finds a mix of reference types", () => {
|
||||
expect(
|
||||
parseReferences(
|
||||
"@wchargin commented on #125, eg https://github.com/sourcecred/sourcecred/pull/125#pullrequestreview-113402856"
|
||||
"@wchargin commented on #125, eg https://github.com/sourcecred/example_.repo/pull/125#pullrequestreview-113402856"
|
||||
)
|
||||
).toEqual([
|
||||
{refType: "BASIC", ref: "#125"},
|
||||
{
|
||||
refType: "BASIC",
|
||||
ref:
|
||||
"https://github.com/sourcecred/sourcecred/pull/125#pullrequestreview-113402856",
|
||||
"https://github.com/sourcecred/example_.repo/pull/125#pullrequestreview-113402856",
|
||||
},
|
||||
{refType: "BASIC", ref: "@wchargin"},
|
||||
]);
|
||||
@ -263,14 +263,14 @@ https://github.com/sourcecred/example-github/pull/3#issuecomment-369162222
|
||||
|
||||
describe("finds references at the start of a non-initial line", () => {
|
||||
it("for repo-numeric references", () => {
|
||||
const f = (number: number) => `sourcecred/example-github#${number}`;
|
||||
const f = (number: number) => `sourcecred/exa_mple-git.hub#${number}`;
|
||||
const input = `${f(1)}\n${f(2)}\n${f(3)}\r\n${f(4)}\r\n${f(5)}`;
|
||||
expect(parseReferences(input)).toEqual([
|
||||
{refType: "BASIC", ref: "sourcecred/example-github#1"},
|
||||
{refType: "BASIC", ref: "sourcecred/example-github#2"},
|
||||
{refType: "BASIC", ref: "sourcecred/example-github#3"},
|
||||
{refType: "BASIC", ref: "sourcecred/example-github#4"},
|
||||
{refType: "BASIC", ref: "sourcecred/example-github#5"},
|
||||
{refType: "BASIC", ref: "sourcecred/exa_mple-git.hub#1"},
|
||||
{refType: "BASIC", ref: "sourcecred/exa_mple-git.hub#2"},
|
||||
{refType: "BASIC", ref: "sourcecred/exa_mple-git.hub#3"},
|
||||
{refType: "BASIC", ref: "sourcecred/exa_mple-git.hub#4"},
|
||||
{refType: "BASIC", ref: "sourcecred/exa_mple-git.hub#5"},
|
||||
]);
|
||||
});
|
||||
it("for numeric references", () => {
|
||||
@ -308,28 +308,28 @@ https://github.com/sourcecred/example-github/pull/3#issuecomment-369162222
|
||||
});
|
||||
it("for GitHub URL references", () => {
|
||||
const f = (number: number) =>
|
||||
"https://github.com/sourcecred/example-github/issues/" + number;
|
||||
"https://github.com/sourcecred/exa_mple-git.hub/issues/" + number;
|
||||
const input = `${f(1)}\n${f(2)}\n${f(3)}\r\n${f(4)}\r\n${f(5)}`;
|
||||
expect(parseReferences(input)).toEqual([
|
||||
{
|
||||
refType: "BASIC",
|
||||
ref: "https://github.com/sourcecred/example-github/issues/1",
|
||||
ref: "https://github.com/sourcecred/exa_mple-git.hub/issues/1",
|
||||
},
|
||||
{
|
||||
refType: "BASIC",
|
||||
ref: "https://github.com/sourcecred/example-github/issues/2",
|
||||
ref: "https://github.com/sourcecred/exa_mple-git.hub/issues/2",
|
||||
},
|
||||
{
|
||||
refType: "BASIC",
|
||||
ref: "https://github.com/sourcecred/example-github/issues/3",
|
||||
ref: "https://github.com/sourcecred/exa_mple-git.hub/issues/3",
|
||||
},
|
||||
{
|
||||
refType: "BASIC",
|
||||
ref: "https://github.com/sourcecred/example-github/issues/4",
|
||||
ref: "https://github.com/sourcecred/exa_mple-git.hub/issues/4",
|
||||
},
|
||||
{
|
||||
refType: "BASIC",
|
||||
ref: "https://github.com/sourcecred/example-github/issues/5",
|
||||
ref: "https://github.com/sourcecred/exa_mple-git.hub/issues/5",
|
||||
},
|
||||
]);
|
||||
});
|
||||
@ -337,12 +337,12 @@ https://github.com/sourcecred/example-github/pull/3#issuecomment-369162222
|
||||
|
||||
describe("finds references separated by a single space", () => {
|
||||
it("for repo-numeric references", () => {
|
||||
const f = (number: number) => `sourcecred/example-github#${number}`;
|
||||
const f = (number: number) => `sourcecred/exa_mple-git.hub#${number}`;
|
||||
const input = `${f(1)} ${f(2)} ${f(3)}`;
|
||||
expect(parseReferences(input)).toEqual([
|
||||
{refType: "BASIC", ref: "sourcecred/example-github#1"},
|
||||
{refType: "BASIC", ref: "sourcecred/example-github#2"},
|
||||
{refType: "BASIC", ref: "sourcecred/example-github#3"},
|
||||
{refType: "BASIC", ref: "sourcecred/exa_mple-git.hub#1"},
|
||||
{refType: "BASIC", ref: "sourcecred/exa_mple-git.hub#2"},
|
||||
{refType: "BASIC", ref: "sourcecred/exa_mple-git.hub#3"},
|
||||
]);
|
||||
});
|
||||
it("for numeric references", () => {
|
||||
@ -374,20 +374,20 @@ https://github.com/sourcecred/example-github/pull/3#issuecomment-369162222
|
||||
});
|
||||
it("for GitHub URL references", () => {
|
||||
const f = (number: number) =>
|
||||
"https://github.com/sourcecred/example-github/issues/" + number;
|
||||
"https://github.com/sourcecred/exa_mple-git.hub/issues/" + number;
|
||||
const input = `${f(1)} ${f(2)} ${f(3)}`;
|
||||
expect(parseReferences(input)).toEqual([
|
||||
{
|
||||
refType: "BASIC",
|
||||
ref: "https://github.com/sourcecred/example-github/issues/1",
|
||||
ref: "https://github.com/sourcecred/exa_mple-git.hub/issues/1",
|
||||
},
|
||||
{
|
||||
refType: "BASIC",
|
||||
ref: "https://github.com/sourcecred/example-github/issues/2",
|
||||
ref: "https://github.com/sourcecred/exa_mple-git.hub/issues/2",
|
||||
},
|
||||
{
|
||||
refType: "BASIC",
|
||||
ref: "https://github.com/sourcecred/example-github/issues/3",
|
||||
ref: "https://github.com/sourcecred/exa_mple-git.hub/issues/3",
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
@ -1,11 +1,12 @@
|
||||
// @flow
|
||||
|
||||
// Includes base github url, and the repo owner and repo name with trailing slash
|
||||
const baseGithubRegex = /^https:\/\/github.com\/(?:[a-zA-Z0-9_-]+)\/(?:[a-zA-Z0-9_-]+)\//;
|
||||
import {githubOwnerPattern, githubRepoPattern} from "../../core/repo";
|
||||
const urlBase = "^https?://github\\.com";
|
||||
const repoUrl = `${urlBase}/(?:${githubOwnerPattern})/(?:${githubRepoPattern})/`;
|
||||
|
||||
export function reviewUrlToId(url: string): string {
|
||||
const suffix = /pull\/\d+#pullrequestreview-(\d+)$/;
|
||||
const regex = new RegExp(baseGithubRegex.source + suffix.source);
|
||||
const regex = new RegExp(repoUrl + suffix.source);
|
||||
const result = regex.exec(url);
|
||||
if (result == null) {
|
||||
throw new Error(`Error parsing review url ${url}`);
|
||||
@ -15,7 +16,7 @@ export function reviewUrlToId(url: string): string {
|
||||
|
||||
export function issueCommentUrlToId(url: string): string {
|
||||
const suffix = /issues\/\d+#issuecomment-(\d+)$/;
|
||||
const regex = new RegExp(baseGithubRegex.source + suffix.source);
|
||||
const regex = new RegExp(repoUrl + suffix.source);
|
||||
const result = regex.exec(url);
|
||||
if (result == null) {
|
||||
throw new Error(`Error parsing issue comment url ${url}`);
|
||||
@ -25,7 +26,7 @@ export function issueCommentUrlToId(url: string): string {
|
||||
|
||||
export function pullCommentUrlToId(url: string): string {
|
||||
const suffix = /pull\/\d+#issuecomment-(\d+)$/;
|
||||
const regex = new RegExp(baseGithubRegex.source + suffix.source);
|
||||
const regex = new RegExp(repoUrl + suffix.source);
|
||||
const result = regex.exec(url);
|
||||
if (result == null) {
|
||||
throw new Error(`Error parsing pull comment url ${url}`);
|
||||
@ -35,7 +36,7 @@ export function pullCommentUrlToId(url: string): string {
|
||||
|
||||
export function reviewCommentUrlToId(url: string): string {
|
||||
const suffix = /pull\/\d+#discussion_r(\d+)/;
|
||||
const regex = new RegExp(baseGithubRegex.source + suffix.source);
|
||||
const regex = new RegExp(repoUrl + suffix.source);
|
||||
const result = regex.exec(url);
|
||||
if (result == null) {
|
||||
throw new Error(`Error parsing review comment url ${url}`);
|
||||
|
||||
@ -9,13 +9,13 @@ import {
|
||||
|
||||
describe("plugins/github/urlToId", () => {
|
||||
const issueComment =
|
||||
"https://github.com/example-owner/example-repo0/issues/350#issuecomment-394939349";
|
||||
"https://github.com/example-owner/exa_mple-rep.o0/issues/350#issuecomment-394939349";
|
||||
const pullComment =
|
||||
"https://github.com/example-owner/example-repo0/pull/363#issuecomment-395836900";
|
||||
"https://github.com/example-owner/exa_mple-rep.o0/pull/363#issuecomment-395836900";
|
||||
const reviewComment =
|
||||
"https://github.com/example-owner/example-repo0/pull/380#discussion_r194816899";
|
||||
"https://github.com/example-owner/exa_mple-rep.o0/pull/380#discussion_r194816899";
|
||||
const review =
|
||||
"https://github.com/example-owner/example-repo0/pull/383#pullrequestreview-128199239";
|
||||
"https://github.com/example-owner/exa_mple-rep.o0/pull/383#pullrequestreview-128199239";
|
||||
|
||||
describe("works correctly", () => {
|
||||
it("for issueComment", () => {
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user