Fix shortlink resolution bug (#1044)

* wip * fixes shortlink resolution glitch --------- Co-authored-by: Karnikaa Velumani <[email protected]>
EthanThatOneKid · Mar 18, 2024 · ecc7720 · ecc7720
1 parent 49fa6cc
commit ecc7720
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 66 deletions.
diff --git a/src/lib/server/links/resolve.test.ts b/src/lib/server/links/resolve.test.ts
@@ -1,103 +1,88 @@
 import { test, expect } from 'vitest';
 import { resolve } from './resolve';
 
+test('resolve resolves shortlinks', () => {
+  const actual = resolve(new URL('https://acmcsuf.com/github'), {
+    github: 'https://github.com/ethanthatonekid/acmcsuf.com',
+  });
+  const expected = new URL('https://github.com/ethanthatonekid/acmcsuf.com');
+  expect(actual.href).toStrictEqual(expected.href);
+});
+
 test('resolve throws for circularly recursive shortlinks', () => {
-  expect(() =>
-    resolve(new URL('https://acmcsuf.com/zig'), { zig: '/zag', zag: '/zig' })
-  ).toThrowError(/too many internal redirects/);
+  function testCircularShortlinks() {
+    return resolve(new URL('https://acmcsuf.com/zig'), { zig: '/zag', zag: '/zig' });
+  }
+
+  expect(testCircularShortlinks).toThrowError(/too many internal redirects/);
 });
 
 test('resolve properly combines queries', () => {
-  const out = resolve(new URL('https://acmcsuf.com/example?foo=bar'), {
+  const actual = resolve(new URL('https://acmcsuf.com/example?foo=bar'), {
     example: 'https://example.com?baz=qux',
   });
-  expect(out).toStrictEqual(new URL('https://example.com/?baz=qux&foo=bar'));
+  const expected = new URL('https://example.com/?baz=qux&foo=bar');
+  expect(actual.href).toStrictEqual(expected.href);
 });
 
 test('resolve properly overwrites hash', () => {
-  const out = resolve(new URL('https://acmcsuf.com/example#yang'), {
+  const actual = resolve(new URL('https://acmcsuf.com/example#yang'), {
     example: 'https://example.com#yin',
   });
-  expect(out).toStrictEqual(new URL('https://example.com/#yang'));
+  expect(actual).toStrictEqual(new URL('https://example.com/#yang'));
 });
 
 test('resolve overwrites hash (2)', () => {
-  const out = resolve(new URL('https://acmcsuf.com/one#uno'), {
+  const actual = resolve(new URL('https://acmcsuf.com/one#uno'), {
     one: '/two',
     two: '/three#dos',
     three: '/example#tres',
     example: 'https://example.com',
   });
-  expect(out).toStrictEqual(new URL('https://example.com/#uno'));
+  const expected = new URL('https://example.com/#uno');
+  expect(actual.href).toStrictEqual(expected.href);
 });
 
 test('resolve appends pathnames', () => {
-  const out = resolve(new URL('https://acmcsuf.com/example/baz/qux'), {
+  const actual = resolve(new URL('https://acmcsuf.com/example/baz/qux'), {
     example: 'https://example.com/foo/bar',
   });
-  expect(out).toStrictEqual(new URL('https://example.com/foo/bar/baz/qux'));
+  const expected = new URL('https://example.com/foo/bar/baz/qux');
+  expect(actual.href).toStrictEqual(expected.href);
 });
 
-const TEST_SHORTLINKS = {
-  github: 'https://github.com/ethanthatonekid/acmcsuf.com',
-  issues: '/github/issues',
-  gfi: '/issues/?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22',
-  bug: '/issues/new/?labels=bug&template=bug_report.md',
-  'dev/template': 'https://example.com/dev-template',
-  'student-pack': '/blog/806',
-};
+test('resolve appends pathnames only if separated by / or end of string', () => {
+  const shortlinks = { c: 'https://example.com/calendar' };
+  const actual1 = resolve(new URL('https://acmcsuf.com/colors'), shortlinks);
+  const expected1 = new URL('https://acmcsuf.com/colors');
+  expect(actual1.href).toStrictEqual(expected1.href);
 
-const VALID_TESTCASES: Array<[URL, ReturnType<typeof resolve>]> = [
-  [
-    new URL('https://acmcsuf.com/github'),
-    new URL('https://github.com/ethanthatonekid/acmcsuf.com'),
-  ],
-  [
-    new URL('https://acmcsuf.com/issues'),
-    new URL('https://github.com/ethanthatonekid/acmcsuf.com/issues'),
-  ],
-  [
-    new URL('https://acmcsuf.com/gfi'),
-    new URL(
-      'https://github.com/ethanthatonekid/acmcsuf.com/issues/?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22'
-    ),
-  ],
-  [
-    new URL('https://acmcsuf.com/bug'),
-    new URL(
-      'https://github.com/ethanthatonekid/acmcsuf.com/issues/new/?labels=bug&template=bug_report.md'
-    ),
-  ],
-  [
-    new URL('https://acmcsuf.com/issues/new'),
-    new URL('https://github.com/ethanthatonekid/acmcsuf.com/issues/new'),
-  ],
-  [
-    new URL('https://acmcsuf.com/dev/template/new'),
-    new URL('https://example.com/dev-template/new'),
-  ],
-];
+  const actual2 = resolve(new URL('https://acmcsuf.com/c'), shortlinks);
+  const expected2 = new URL('https://example.com/calendar');
+  expect(actual2.href).toStrictEqual(expected2.href);
+});
 
-test('resolve resolves valid URLs', () => {
-  for (const [inURL, outURL] of VALID_TESTCASES) {
-    expect(resolve(inURL, TEST_SHORTLINKS), `failed on ${inURL}`).toStrictEqual(outURL);
-  }
+test('resolve resolves relative paths', () => {
+  const actual = resolve(new URL('https://acmcsuf.com/student-pack'), {
+    'student-pack': '/blog/806',
+  });
+  const expected = new URL('https://acmcsuf.com/blog/806');
+  expect(actual.href).toStrictEqual(expected.href);
 });
 
 test('resolve resolves alias shortlink', () => {
-  const inURL = new URL('https://acmcsuf.com/student-pack');
-  const outURL = new URL('https://acmcsuf.com/blog/806');
-  expect(resolve(inURL, TEST_SHORTLINKS), `failed on ${inURL}`).toStrictEqual(outURL);
+  const expected = new URL('https://acmcsuf.com/blog/806');
+  const input = new URL('https://acmcsuf.com/student-pack');
+  const actual = resolve(input, { 'student-pack': '/blog/806' });
+  expect(actual.href).toStrictEqual(expected.href);
 });
 
-const INVALID_TESTCASES: Array<URL> = [
-  new URL('https://acmcsuf.com/doesnotexist'),
-  new URL('https://acmcsuf.com/does/not/exist'),
-  new URL('https://acmcsuf.com/<invalid>'),
-];
-
 test('resolve returns passed URL if invalid or not found', () => {
-  for (const inURL of INVALID_TESTCASES) {
-    expect(resolve(inURL, TEST_SHORTLINKS), `failed on ${inURL}`).toStrictEqual(inURL);
-  }
+  [
+    new URL('https://acmcsuf.com/doesnotexist'),
+    new URL('https://acmcsuf.com/does/not/exist'),
+    new URL('https://acmcsuf.com/<invalid>'),
+  ].forEach((input) => {
+    expect(resolve(input, {}), `failed on ${input}`).toStrictEqual(input);
+  });
 });
diff --git a/src/lib/server/links/resolve.ts b/src/lib/server/links/resolve.ts
@@ -54,7 +54,12 @@ function findURL<ID extends string>(
 
     let id = pathname.slice(1) as ID;
     while (id.length > 0 && !shortlinks[id]) {
-      id = id.slice(0, id.lastIndexOf('/')) as ID;
+      const slashIndex = id.lastIndexOf('/');
+      if (slashIndex === -1) {
+        break;
+      }
+
+      id = id.slice(0, slashIndex) as ID;
     }
 
     if (!shortlinks[id]) {