Merge pull request #1571 from Human-Connection/1276-hashtag-links-url-safe

🍰 Make hashtag links URL safe

backend/src/middleware/hashtags/extractHashtags.js

@@ -4,23 +4,23 @@ import { exec, build } from 'xregexp/xregexp-all.js'
 //   https://en.wikipedia.org/w/index.php?title=Hashtag&oldid=905141980#Style
 // here:
 //    0. Search for whole string.
-//    1. Hashtag has only all unicode characters and '0-9'.
-//    2. If it starts with a digit '0-9' than a unicode character has to follow.
-const regX = build('^/search/hashtag/((\\pL+[\\pL0-9]*)|([0-9]+\\pL+[\\pL0-9]*))$')
+//    1. Hashtag has only all unicode letters and '0-9'.
+//    2. If it starts with a digit '0-9' than a unicode letter has to follow.
+const regX = build('^((\\pL+[\\pL0-9]*)|([0-9]+\\pL+[\\pL0-9]*))$')
 
 export default function(content) {
   if (!content) return []
   const $ = cheerio.load(content)
   // We can not search for class '.hashtag', because the classes are removed at the 'xss' middleware.
   //   But we have to know, which Hashtags are removed from the content as well, so we search for the 'a' html-tag.
-  const urls = $('a')
+  const ids = $('a[data-hashtag-id]')
     .map((_, el) => {
-      return $(el).attr('href')
+      return $(el).attr('data-hashtag-id')
     })
     .get()
   const hashtags = []
-  urls.forEach(url => {
-    const match = exec(url, regX)
+  ids.forEach(id => {
+    const match = exec(id, regX)
     if (match != null) {
       hashtags.push(match[1])
     }

backend/src/middleware/hashtags/extractHashtags.spec.js

@@ -8,9 +8,24 @@ describe('extractHashtags', () => {
   })
 
   describe('searches through links', () => {
-    it('finds links with and without ".hashtag" class and extracts Hashtag names', () => {
-      const content =
-        '<p><a class="hashtag" href="/search/hashtag/Elections">#Elections</a><a href="/search/hashtag/Democracy">#Democracy</a></p>'
+    it('without `class="hashtag"` but `data-hashtag-id="something"`, and extracts the Hashtag to make a Hashtag link', () => {
+      const content = `
+        <p>
+          <a
+            class="hashtag"
+            data-hashtag-id="Elections"
+            href="/?hashtag=Elections"
+          >
+            #Elections
+          </a>
+          <a
+            data-hashtag-id="Democracy"
+            href="/?hashtag=Democracy"
+          >
+            #Democracy
+          </a>
+        </p>
+      `
       expect(extractHashtags(content)).toEqual(['Elections', 'Democracy'])
     })
 
@@ -20,23 +35,57 @@ describe('extractHashtags', () => {
       expect(extractHashtags(content)).toEqual([])
     })
 
-    describe('handles links', () => {
-      it('ignores links with domains', () => {
-        const content =
-          '<p><a class="hashtag" href="http://localhost:3000/search/hashtag/Elections">#Elections</a><a href="/search/hashtag/Democracy">#Democracy</a></p>'
-        expect(extractHashtags(content)).toEqual(['Democracy'])
-      })
-
-      it('ignores Hashtag links with not allowed character combinations', () => {
-        // Allowed are all unicode letters '\pL' and all digits '0-9'. There haveto be at least one letter in it.
-        const content =
-          '<p>Something inspirational about <a href="/search/hashtag/AbcDefXyz0123456789!*(),2" class="hashtag" target="_blank">#AbcDefXyz0123456789!*(),2</a>, <a href="/search/hashtag/0123456789" class="hashtag" target="_blank">#0123456789</a>, <a href="/search/hashtag/0123456789a" class="hashtag" target="_blank">#0123456789a</a>, <a href="/search/hashtag/AbcDefXyz0123456789" target="_blank">#AbcDefXyz0123456789</a>, and <a href="/search/hashtag/λαπ" target="_blank">#λαπ</a>.</p>'
-        expect(extractHashtags(content).sort()).toEqual([
-          '0123456789a',
-          'AbcDefXyz0123456789',
-          'λαπ',
-        ])
-      })
+    it('ignores hashtag links with unsupported character combinations', () => {
+      // Allowed are all unicode letters '\pL' and all digits '0-9'. There haveto be at least one letter in it.
+      const content = `
+      <p>
+        Something inspirational about
+        <a
+          href="/?hashtag=AbcDefXyz0123456789!*(),2"
+          data-hashtag-id="AbcDefXyz0123456789!*(),2"
+          class="hashtag"
+          target="_blank"
+        >
+          #AbcDefXyz0123456789!*(),2
+        </a>,
+        <a
+          href="/?hashtag=0123456789"
+          data-hashtag-id="0123456789"
+          class="hashtag"
+          target="_blank"
+        >
+          #0123456789
+        </a>,
+        <a href="?hashtag=0123456789a"
+          data-hashtag-id="0123456789a"
+          class="hashtag"
+          target="_blank"
+        >
+          #0123456789a
+        </a>,
+        <a
+          href="/?hashtag=AbcDefXyz0123456789"
+          data-hashtag-id="AbcDefXyz0123456789"
+          class="hashtag"
+          target="_blank"
+        >
+          #AbcDefXyz0123456789
+        </a>, and
+        <a
+          href="/?hashtag=%C4%A7%CF%80%CE%B1%CE%BB"
+          data-hashtag-id="ħπαλ"
+          class="hashtag"
+          target="_blank"
+        >
+          #ħπαλ
+        </a>.
+      </p>
+      `
+      expect(extractHashtags(content).sort()).toEqual([
+        '0123456789a',
+        'AbcDefXyz0123456789',
+        'ħπαλ',
+      ])
     })
 
     describe('does not crash if', () => {

backend/src/middleware/hashtags/hashtagsMiddleware.spec.js

@@ -69,8 +69,27 @@ afterEach(async () => {
 describe('hashtags', () => {
   const id = 'p135'
   const title = 'Two Hashtags'
-  const postContent =
-    '<p>Hey Dude, <a class="hashtag" href="/search/hashtag/Democracy">#Democracy</a> should work equal for everybody!? That seems to be the only way to have equal <a class="hashtag" href="/search/hashtag/Liberty">#Liberty</a> for everyone.</p>'
+  const postContent = `
+    <p>
+      Hey Dude,
+      <a
+        class="hashtag"
+        data-hashtag-id="Democracy"
+        href="/?hashtag=Democracy">
+          #Democracy
+      </a>
+      should work equal for everybody!? That seems to be the only way to have
+      equal
+      <a
+        class="hashtag"
+        data-hashtag-id="Liberty"
+        href="/?hashtag=Liberty"
+      >
+        #Liberty
+      </a>
+      for everyone.
+    </p>
+  `
   const postWithHastagsQuery = gql`
     query($id: ID) {
       Post(id: $id) {
@@ -129,10 +148,29 @@ describe('hashtags', () => {
         )
       })
 
-      describe('afterwards update the Post by removing a Hashtag, leaving a Hashtag and add a Hashtag', () => {
-        // The already existing Hashtag has no class at this point.
-        const postContent =
-          '<p>Hey Dude, <a class="hashtag" href="/search/hashtag/Elections">#Elections</a> should work equal for everybody!? That seems to be the only way to have equal <a href="/search/hashtag/Liberty">#Liberty</a> for everyone.</p>'
+      describe('updates the Post by removing, keeping and adding one hashtag respectively', () => {
+        // The already existing hashtag has no class at this point.
+        const postContent = `
+          <p>
+            Hey Dude,
+            <a
+              class="hashtag"
+              data-hashtag-id="Elections"
+              href="?hashtag=Elections"
+            >
+              #Elections
+            </a>
+            should work equal for everybody!? That seems to be the only way to
+            have equal
+            <a
+              data-hashtag-id="Liberty"
+              href="?hashtag=Liberty"
+            >
+              #Liberty
+            </a>
+            for everyone.
+          </p>
+        `
 
         it('only one previous Hashtag and the new Hashtag exists', async () => {
           await mutate({

backend/src/seed/seed-db.js

@@ -413,9 +413,9 @@ import { gql } from '../jest/helpers'
     const mention2 =
       'Hey <a class="mention" data-mention-id="u3" href="/profile/u3">@jenny-rostock</a>, here is another notification for you!'
     const hashtag1 =
-      'See <a class="hashtag" href="/search/hashtag/NaturphilosophieYoga">#NaturphilosophieYoga</a> can really help you!'
+      'See <a class="hashtag" data-hashtag-id="NaturphilosophieYoga" href="/?hashtag=NaturphilosophieYoga">#NaturphilosophieYoga</a>, it can really help you!'
     const hashtagAndMention1 =
-      'The new physics of <a class="hashtag" href="/search/hashtag/QuantenFlussTheorie">#QuantenFlussTheorie</a> can explain <a class="hashtag" href="/search/hashtag/QuantumGravity">#QuantumGravity</a>! <a class="mention" data-mention-id="u1" href="/profile/u1">@peter-lustig</a> got that already. ;-)'
+      'The new physics of <a class="hashtag" data-hashtag-id="QuantenFlussTheorie" href="/?hashtag=QuantenFlussTheorie">#QuantenFlussTheorie</a> can explain <a class="hashtag" data-hashtag-id="QuantumGravity" href="/?hashtag=QuantumGravity">#QuantumGravity</a>! <a class="mention" data-mention-id="u1" href="/profile/u1">@peter-lustig</a> got that already. ;-)'
     const createPostMutation = gql`
       mutation($id: ID, $title: String!, $content: String!, $categoryIds: [ID]) {
         CreatePost(id: $id, title: $title, content: $content, categoryIds: $categoryIds) {
@@ -470,9 +470,9 @@ import { gql } from '../jest/helpers'
 
     authenticatedUser = await dewey.toJson()
     const mentionInComment1 =
-      'I heard <a class="mention" data-mention-id="u3" href="/profile/u3">@jenny-rostock</a>, practice it since 3 years now.'
+      'I heard <a class="mention" data-mention-id="u3" href="/profile/u3">@jenny-rostock</a> has practiced it for 3 years now.'
     const mentionInComment2 =
-      'Did <a class="mention" data-mention-id="u1" href="/profile/u1">@peter-lustig</a> told you?'
+      'Did <a class="mention" data-mention-id="u1" href="/profile/u1">@peter-lustig</a> tell you?'
     const createCommentMutation = gql`
       mutation($id: ID, $postId: ID!, $content: String!) {
         CreateComment(id: $id, postId: $postId, content: $content) {
@@ -661,21 +661,21 @@ import { gql } from '../jest/helpers'
       mutate({
         mutation: reportMutation,
         variables: {
-          description: "I don't like this comment",
+          description: 'This comment is bigoted',
           id: 'c1',
         },
       }),
       mutate({
         mutation: reportMutation,
         variables: {
-          description: "I don't like this post",
+          description: 'This post is bigoted',
           id: 'p1',
         },
       }),
       mutate({
         mutation: reportMutation,
         variables: {
-          description: "I don't like this user",
+          description: 'This user is harassing me with bigoted remarks',
           id: 'u1',
         },
       }),

webapp/components/Editor/ContentViewer.vue

@@ -4,6 +4,7 @@
 
 <script>
 import defaultExtensions from './defaultExtensions.js'
+import Hashtag from './nodes/Hashtag.js'
 import { Editor, EditorContent } from 'tiptap'
 
 export default {
@@ -21,7 +22,12 @@ export default {
         doc: this.doc,
         content: this.content,
         editable: false,
-        extensions: defaultExtensions(this),
+        extensions: [
+          // Hashtags must come first, see
+          // https://github.com/scrumpy/tiptap/issues/421#issuecomment-523037460
+          new Hashtag(),
+          ...defaultExtensions(this),
+        ],
       }),
     }
   },

webapp/components/Editor/Editor.vue

@@ -134,10 +134,12 @@ export default {
       content: this.value || '',
       doc: this.doc,
       extensions: [
+        // Hashtags must come first, see
+        // https://github.com/scrumpy/tiptap/issues/421#issuecomment-523037460
+        ...this.optionalExtensions,
         ...defaultExtensions(this),
         new EventHandler(),
         new History(),
-        ...this.optionalExtensions,
       ],
       onUpdate: e => {
         clearTimeout(throttleInputEvent)

webapp/components/Editor/nodes/Hashtag.js

@@ -21,11 +21,15 @@ export default class Hashtag extends TipTapMention {
     return {
       ...super.schema,
       toDOM: node => {
+        // use a dummy domain because URL cannot handle relative urls
+        const url = new URL('/', 'https://human-connection.org')
+        url.searchParams.append('hashtag', node.attrs.id)
+
         return [
           'a',
           {
             class: this.options.mentionClass,
-            href: `/search/hashtag/${node.attrs.id}`,
+            href: `/${url.search}`,
             'data-hashtag-id': node.attrs.id,
             target: '_blank',
           },
@@ -33,8 +37,14 @@ export default class Hashtag extends TipTapMention {
         ]
       },
       parseDOM: [
-        // simply don't parse mentions from html
-        // just treat them as normal links
+        {
+          tag: 'a[data-hashtag-id]',
+          getAttrs: dom => {
+            const id = dom.getAttribute('data-hashtag-id')
+            const label = dom.innerText.split(this.options.matcher.char).join('')
+            return { id, label }
+          },
+        },
       ],
     }
   }

webapp/components/Editor/nodes/Link.js

@@ -15,7 +15,8 @@ export default class Link extends TipTapLink {
       inclusive: false,
       parseDOM: [
         {
-          tag: 'a[href]:not(.embed)', // do not trigger on embed links
+          // if this is an embed link or a hashtag, ignore
+          tag: 'a[href]:not(.embed):not([data-hashtag-id])',
           getAttrs: dom => ({
             href: dom.getAttribute('href'),
           }),

webapp/locales/de.json

@@ -269,7 +269,7 @@
       "categoryName": "Name",
       "postCount": "Beiträge"
     },
-    "tags": {
+    "hashtags": {
       "name": "Hashtags",
       "number": "Nr.",
       "nameOfHashtag": "Name",

webapp/locales/en.json

@@ -270,7 +270,7 @@
       "categoryName": "Name",
       "postCount": "Posts"
     },
-    "tags": {
+    "hashtags": {
       "name": "Hashtags",
       "number": "No.",
       "nameOfHashtag": "Name",

webapp/pages/admin.vue

@@ -48,8 +48,8 @@ export default {
           path: `/admin/categories`,
         },
         {
-          name: this.$t('admin.tags.name'),
-          path: `/admin/tags`,
+          name: this.$t('admin.hashtags.name'),
+          path: `/admin/hashtags`,
         },
         {
           name: this.$t('admin.invites.name'),

webapp/pages/admin/hashtags.vue

@@ -1,11 +1,11 @@
 <template>
-  <ds-card :header="$t('admin.tags.name')">
+  <ds-card :header="$t('admin.hashtags.name')">
     <ds-table :data="Tag" :fields="fields" condensed>
       <template slot="index" slot-scope="scope">
         {{ scope.index + 1 }}.
       </template>
       <template slot="id" slot-scope="scope">
-        <nuxt-link :to="{ path: '/', query: { hashtag: scope.row.id } }">
+        <nuxt-link :to="{ path: '/', query: { hashtag: encodeURI(scope.row.id) } }">
           <b>#{{ scope.row.id | truncate(20) }}</b>
         </nuxt-link>
       </template>
@@ -25,14 +25,14 @@ export default {
   computed: {
     fields() {
       return {
-        index: this.$t('admin.tags.number'),
-        id: this.$t('admin.tags.name'),
+        index: this.$t('admin.hashtags.number'),
+        id: this.$t('admin.hashtags.name'),
         taggedCountUnique: {
-          label: this.$t('admin.tags.tagCountUnique'),
+          label: this.$t('admin.hashtags.tagCountUnique'),
           align: 'right',
         },
         taggedCount: {
-          label: this.$t('admin.tags.tagCount'),
+          label: this.$t('admin.hashtags.tagCount'),
           align: 'right',
         },
       }

webapp/pages/search/hashtag/_id.vue

@@ -1,12 +0,0 @@
-<template>
-  <div></div>
-</template>
-
-<script>
-export default {
-  mounted() {
-    const { id: hashtag } = this.$route.params
-    this.$router.push({ path: '/', query: { hashtag } })
-  },
-}
-</script>