From bfe8144fda15932b4aaf332b837c8337dd021ec2 Mon Sep 17 00:00:00 2001
From: tobi <31960611+tsmethurst@users.noreply.github.com>
Date: Tue, 28 Jan 2025 13:32:37 +0100
Subject: [bugfix] Allow processing null ID emojis (#3702)

* [bugfix] Allow processing null ID emojis

* document emojis

* blah

* typo

* array thingy
---
 internal/ap/extract.go             |  66 ++++++----
 internal/ap/extractemojis_test.go  | 255 +++++++++++++++++++++++++++++++++++++
 internal/typeutils/astointernal.go |   4 +-
 3 files changed, 298 insertions(+), 27 deletions(-)
 create mode 100644 internal/ap/extractemojis_test.go

(limited to 'internal')

diff --git a/internal/ap/extract.go b/internal/ap/extract.go
index 543ee8dca..02b72591c 100644
--- a/internal/ap/extract.go
+++ b/internal/ap/extract.go
@@ -805,7 +805,7 @@ func extractHashtag(i Hashtaggable) (*gtsmodel.Tag, error) {
 // ExtractEmojis extracts a slice of minimal gtsmodel.Emojis
 // from a WithTag. If an entry in the WithTag is not an emoji,
 // it will be quietly ignored.
-func ExtractEmojis(i WithTag) ([]*gtsmodel.Emoji, error) {
+func ExtractEmojis(i WithTag, host string) ([]*gtsmodel.Emoji, error) {
 	tagsProp := i.GetActivityStreamsTag()
 	if tagsProp == nil {
 		return nil, nil
@@ -827,7 +827,7 @@ func ExtractEmojis(i WithTag) ([]*gtsmodel.Emoji, error) {
 			continue
 		}
 
-		emoji, err := ExtractEmoji(tootEmoji)
+		emoji, err := ExtractEmoji(tootEmoji, host)
 		if err != nil {
 			return nil, err
 		}
@@ -844,41 +844,57 @@ func ExtractEmojis(i WithTag) ([]*gtsmodel.Emoji, error) {
 	return emojis, nil
 }
 
-// ExtractEmoji extracts a minimal gtsmodel.Emoji
-// from the given Emojiable.
-func ExtractEmoji(i Emojiable) (*gtsmodel.Emoji, error) {
-	// Use AP ID as emoji URI.
-	idProp := i.GetJSONLDId()
-	if idProp == nil || !idProp.IsIRI() {
-		return nil, gtserror.New("no id for emoji")
-	}
-	uri := idProp.GetIRI()
-
-	// Extract emoji last updated time (optional).
-	var updatedAt time.Time
-	updatedProp := i.GetActivityStreamsUpdated()
-	if updatedProp != nil && updatedProp.IsXMLSchemaDateTime() {
-		updatedAt = updatedProp.Get()
-	}
-
-	// Extract emoji name aka shortcode.
-	name := ExtractName(i)
+// ExtractEmoji extracts a minimal gtsmodel.Emoji from
+// the given Emojiable. The host (eg., "example.org")
+// of the emoji should be passed in as well, so that a
+// dummy URI for the emoji can be constructed in case
+// there's no id property or id property is null.
+//
+// https://github.com/superseriousbusiness/gotosocial/issues/3384)
+func ExtractEmoji(
+	e Emojiable,
+	host string,
+) (*gtsmodel.Emoji, error) {
+	// Extract emoji name,
+	// eg., ":some_emoji".
+	name := ExtractName(e)
 	if name == "" {
 		return nil, gtserror.New("name prop empty")
 	}
+	name = strings.TrimSpace(name)
+
+	// Derive shortcode from
+	// name, eg., "some_emoji".
 	shortcode := strings.Trim(name, ":")
+	shortcode = strings.TrimSpace(shortcode)
 
-	// Extract emoji image URL from Icon property.
-	imageRemoteURL, err := ExtractIconURI(i)
+	// Extract emoji image
+	// URL from Icon property.
+	imageRemoteURL, err := ExtractIconURI(e)
 	if err != nil {
 		return nil, gtserror.New("no url for emoji image")
 	}
 	imageRemoteURLStr := imageRemoteURL.String()
 
+	// Use AP ID as emoji URI, or fall
+	// back to dummy URI if not present.
+	uri := GetJSONLDId(e)
+	if uri == nil {
+		// No ID was set,
+		// construct dummy.
+		uri, err = url.Parse(
+			// eg., https://example.org/dummy_emoji_path?shortcode=some_emoji
+			"https://" + host + "/dummy_emoji_path?shortcode=" + url.QueryEscape(shortcode),
+		)
+		if err != nil {
+			return nil, gtserror.Newf("error constructing dummy path: %w", err)
+		}
+	}
+
 	return &gtsmodel.Emoji{
-		UpdatedAt:       updatedAt,
+		UpdatedAt:       GetUpdated(e),
 		Shortcode:       shortcode,
-		Domain:          uri.Host,
+		Domain:          host,
 		ImageRemoteURL:  imageRemoteURLStr,
 		URI:             uri.String(),
 		Disabled:        new(bool), // Assume false by default.
diff --git a/internal/ap/extractemojis_test.go b/internal/ap/extractemojis_test.go
new file mode 100644
index 000000000..69406f322
--- /dev/null
+++ b/internal/ap/extractemojis_test.go
@@ -0,0 +1,255 @@
+// GoToSocial
+// Copyright (C) GoToSocial Authors admin@gotosocial.org
+// SPDX-License-Identifier: AGPL-3.0-or-later
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package ap_test
+
+import (
+	"bytes"
+	"context"
+	"io"
+	"testing"
+
+	"github.com/stretchr/testify/suite"
+	"github.com/superseriousbusiness/gotosocial/internal/ap"
+)
+
+type ExtractEmojisTestSuite struct {
+	APTestSuite
+}
+
+func (suite *ExtractEmojisTestSuite) TestExtractEmojis() {
+	const noteWithEmojis = `{
+  "@context": [
+    "https://gotosocial.org/ns",
+    "https://www.w3.org/ns/activitystreams",
+    {
+      "Emoji": "toot:Emoji",
+      "sensitive": "as:sensitive",
+      "toot": "http://joinmastodon.org/ns#"
+    }
+  ],
+  "attributedTo": "https://example.org/users/tobi",
+  "content": "<p>i hear that the GoToSocial devs are anti-capitalists and even <em>shocked gasp</em> communists :shocked_pikachu: totally unreasonable people</p>",
+  "id": "https://example.org/users/tobi/statuses/01HV11D2BS7M94ZS499VBW7RX5",
+  "tag": {
+    "icon": {
+      "mediaType": "image/png",
+      "type": "Image",
+      "url": "https://example.org/fileserver/01BPSX2MKCRVMD4YN4D71G9CP5/emoji/original/01AZY1Y5YQD6TREB5W50HGTCSZ.png"
+    },
+    "id": "https://example.org/emoji/01AZY1Y5YQD6TREB5W50HGTCSZ",
+    "name": ":shocked_pikachu:",
+    "type": "Emoji",
+    "updated": "2022-11-17T11:36:05Z"
+  },
+  "to": "https://www.w3.org/ns/activitystreams#Public",
+  "type": "Note"
+}`
+
+	statusable, err := ap.ResolveStatusable(
+		context.Background(),
+		io.NopCloser(bytes.NewBufferString(noteWithEmojis)),
+	)
+	if err != nil {
+		suite.FailNow(err.Error())
+	}
+
+	emojis, err := ap.ExtractEmojis(statusable, "example.org")
+	if err != nil {
+		suite.FailNow(err.Error())
+	}
+
+	if l := len(emojis); l != 1 {
+		suite.FailNow("", "expected length 1 for emojis, got %d", l)
+	}
+
+	emoji := emojis[0]
+	suite.Equal("shocked_pikachu", emoji.Shortcode)
+	suite.Equal("example.org", emoji.Domain)
+	suite.Equal("https://example.org/fileserver/01BPSX2MKCRVMD4YN4D71G9CP5/emoji/original/01AZY1Y5YQD6TREB5W50HGTCSZ.png", emoji.ImageRemoteURL)
+	suite.False(*emoji.Disabled)
+	suite.Equal("https://example.org/emoji/01AZY1Y5YQD6TREB5W50HGTCSZ", emoji.URI)
+	suite.False(*emoji.VisibleInPicker)
+}
+
+func (suite *ExtractEmojisTestSuite) TestExtractEmojisNoID() {
+	const noteWithEmojis = `{
+  "@context": [
+    "https://gotosocial.org/ns",
+    "https://www.w3.org/ns/activitystreams",
+    {
+      "Emoji": "toot:Emoji",
+      "sensitive": "as:sensitive",
+      "toot": "http://joinmastodon.org/ns#"
+    }
+  ],
+  "attributedTo": "https://example.org/users/tobi",
+  "content": "<p>i hear that the GoToSocial devs are anti-capitalists and even <em>shocked gasp</em> communists :shocked_pikachu: totally unreasonable people</p>",
+  "id": "https://example.org/users/tobi/statuses/01HV11D2BS7M94ZS499VBW7RX5",
+  "tag": {
+    "icon": {
+      "mediaType": "image/png",
+      "type": "Image",
+      "url": "https://example.org/fileserver/01BPSX2MKCRVMD4YN4D71G9CP5/emoji/original/01AZY1Y5YQD6TREB5W50HGTCSZ.png"
+    },
+    "name": ":shocked_pikachu:",
+    "type": "Emoji",
+    "updated": "2022-11-17T11:36:05Z"
+  },
+  "to": "https://www.w3.org/ns/activitystreams#Public",
+  "type": "Note"
+}`
+
+	statusable, err := ap.ResolveStatusable(
+		context.Background(),
+		io.NopCloser(bytes.NewBufferString(noteWithEmojis)),
+	)
+	if err != nil {
+		suite.FailNow(err.Error())
+	}
+
+	emojis, err := ap.ExtractEmojis(statusable, "example.org")
+	if err != nil {
+		suite.FailNow(err.Error())
+	}
+
+	if l := len(emojis); l != 1 {
+		suite.FailNow("", "expected length 1 for emojis, got %d", l)
+	}
+
+	emoji := emojis[0]
+	suite.Equal("shocked_pikachu", emoji.Shortcode)
+	suite.Equal("example.org", emoji.Domain)
+	suite.Equal("https://example.org/fileserver/01BPSX2MKCRVMD4YN4D71G9CP5/emoji/original/01AZY1Y5YQD6TREB5W50HGTCSZ.png", emoji.ImageRemoteURL)
+	suite.False(*emoji.Disabled)
+	suite.Equal("https://example.org/dummy_emoji_path?shortcode=shocked_pikachu", emoji.URI)
+	suite.False(*emoji.VisibleInPicker)
+}
+
+func (suite *ExtractEmojisTestSuite) TestExtractEmojisNullID() {
+	const noteWithEmojis = `{
+  "@context": [
+    "https://gotosocial.org/ns",
+    "https://www.w3.org/ns/activitystreams",
+    {
+      "Emoji": "toot:Emoji",
+      "sensitive": "as:sensitive",
+      "toot": "http://joinmastodon.org/ns#"
+    }
+  ],
+    "attributedTo": "https://example.org/users/tobi",
+  "content": "<p>i hear that the GoToSocial devs are anti-capitalists and even <em>shocked gasp</em> communists :shocked_pikachu: totally unreasonable people</p>",
+  "id": "https://example.org/users/tobi/statuses/01HV11D2BS7M94ZS499VBW7RX5",
+  "tag": {
+    "icon": {
+      "mediaType": "image/png",
+      "type": "Image",
+      "url": "https://example.org/fileserver/01BPSX2MKCRVMD4YN4D71G9CP5/emoji/original/01AZY1Y5YQD6TREB5W50HGTCSZ.png"
+    },
+    "id": null,
+    "name": ":shocked_pikachu:",
+    "type": "Emoji",
+    "updated": "2022-11-17T11:36:05Z"
+  },
+  "to": "https://www.w3.org/ns/activitystreams#Public",
+  "type": "Note"
+}`
+
+	statusable, err := ap.ResolveStatusable(
+		context.Background(),
+		io.NopCloser(bytes.NewBufferString(noteWithEmojis)),
+	)
+	if err != nil {
+		suite.FailNow(err.Error())
+	}
+
+	emojis, err := ap.ExtractEmojis(statusable, "example.org")
+	if err != nil {
+		suite.FailNow(err.Error())
+	}
+
+	if l := len(emojis); l != 1 {
+		suite.FailNow("", "expected length 1 for emojis, got %d", l)
+	}
+
+	emoji := emojis[0]
+	suite.Equal("shocked_pikachu", emoji.Shortcode)
+	suite.Equal("example.org", emoji.Domain)
+	suite.Equal("https://example.org/fileserver/01BPSX2MKCRVMD4YN4D71G9CP5/emoji/original/01AZY1Y5YQD6TREB5W50HGTCSZ.png", emoji.ImageRemoteURL)
+	suite.False(*emoji.Disabled)
+	suite.Equal("https://example.org/dummy_emoji_path?shortcode=shocked_pikachu", emoji.URI)
+	suite.False(*emoji.VisibleInPicker)
+}
+
+func (suite *ExtractEmojisTestSuite) TestExtractEmojisEmptyID() {
+	const noteWithEmojis = `{
+  "@context": [
+    "https://gotosocial.org/ns",
+    "https://www.w3.org/ns/activitystreams",
+    {
+      "Emoji": "toot:Emoji",
+      "sensitive": "as:sensitive",
+      "toot": "http://joinmastodon.org/ns#"
+    }
+  ],
+    "attributedTo": "https://example.org/users/tobi",
+  "content": "<p>i hear that the GoToSocial devs are anti-capitalists and even <em>shocked gasp</em> communists :shocked_pikachu: totally unreasonable people</p>",
+  "id": "https://example.org/users/tobi/statuses/01HV11D2BS7M94ZS499VBW7RX5",
+  "tag": {
+    "icon": {
+      "mediaType": "image/png",
+      "type": "Image",
+      "url": "https://example.org/fileserver/01BPSX2MKCRVMD4YN4D71G9CP5/emoji/original/01AZY1Y5YQD6TREB5W50HGTCSZ.png"
+    },
+    "id": "",
+    "name": ":shocked_pikachu:",
+    "type": "Emoji",
+    "updated": "2022-11-17T11:36:05Z"
+  },
+  "to": "https://www.w3.org/ns/activitystreams#Public",
+  "type": "Note"
+}`
+
+	statusable, err := ap.ResolveStatusable(
+		context.Background(),
+		io.NopCloser(bytes.NewBufferString(noteWithEmojis)),
+	)
+	if err != nil {
+		suite.FailNow(err.Error())
+	}
+
+	emojis, err := ap.ExtractEmojis(statusable, "example.org")
+	if err != nil {
+		suite.FailNow(err.Error())
+	}
+
+	if l := len(emojis); l != 1 {
+		suite.FailNow("", "expected length 1 for emojis, got %d", l)
+	}
+
+	emoji := emojis[0]
+	suite.Equal("shocked_pikachu", emoji.Shortcode)
+	suite.Equal("example.org", emoji.Domain)
+	suite.Equal("https://example.org/fileserver/01BPSX2MKCRVMD4YN4D71G9CP5/emoji/original/01AZY1Y5YQD6TREB5W50HGTCSZ.png", emoji.ImageRemoteURL)
+	suite.False(*emoji.Disabled)
+	suite.Equal("https://example.org/dummy_emoji_path?shortcode=shocked_pikachu", emoji.URI)
+	suite.False(*emoji.VisibleInPicker)
+}
+
+func TestExtractEmojisTestSuite(t *testing.T) {
+	suite.Run(t, &ExtractEmojisTestSuite{})
+}
diff --git a/internal/typeutils/astointernal.go b/internal/typeutils/astointernal.go
index 0ad9a6ff7..16aa430a3 100644
--- a/internal/typeutils/astointernal.go
+++ b/internal/typeutils/astointernal.go
@@ -149,7 +149,7 @@ func (c *Converter) ASRepresentationToAccount(
 	}
 
 	// account emojis (used in bio, display name, fields)
-	acct.Emojis, err = ap.ExtractEmojis(accountable)
+	acct.Emojis, err = ap.ExtractEmojis(accountable, acct.Domain)
 	if err != nil {
 		log.Warnf(ctx, "error(s) extracting account emojis for %s: %v", uri, err)
 	}
@@ -325,7 +325,7 @@ func (c *Converter) ASStatusToStatus(ctx context.Context, statusable ap.Statusab
 	// status.Emojis
 	//
 	// Custom emojis for later dereferencing.
-	if emojis, err := ap.ExtractEmojis(statusable); err != nil {
+	if emojis, err := ap.ExtractEmojis(statusable, uriObj.Host); err != nil {
 		log.Warnf(ctx, "error extracting emojis for %s: %v", uri, err)
 	} else {
 		status.Emojis = emojis
-- 
cgit v1.2.3