From c3f7b29d449f9db3d08be7cecb8d2305de568ad8 Mon Sep 17 00:00:00 2001 From: Michael C Date: Tue, 24 Aug 2021 19:12:58 -0400 Subject: [PATCH 1/8] throw error 400 when start or endtime has colon --- src/routes/postSkipSegments.ts | 9 +++++++++ test/cases/postSkipSegments.ts | 22 ++++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/src/routes/postSkipSegments.ts b/src/routes/postSkipSegments.ts index 492e5dc..e871c15 100644 --- a/src/routes/postSkipSegments.ts +++ b/src/routes/postSkipSegments.ts @@ -324,6 +324,15 @@ function checkInvalidFields(videoID: any, userID: any, segments: Array): Ch if (!Array.isArray(segments) || segments.length < 1) { invalidFields.push("segments"); } + // validate start and end times (no : marks) + for (const segmentPair of segments) { + const startTime = segmentPair.segment[0]; + const endTime = segmentPair.segment[1]; + if ((typeof startTime === "string" && startTime.includes(":")) || + (typeof endTime === "string" && endTime.includes(":"))) { + invalidFields.push("segment time"); + } + } if (invalidFields.length !== 0) { // invalid request diff --git a/test/cases/postSkipSegments.ts b/test/cases/postSkipSegments.ts index b6643c2..de502e1 100644 --- a/test/cases/postSkipSegments.ts +++ b/test/cases/postSkipSegments.ts @@ -987,4 +987,26 @@ describe("postSkipSegments", () => { }) .catch(err => done(err)); }); + + it("Should not be able to submit with colons in timestamps", (done: Done) => { + fetch(`${getbaseURL()}/api/postVideoSponsorTimes`, { + method: "POST", + headers: { + "Content-Type": "application/json" + }, + body: JSON.stringify({ + userID: "testtesttesttesttesttesttesttesttest", + videoID: "colon-1", + segments: [{ + segment: ["0:2.000", "3:10.392"], + category: "sponsor", + }] + }), + }) + .then(async res => { + assert.strictEqual(res.status, 400); + done(); + }) + .catch(err => done(err)); + }); }); From 265a01dcded19114353aff1d296600d3dde7cf84 Mon Sep 17 00:00:00 2001 From: Michael C Date: Wed, 25 Aug 2021 01:56:34 -0400 Subject: [PATCH 2/8] re-shadowban user if user is already shadowbanned but unhideOldSubmissions is true, sets all submissions to hidden. If not true, then return 409 duplicate --- src/routes/shadowBanUser.ts | 22 +++++++++++++++++++-- test/cases/shadowBanUser.ts | 38 +++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/src/routes/shadowBanUser.ts b/src/routes/shadowBanUser.ts index 3ecb653..b1196b0 100644 --- a/src/routes/shadowBanUser.ts +++ b/src/routes/shadowBanUser.ts @@ -49,8 +49,8 @@ export async function shadowBanUser(req: Request, res: Response): Promise `'${c}'`).join(",")}) - AND NOT EXISTS ( SELECT "videoID", "category" FROM "lockCategories" WHERE - "sponsorTimes"."videoID" = "lockCategories"."videoID" AND "sponsorTimes"."category" = "lockCategories"."category")`, [userID]); + AND NOT EXISTS ( SELECT "videoID", "category" FROM "lockCategories" WHERE + "sponsorTimes"."videoID" = "lockCategories"."videoID" AND "sponsorTimes"."category" = "lockCategories"."category")`, [userID]); // clear cache for all old videos (await db.prepare("all", `SELECT "videoID", "hashedVideoID", "service", "votes", "views" FROM "sponsorTimes" WHERE "userID" = ?`, [userID])) @@ -84,6 +84,24 @@ export async function shadowBanUser(req: Request, res: Response): Promise `'${c}'`).join(",")})`, [UUID]); })); } + // already shadowbanned + } else if (enabled && row.userCount > 0) { + // apply unHideOldSubmissions if applicable + if (unHideOldSubmissions) { + await db.prepare("run", `UPDATE "sponsorTimes" SET "shadowHidden" = 1 WHERE "userID" = ? AND "category" in (${categories.map((c) => `'${c}'`).join(",")}) + AND NOT EXISTS ( SELECT "videoID", "category" FROM "lockCategories" WHERE + "sponsorTimes"."videoID" = "lockCategories"."videoID" AND "sponsorTimes"."category" = "lockCategories"."category")`, [userID]); + + // clear cache for all old videos + (await db.prepare("all", `SELECT "videoID", "hashedVideoID", "service", "votes", "views" FROM "sponsorTimes" WHERE "userID" = ?`, [userID])) + .forEach((videoInfo: {category: Category, videoID: VideoID, hashedVideoID: VideoIDHash, service: Service, userID: UserID}) => { + QueryCacher.clearVideoCache(videoInfo); + } + ); + return res.sendStatus(200); + } + // otherwise ban already exists, send 409 + return res.sendStatus(409); } } else if (hashedIP) { //check to see if this user is already shadowbanned diff --git a/test/cases/shadowBanUser.ts b/test/cases/shadowBanUser.ts index 419c324..56cf15f 100644 --- a/test/cases/shadowBanUser.ts +++ b/test/cases/shadowBanUser.ts @@ -18,7 +18,11 @@ describe("shadowBanUser", () => { await db.prepare("run", insertQuery, ["testtesttest", 1, 11, 2, 0, "shadow-3-uuid-0", "shadowBanned3", 0, 50, "sponsor", "YouTube", 100, 0, 1, getHash("testtesttest", 1)]); await db.prepare("run", insertQuery, ["testtesttest2", 1, 11, 2, 0, "shadow-3-uuid-0-1", "shadowBanned3", 0, 50, "sponsor", "PeerTube", 120, 0, 1, getHash("testtesttest2", 1)]); await db.prepare("run", insertQuery, ["testtesttest", 20, 33, 2, 0, "shadow-3-uuid-2", "shadowBanned3", 0, 50, "intro", "YouTube", 101, 0, 1, getHash("testtesttest", 1)]); + + await db.prepare("run", insertQuery, ["testtesttest", 21, 34, 2, 0, "shadow-4-uuid-1", "shadowBanned4", 0, 50, "sponsor", "YouTube", 101, 0, 0, getHash("testtesttest", 1)]); + await db.prepare("run", `INSERT INTO "shadowBannedUsers" ("userID") VALUES(?)`, ["shadowBanned3"]); + await db.prepare("run", `INSERT INTO "shadowBannedUsers" ("userID") VALUES(?)`, ["shadowBanned4"]); await db.prepare("run", `INSERT INTO "vipUsers" ("userID") VALUES(?)`, [getHash("shadow-ban-vip")]); }); @@ -106,4 +110,38 @@ describe("shadowBanUser", () => { .catch(err => done(err)); }); + it("Should get 409 when re-shadowbanning user", (done: Done) => { + fetch(`${getbaseURL() + }/api/shadowBanUser?userID=shadowBanned4&adminUserID=shadow-ban-vip&enabled=true&categories=["sponsor"]&unHideOldSubmissions=false`, { + method: "POST" + }) + .then(async res => { + assert.strictEqual(res.status, 409); + const videoRow = await db.prepare("all", `SELECT "shadowHidden", "category" FROM "sponsorTimes" WHERE "userID" = ? AND "shadowHidden" = ?`, ["shadowBanned4", 0]); + const shadowRow = await db.prepare("get", `SELECT * FROM "shadowBannedUsers" WHERE "userID" = ?`, ["shadowBanned4"]); + assert.ok(shadowRow); // ban still exists + assert.strictEqual(videoRow.length, 1); // videos should not be hidden + assert.strictEqual(videoRow[0].category, "sponsor"); + done(); + }) + .catch(err => done(err)); + }); + + it("Should be able to re-shadowban user to hide old submissions", (done: Done) => { + fetch(`${getbaseURL() + }/api/shadowBanUser?userID=shadowBanned4&adminUserID=shadow-ban-vip&enabled=true&categories=["sponsor"]&unHideOldSubmissions=true`, { + method: "POST" + }) + .then(async res => { + assert.strictEqual(res.status, 200); + const videoRow = await db.prepare("all", `SELECT "shadowHidden", "category" FROM "sponsorTimes" WHERE "userID" = ? AND "shadowHidden" = ?`, ["shadowBanned4", 1]); + const shadowRow = await db.prepare("get", `SELECT * FROM "shadowBannedUsers" WHERE "userID" = ?`, ["shadowBanned4"]); + assert.ok(shadowRow); // ban still exists + assert.strictEqual(videoRow.length, 1); // videos should be hidden + assert.strictEqual(videoRow[0].category, "sponsor"); + done(); + }) + .catch(err => done(err)); + }); + }); From 268008945c0cd645369676db0c805b1c3839fd01 Mon Sep 17 00:00:00 2001 From: Michael C Date: Fri, 27 Aug 2021 16:41:26 -0400 Subject: [PATCH 3/8] disallow POI before 1 second --- src/routes/postSkipSegments.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/routes/postSkipSegments.ts b/src/routes/postSkipSegments.ts index 492e5dc..4b99d8b 100644 --- a/src/routes/postSkipSegments.ts +++ b/src/routes/postSkipSegments.ts @@ -381,6 +381,11 @@ async function checkEachSegmentValid(userID: string, videoID: VideoID return { pass: false, errorMessage: "One of your segments times are invalid (too short, startTime before endTime, etc.)", errorCode: 400}; } + // Check for POI segments before 1 second + if (segments[i].category === CategoryActionType.POI && startTime < 1) { + return { pass: false, errorMessage: "POI must be after 1 second", errorCode: 400}; + } + if (!isVIP && segments[i].category === "sponsor" && Math.abs(startTime - endTime) < 1) { // Too short return { pass: false, errorMessage: "Sponsors must be longer than 1 second long", errorCode: 400}; From c448bb3d9a7e6ce8d0d06e944d76439d372d6c73 Mon Sep 17 00:00:00 2001 From: Michael C Date: Fri, 27 Aug 2021 16:44:29 -0400 Subject: [PATCH 4/8] add test case --- test/cases/postSkipSegments.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/cases/postSkipSegments.ts b/test/cases/postSkipSegments.ts index b6643c2..2ae86ab 100644 --- a/test/cases/postSkipSegments.ts +++ b/test/cases/postSkipSegments.ts @@ -987,4 +987,16 @@ describe("postSkipSegments", () => { }) .catch(err => done(err)); }); + + it("Should be rejected if a POI is at less than 1 second", (done: Done) => { + fetch(`${getbaseURL() + }/api/skipSegments?videoID=qqwerty&startTime=0.5&endTime=0.5&userID=testtesttesttesttesttesttesttesttesting`, { + method: "POST", + }) + .then(res => { + assert.strictEqual(res.status, 400); + done(); + }) + .catch(err => done(err)); + }); }); From c3a5b22dadd699b3f33aefe230ffc0355cb4d189 Mon Sep 17 00:00:00 2001 From: Ajay Ramachandran Date: Sat, 28 Aug 2021 00:18:31 -0400 Subject: [PATCH 5/8] Move unHideSubmissions to helper function --- src/routes/shadowBanUser.ts | 36 ++++++++++++++++-------------------- 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/src/routes/shadowBanUser.ts b/src/routes/shadowBanUser.ts index b1196b0..bd04760 100644 --- a/src/routes/shadowBanUser.ts +++ b/src/routes/shadowBanUser.ts @@ -48,16 +48,7 @@ export async function shadowBanUser(req: Request, res: Response): Promise `'${c}'`).join(",")}) - AND NOT EXISTS ( SELECT "videoID", "category" FROM "lockCategories" WHERE - "sponsorTimes"."videoID" = "lockCategories"."videoID" AND "sponsorTimes"."category" = "lockCategories"."category")`, [userID]); - - // clear cache for all old videos - (await db.prepare("all", `SELECT "videoID", "hashedVideoID", "service", "votes", "views" FROM "sponsorTimes" WHERE "userID" = ?`, [userID])) - .forEach((videoInfo: {category: Category, videoID: VideoID, hashedVideoID: VideoIDHash, service: Service, userID: UserID}) => { - QueryCacher.clearVideoCache(videoInfo); - } - ); + await unHideSubmissions(categories, userID); } } else if (!enabled && row.userCount > 0) { //remove them from the shadow ban list @@ -88,18 +79,10 @@ export async function shadowBanUser(req: Request, res: Response): Promise 0) { // apply unHideOldSubmissions if applicable if (unHideOldSubmissions) { - await db.prepare("run", `UPDATE "sponsorTimes" SET "shadowHidden" = 1 WHERE "userID" = ? AND "category" in (${categories.map((c) => `'${c}'`).join(",")}) - AND NOT EXISTS ( SELECT "videoID", "category" FROM "lockCategories" WHERE - "sponsorTimes"."videoID" = "lockCategories"."videoID" AND "sponsorTimes"."category" = "lockCategories"."category")`, [userID]); - - // clear cache for all old videos - (await db.prepare("all", `SELECT "videoID", "hashedVideoID", "service", "votes", "views" FROM "sponsorTimes" WHERE "userID" = ?`, [userID])) - .forEach((videoInfo: {category: Category, videoID: VideoID, hashedVideoID: VideoIDHash, service: Service, userID: UserID}) => { - QueryCacher.clearVideoCache(videoInfo); - } - ); + await unHideSubmissions(categories, userID); return res.sendStatus(200); } + // otherwise ban already exists, send 409 return res.sendStatus(409); } @@ -133,3 +116,16 @@ export async function shadowBanUser(req: Request, res: Response): Promise `'${c}'`).join(",")}) + AND NOT EXISTS ( SELECT "videoID", "category" FROM "lockCategories" WHERE + "sponsorTimes"."videoID" = "lockCategories"."videoID" AND "sponsorTimes"."category" = "lockCategories"."category")`, [userID]); + + // clear cache for all old videos + (await db.prepare("all", `SELECT "videoID", "hashedVideoID", "service", "votes", "views" FROM "sponsorTimes" WHERE "userID" = ?`, [userID])) + .forEach((videoInfo: { category: Category; videoID: VideoID; hashedVideoID: VideoIDHash; service: Service; userID: UserID; }) => { + QueryCacher.clearVideoCache(videoInfo); + } + ); //eslint-disable-line +} \ No newline at end of file From d494c2305941e79b54f5ea7fe3cb0a32ba304416 Mon Sep 17 00:00:00 2001 From: "Michael M. Chang" Date: Sat, 28 Aug 2021 01:36:41 -0400 Subject: [PATCH 6/8] Update src/routes/postSkipSegments.ts Co-authored-by: Ajay Ramachandran --- src/routes/postSkipSegments.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/routes/postSkipSegments.ts b/src/routes/postSkipSegments.ts index 4b99d8b..a646fd0 100644 --- a/src/routes/postSkipSegments.ts +++ b/src/routes/postSkipSegments.ts @@ -382,7 +382,7 @@ async function checkEachSegmentValid(userID: string, videoID: VideoID } // Check for POI segments before 1 second - if (segments[i].category === CategoryActionType.POI && startTime < 1) { + if (getCategoryActionType(segments[i].category) === CategoryActionType.POI && startTime < 1) { return { pass: false, errorMessage: "POI must be after 1 second", errorCode: 400}; } From b3320ab0fd7f4bf48cb745f1c353fc9c4e326b08 Mon Sep 17 00:00:00 2001 From: Michael C Date: Sat, 28 Aug 2021 01:38:00 -0400 Subject: [PATCH 7/8] submit test as poi_highlight --- test/cases/postSkipSegments.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/cases/postSkipSegments.ts b/test/cases/postSkipSegments.ts index 2ae86ab..3751521 100644 --- a/test/cases/postSkipSegments.ts +++ b/test/cases/postSkipSegments.ts @@ -990,7 +990,7 @@ describe("postSkipSegments", () => { it("Should be rejected if a POI is at less than 1 second", (done: Done) => { fetch(`${getbaseURL() - }/api/skipSegments?videoID=qqwerty&startTime=0.5&endTime=0.5&userID=testtesttesttesttesttesttesttesttesting`, { + }/api/skipSegments?videoID=qqwerty&startTime=0.5&endTime=0.5&category=poi_highlight&userID=testtesttesttesttesttesttesttesttesting`, { method: "POST", }) .then(res => { From 5f8a319f48eddd5c2f4710d924ba2589fe0ae494 Mon Sep 17 00:00:00 2001 From: Michael C Date: Sun, 29 Aug 2021 14:58:12 -0400 Subject: [PATCH 8/8] update matrix link --- src/routes/postSkipSegments.ts | 4 ++-- test/cases/postSkipSegments.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/routes/postSkipSegments.ts b/src/routes/postSkipSegments.ts index 30cef25..243a301 100644 --- a/src/routes/postSkipSegments.ts +++ b/src/routes/postSkipSegments.ts @@ -298,7 +298,7 @@ async function checkUserActiveWarning(userID: string): Promise { if (warnings?.length >= config.maxNumberOfActiveWarnings) { const defaultMessage = "Submission rejected due to a warning from a moderator. This means that we noticed you were making some common mistakes" + " that are not malicious, and we just want to clarify the rules. " - + "Could you please send a message in discord.gg/SponsorBlock or matrix.to/#/+sponsor:ajay.app so we can further help you? " + + "Could you please send a message in discord.gg/SponsorBlock or matrix.to/#/#sponsor:ajay.app so we can further help you? " + `Your userID is ${userID}.`; return { @@ -375,7 +375,7 @@ async function checkEachSegmentValid(userID: string, videoID: VideoID `${lockedCategoryList[lockIndex].reason?.length !== 0 ? `\nLock reason: '${lockedCategoryList[lockIndex].reason}'` : ""}\n` + `${(segments[i].category === "sponsor" ? "\nMaybe the segment you are submitting is a different category that you have not enabled and is not a sponsor. " + "Categories that aren't sponsor, such as self-promotion can be enabled in the options.\n" : "")}` + - `\nIf you believe this is incorrect, please contact someone on discord.gg/SponsorBlock or matrix.to/#/+sponsor:ajay.app` + `\nIf you believe this is incorrect, please contact someone on discord.gg/SponsorBlock or matrix.to/#/#sponsor:ajay.app` }; } diff --git a/test/cases/postSkipSegments.ts b/test/cases/postSkipSegments.ts index cc61761..b1f9929 100644 --- a/test/cases/postSkipSegments.ts +++ b/test/cases/postSkipSegments.ts @@ -612,7 +612,7 @@ describe("postSkipSegments", () => { const userID = "09dee632bfbb1acc9fda3169cc14b46e459b45cee4f4449be305590e612b5eb7"; const expected = "Submission rejected due to a warning from a moderator. This means that we noticed you were making some common mistakes" + " that are not malicious, and we just want to clarify the rules. " - + "Could you please send a message in discord.gg/SponsorBlock or matrix.to/#/+sponsor:ajay.app so we can further help you? " + + "Could you please send a message in discord.gg/SponsorBlock or matrix.to/#/#sponsor:ajay.app so we can further help you? " + `Your userID is ${userID}.\n\nWarning reason: '${reason}'`; assert.strictEqual(errorMessage, expected);