d->m: Try too-large PNGs as JPEG for upload

Large PNGs were almost certainly originally JPEG photos that were
converted to PNG by the user copy-pasting.
If the PNG is just supposed to be that large, you can still get the
original through the external URL option.

Had to change _removeExpiryParams to be based on URL parsing instead of
regexp because regexp didn't work with &format=jpeg on the end.

readme.md

@@ -8,6 +8,10 @@ Modern Matrix-to-Discord appservice bridge, created by [@cadence:cadence.moe](ht
 
 ![](https://cadence.moe/i/f42a3f)
 
+## AI policy
+
+Out Of Your Element is 100% organically cultivated ethical hand-fed code. Let's work together to conserve that!
+
 ## Why a new bridge?
 
 * Modern: Supports new Discord features like replies, threads and stickers, and new Matrix features like edits, spaces and space membership.

src/d2m/actions/register-pk-user.js

@@ -3,6 +3,7 @@
 const assert = require("assert").strict
 const {reg} = require("../../matrix/read-registration")
 const Ty = require("../../types")
+const {scheduler} = require("timers/promises")
 
 const passthrough = require("../../passthrough")
 const {sync, db, select, from} = passthrough
@@ -14,14 +15,21 @@ const file = sync.require("../../matrix/file")
 const registerUser = sync.require("./register-user")
 
 /** @returns {Promise<Ty.PkMessage>} */
-async function fetchMessage(messageID) {
+async function fetchMessage(messageID, attempt = 1) {
 	try {
 		var res = await fetch(`https://api.pluralkit.me/v2/messages/${messageID}`)
 	} catch (networkError) {
 		// Network issue, raise a more readable message
 		throw new Error(`Failed to connect to PK API: ${networkError.toString()}`)
 	}
-	if (!res.ok) throw new Error(`PK API returned an error: ${await res.text()}`)
+	if (!res.ok) {
+		if (attempt < 2) {
+			await scheduler.wait(5000)
+			return fetchMessage(messageID, attempt + 1)
+		} else {
+			throw new Error(`PK API returned an error: ${await res.text()}`)
+		}
+	}
 	/** @type {any} */
 	const root = await res.json()
 	if (!root.member) throw new Error(`PK API didn't return member data: ${JSON.stringify(root)}`)

src/d2m/actions/retrigger.js

@@ -98,7 +98,7 @@ function waitFor(id, resolve, existsInDatabase) {
 const GET_EVENT_PREPARED = from("event_message").select("event_id").and("WHERE event_id = ?").prepare().raw()
 /**
  * @param {string} eventID
- * @returns {Promise<boolean>} if true then the message did not arrive
+ * @returns {Promise<boolean>} if false then the message did not arrive
  */
 function waitForEvent(eventID) {
 	const {promise, resolve} = Promise.withResolvers()
@@ -109,7 +109,7 @@ function waitForEvent(eventID) {
 const GET_MESSAGE_PREPARED = from("event_message").select("message_id").and("WHERE message_id = ?").prepare().raw()
 /**
  * @param {string} messageID
- * @returns {Promise<boolean>} if true then the message did not arrive
+ * @returns {Promise<boolean>} if false then the message did not arrive
  */
 function waitForMessage(messageID) {
 	const {promise, resolve} = Promise.withResolvers()
@@ -120,7 +120,7 @@ function waitForMessage(messageID) {
 const GET_REACTION_EVENT_PREPARED = from("reaction").select("hashed_event_id").and("WHERE hashed_event_id = ?").prepare().raw()
 /**
  * @param {string} eventID
- * @returns {Promise<boolean>} if true then the message did not arrive
+ * @returns {Promise<boolean>} if false then the message did not arrive
  */
 function waitForReactionEvent(eventID) {
 	const {promise, resolve} = Promise.withResolvers()

src/d2m/actions/send-message.js

@@ -55,7 +55,7 @@ async function sendMessage(message, channel, guild) {
 		}
 	}
 
-	const events = await messageToEvent.messageToEvent(message, guild, {}, {api, snow: discord.snow})
+	const events = await messageToEvent.messageToEvent(message, guild, {}, {api, snow: discord.snow, fetch})
 	const eventIDs = []
 	if (events.length) {
 		db.prepare("INSERT OR IGNORE INTO message_room (message_id, historical_room_index) VALUES (?, ?)").run(message.id, historicalRoomIndex)

src/d2m/converters/edit-to-changes.js

@@ -69,8 +69,8 @@ async function editToChanges(message, guild, api) {
 
 	// Figure out what we will be replacing them with
 
-	const newFallbackContent = await messageToEvent.messageToEvent(message, guild, {includeEditFallbackStar: true}, {api})
+	const newFallbackContent = await messageToEvent.messageToEvent(message, guild, {includeEditFallbackStar: true}, {api, fetch})
-	const newInnerContent = await messageToEvent.messageToEvent(message, guild, {includeReplyFallback: false}, {api})
+	const newInnerContent = await messageToEvent.messageToEvent(message, guild, {includeReplyFallback: false}, {api, fetch})
 	assert.ok(newFallbackContent.length === newInnerContent.length)
 
 	// Match the new events to the old events

src/d2m/converters/message-to-event.js

@@ -109,10 +109,11 @@ const embedTitleParser = markdown.markdownEngine.parserFor({
 
 /**
  * @param {{room?: boolean, user_ids?: string[]}} mentions
- * @param {Omit<DiscordTypes.APIAttachment, "id" | "proxy_url">} attachment
+ * @param {Omit<DiscordTypes.APIAttachment, "id" | "proxy_url"> & {proxy_url?: string}} attachment
  * @param {boolean} [alwaysLink]
+ * @param {{fetch?: typeof fetch}} [di]
  */
-async function attachmentToEvent(mentions, attachment, alwaysLink) {
+async function attachmentToEvent(mentions, attachment, alwaysLink, di) {
 	const external_url = dUtils.getPublicUrlForCdn(attachment.url)
 	const emoji =
 		attachment.content_type?.startsWith("image/jp") ? "📸"
@@ -132,8 +133,37 @@ async function attachmentToEvent(mentions, attachment, alwaysLink) {
 			formatted_body: `<blockquote>${emoji} Uploaded SPOILER file: <a href="${external_url}">${external_url}</a> (${pb(attachment.size)})</blockquote>`
 		}
 	}
+	// attempt to convert large PNG image files to JPEG, since it's almost certainly a photo that was forced into PNG by the user copy-pasting.
+	if (
+		attachment.content_type === "image/png" && attachment.size > reg.ooye.max_file_size
+		&& !alwaysLink && attachment.proxy_url && attachment.width && attachment.height && di?.fetch
+	) {
+		const proxyUrl = new URL(attachment.proxy_url)
+		proxyUrl.searchParams.set("format", "jpeg")
+		const jpegUrl = proxyUrl.toString()
+		const res = await di.fetch(jpegUrl, {method: "HEAD"})
+		const newFilename = attachment.filename.replace(/\.png$/, ".jpg")
+		const newSize = Number(res.headers.get("content-length"))
+		if (res.ok && res.headers.get("content-type") === "image/jpeg" && !res.headers.has("content-encoding") && newSize <= reg.ooye.max_file_size) {
+			return {
+				$type: "m.room.message",
+				"m.mentions": mentions,
+				msgtype: "m.image",
+				url: await file.uploadDiscordFileToMxc(jpegUrl),
+				external_url,
+				body: attachment.description || newFilename,
+				filename: newFilename,
+				info: {
+					mimetype: "image/jpeg",
+					w: attachment.width,
+					h: attachment.height,
+					size: newSize
+				}
+			}
+		}
+	}
 	// for large files, always link them instead of uploading so I don't use up all the space in the content repo
-	else if (alwaysLink || attachment.size > reg.ooye.max_file_size) {
+	if (alwaysLink || attachment.size > reg.ooye.max_file_size) {
 		return {
 			$type: "m.room.message",
 			"m.mentions": mentions,
@@ -142,7 +172,8 @@ async function attachmentToEvent(mentions, attachment, alwaysLink) {
 			format: "org.matrix.custom.html",
 			formatted_body: `${emoji} Uploaded file: <a href="${external_url}">${attachment.filename}</a> (${pb(attachment.size)})`
 		}
-	} else if (attachment.content_type?.startsWith("image/") && attachment.width && attachment.height) {
+	}
+	if (attachment.content_type?.startsWith("image/") && attachment.width && attachment.height) {
 		return {
 			$type: "m.room.message",
 			"m.mentions": mentions,
@@ -158,7 +189,8 @@ async function attachmentToEvent(mentions, attachment, alwaysLink) {
 				size: attachment.size
 			}
 		}
-	} else if (attachment.content_type?.startsWith("video/") && attachment.width && attachment.height) {
+	}
+	if (attachment.content_type?.startsWith("video/") && attachment.width && attachment.height) {
 		return {
 			$type: "m.room.message",
 			"m.mentions": mentions,
@@ -174,7 +206,8 @@ async function attachmentToEvent(mentions, attachment, alwaysLink) {
 				size: attachment.size
 			}
 		}
-	} else if (attachment.content_type?.startsWith("audio/")) {
+	}
+	if (attachment.content_type?.startsWith("audio/")) {
 		return {
 			$type: "m.room.message",
 			"m.mentions": mentions,
@@ -189,19 +222,19 @@ async function attachmentToEvent(mentions, attachment, alwaysLink) {
 				duration: attachment.duration_secs && Math.round(attachment.duration_secs * 1000)
 			}
 		}
-	} else {
+	}
-		return {
+	// else
-			$type: "m.room.message",
+	return {
-			"m.mentions": mentions,
+		$type: "m.room.message",
-			msgtype: "m.file",
+		"m.mentions": mentions,
-			url: await file.uploadDiscordFileToMxc(attachment.url),
+		msgtype: "m.file",
-			external_url,
+		url: await file.uploadDiscordFileToMxc(attachment.url),
-			body: attachment.description || attachment.filename,
+		external_url,
-			filename: attachment.filename,
+		body: attachment.description || attachment.filename,
-			info: {
+		filename: attachment.filename,
-				mimetype: attachment.content_type,
+		info: {
-				size: attachment.size
+			mimetype: attachment.content_type,
-			}
+			size: attachment.size
 		}
 	}
 }
@@ -295,7 +328,7 @@ function mergeTextEvents(newEvents, events, forceSameMsgtype, forceMerge = false
  * - includeEditFallbackStar: false
  * - alwaysReturnFormattedBody: false - formatted_body will be skipped if it is the same as body because the message is plaintext. if you want the formatted_body to be returned anyway, for example to merge it with another message, then set this to true.
  * - scanTextForMentions: true - needs to be set to false when converting forwarded messages etc which may be from a different channel that can't be scanned.
- * @param {{api: import("../../matrix/api"), snow?: import("snowtransfer").SnowTransfer}} di simple-as-nails dependency injection for the matrix API
+ * @param {{api: import("../../matrix/api"), snow?: import("snowtransfer").SnowTransfer, fetch?: typeof fetch}} di simple-as-nails dependency injection for the matrix API
  * @returns {Promise<{$type: string, $sender?: string, [x: string]: any}[]>}
  */
 async function messageToEvent(message, guild, options = {}, di) {
@@ -882,7 +915,7 @@ async function messageToEvent(message, guild, options = {}, di) {
 
 	// Then attachments
 	if (message.attachments) {
-		const attachmentEvents = await Promise.all(message.attachments.map(attachment => attachmentToEvent(mentions, attachment)))
+		const attachmentEvents = await Promise.all(message.attachments.map(attachment => attachmentToEvent(mentions, attachment, false, {fetch: di?.fetch})))
 
 		// Try to merge attachment events with the previous event
 		// This means that if the attachments ended up as a text link, and especially if there were many of them, the events will be joined together.
@@ -913,7 +946,7 @@ async function messageToEvent(message, guild, options = {}, di) {
 					url: file.url,
 					height: file.height,
 					width: file.width,
-				}, true)
+				}, true, {fetch: di?.fetch})
 				stack.msb.addLine(ev.body, ev.formatted_body)
 			}
 			else if (component.type === DiscordTypes.ComponentType.MediaGallery) {

src/d2m/converters/message-to-event.test.js

@@ -1103,6 +1103,45 @@ test("message2event: very large attachment is linked instead of being uploaded",
 	}])
 })
 
+test("message2event: very large png is converted to jpeg for upload", async t => {
+	const events = await messageToEvent({
+		content: "",
+		attachments: [{
+			filename: "855064e9-bdae-487a-a497-c662bd510487.png",
+			url: "https://cdn.discordapp.com/attachments/1160894080998461480/1548656385292509274/855064e9-bdae-487a-a497-c662bd510487.png",
+			proxy_url: "https://media.discordapp.net/attachments/1160894080998461480/1548656385292509274/855064e9-bdae-487a-a497-c662bd510487.png",
+			content_type: "image/png",
+			width: 5472,
+			height: 3648,
+			size: 14967919
+		}]
+	}, data.guild.general, {}, {
+		async fetch(url, init) {
+			t.equal(url, "https://media.discordapp.net/attachments/1160894080998461480/1548656385292509274/855064e9-bdae-487a-a497-c662bd510487.png?format=jpeg")
+			t.equal(init.method, "HEAD")
+			return new Response("wa", {headers: {
+				"Content-Type": "image/jpeg",
+				"Content-Length": 483085
+			}})
+		}
+	})
+	t.deepEqual(events, [{
+		$type: "m.room.message",
+		"m.mentions": {},
+		msgtype: "m.image",
+		body: "855064e9-bdae-487a-a497-c662bd510487.jpg",
+		info: {
+			w: 5472,
+			h: 3648,
+			mimetype: "image/jpeg",
+			size: 483085
+		},
+		external_url: "https://bridge.example.org/download/discordcdn/1160894080998461480/1548656385292509274/855064e9-bdae-487a-a497-c662bd510487.png",
+		filename: "855064e9-bdae-487a-a497-c662bd510487.jpg",
+		url: "mxc://cadence.moe/zXNXRvJwRDMIzZZVqHUXtGeD"
+	}])
+})
+
 test("message2event: multiple attachments are combined into the same event where possible", async t => {
 	const events = await messageToEvent({
 		content: "hey",

src/d2m/discord-client.js

@@ -55,9 +55,7 @@ class DiscordClient {
 		this.guildChannelMap = new Map()
 		if (listen !== "no") {
 			this.cloud.on("event", message => {
-				process.nextTick(() => {
+				discordPackets.onPacket(this, message, listen)
-					discordPackets.onPacket(this, message, listen)
-				})
 			})
 		}
 

src/d2m/discord-packets.js

@@ -1,6 +1,6 @@
 // @ts-check
 
-const assert = require("assert")
+const assert = require("assert").strict
 const {scheduler} = require("timers/promises")
 const passthrough = require("../passthrough")
 const {sync} = passthrough
@@ -8,7 +8,32 @@ const {sync} = passthrough
 /** @type {import("../matrix/homeserver-status")} */
 const homeserverStatus = sync.require("../matrix/homeserver-status")
 
-let checkedHomeserver = false
+const guildReadyStatus = new class {
+	/** @type {Set<string> | null} */
+	unavailableGuilds = null
+	_allReady = Promise.withResolvers()
+
+	/**
+	 * @param {string} guildID
+	 * @returns {boolean} true if it was the last one
+	 */
+	makeReady(guildID) {
+		assert(this.unavailableGuilds)
+		if (this.unavailableGuilds.delete(guildID) && this.unavailableGuilds.size === 0) {
+			this._allReady.resolve(null)
+			return true
+		}
+		return false
+	}
+
+	allReady() {
+		return this.unavailableGuilds && this.unavailableGuilds.size === 0
+	}
+
+	waitForAllReady() {
+		return this._allReady.promise
+	}
+}
 
 /**
  * @param {import("./discord-client")} client
@@ -28,12 +53,27 @@ async function onPacket(client, message, listen) {
 		client.ready = true
 		client.user = message.d.user
 		client.application = message.d.application
+		guildReadyStatus.unavailableGuilds = new Set(message.d.guilds.filter(g => g.unavailable).map(g => g.id))
 		console.log(`Discord logged in as ${client.user.username}#${client.user.discriminator} (${client.user.id})`)
+		process.stdout.write("Waiting for guilds to warm up... ")
 		interactions.registerInteractions()
 
 	} else if (message.t === "GUILD_CREATE") {
-		message.d.members = message.d.members.filter(m => m.user.id === client.user.id) // only keep the bot's own member - it's needed to determine private channels on web
+		message.d.members = message.d.members.filter(m => m.user.id === client.user.id) // only keep the bot account's member - it's needed for roles to determine private channels on web
 		client.guilds.set(message.d.id, message.d)
+
+		/*
+			Info about guilds is populated one guild at a time.
+			For m->d bridging to work, the guild needs to be populated, so we need to have GUILD_CREATE for the guild.
+			If we ping the homeserver, it will send us any pending events, so we need to wait for all GUILD_CREATES before we ping.
+			We must attempt a ping because we don't want to try sending missed d->m messages to an offline homeserver.
+			The "all guilds ready" delay can be removed if ONE of the following is done:
+				1. m->d can queue incoming events until their guild exists in memory
+				2. d->m missed messages can have their errors handled and added to queue, rather than pinging first
+		*/
+		const firstReady = guildReadyStatus.allReady()
+		const lastGuildReady = guildReadyStatus.makeReady(message.d.id)
+
 		const arr = []
 		client.guildChannelMap.set(message.d.id, arr)
 		for (const channel of message.d.channels || []) {
@@ -51,28 +91,14 @@ async function onPacket(client, message, listen) {
 
 		if (listen === "full") {
 			try {
-				/*
+				// Wait for guilds to be connected and homeserver to be online. If this is the last guild, a different code path is used to trigger the homeserver check.
-					Info about guilds is populated one guild at a time.
+				if (lastGuildReady) {
-					For m->d bridging to work, the guild needs to be populated, so we need to have GUILD_CREATE for the guild.
+					process.stdout.write(`ok, ${client.guilds.size} available.\nConnecting to homeserver... `)
-					If we ping the homeserver, it will send us any pending events, so we need to wait for all GUILD_CREATES before we ping.
+					// await guildReadyStatus.waitForAllReady() - no need, we already checked this is the last guild
-					We must attempt a ping because we don't want to try sending missed d->m messages to an offline homeserver.
-					This delay can be removed if ONE of the following is done:
-						1. m->d can queue incoming events until their guild exists in memory
-						2. d->m missed messages can have their errors handled and added to queue, rather than pinging first
-				*/
-				let isMainCharacter = false
-				if (!checkedHomeserver) {
-					checkedHomeserver = true
-					isMainCharacter = true
-					console.log("Warming up guilds~")
-				}
-				await scheduler.wait(5000)
-				if (isMainCharacter) {
-					checkedHomeserver = true
-					process.stdout.write("Connecting to homeserver... ")
 					await homeserverStatus.homeserverStatus.waitForOnline(true)
 					console.log("ok.\nReplaying past events. Welcome to Out Of Your Element.")
 				} else {
+					await guildReadyStatus.waitForAllReady()
 					await homeserverStatus.homeserverStatus.waitForOnline(false)
 				}
 
@@ -81,7 +107,9 @@ async function onPacket(client, message, listen) {
 				await eventDispatcher.checkMissedPins(client, message.d)
 				await eventDispatcher.checkMissedLeaves(client, message.d)
 			} catch (e) {
-				console.error("Failed to sync missed events. To retry, please fix this error and restart OOYE:")
+				if (firstReady) {
+					console.error("Failed to sync missed events. To retry, please fix this error and restart OOYE:")
+				}
 				console.error(e)
 			}
 		}
@@ -225,3 +253,4 @@ async function dispatchPacketToBridge(client, message) {
 
 module.exports.onPacket = onPacket
 module.exports.dispatchPacketToBridge = dispatchPacketToBridge
+module.exports.guildReadyStatus = guildReadyStatus

src/m2d/actions/send-event.js

@@ -153,7 +153,7 @@ async function sendEvent(event) {
 					channel_id: messageResponse.channel_id,
 					guild_id: guild.id,
 					embeds: messageResponse.embeds
-				}, guild, null)
+				}, guild)
 			)
 		}
 	}

src/m2d/converters/event-to-message.js

@@ -32,6 +32,8 @@ const setupEmojis = sync.require("../actions/setup-emojis")
 const userToMxid = sync.require("../../d2m/converters/user-to-mxid")
 /** @type {import("../../web/routes/letter-avatar")} */
 const letterAvatar = sync.require("../../web/routes/letter-avatar")
+/** @type {import("../../d2m/actions/retrigger")} */
+const retrigger = sync.require("../../d2m/actions/retrigger")
 
 /** @type {[RegExp, string][]} */
 const markdownEscapes = [
@@ -696,6 +698,7 @@ async function eventToMessage(event, guild, channel, di) {
 			// Check if we have a pointer to what was edited
 			const originalEventId = relatesTo.event_id
 			if (!originalEventId) return
+			if (!await retrigger.waitForEvent(originalEventId)) return
 			messageIDsToEdit = select("event_message", "message_id", {event_id: originalEventId}, "ORDER BY part").pluck().all()
 			if (!messageIDsToEdit.length) return
 

src/matrix/file.js

@@ -15,10 +15,15 @@ const IMAGE_SIZE = 1024
 const inflight = new Map()
 
 /**
- * @param {string} url
+ * @param {string} urlString
  */
-function _removeExpiryParams(url) {
+function _removeExpiryParams(urlString) {
-	return url.replace(/\?(?:(?:ex|is|sg|hm)=[a-f0-9]+&?)*$/, "")
+	const url = new URL(urlString)
+	url.searchParams.delete("ex")
+	url.searchParams.delete("is")
+	url.searchParams.delete("sg")
+	url.searchParams.delete("hm")
+	return url.toString()
 }
 
 /**

src/matrix/file.test.js

@@ -20,3 +20,9 @@ test("removeExpiryParams: rearranged params are removed", t => {
 	const result = file._removeExpiryParams(url)
 	t.equal(result, "https://cdn.discordapp.com/attachments/112760669178241024/1157363960518029322/image.png")
 })
+
+test("removeExpiryParams: works on media proxy and keeps quality", t => {
+	const url = "https://media.discordapp.net/attachments/1160894080998461480/1548665030898094240/855064e9-bdae-487a-a497-c662bd510487.png?ex=6aa7e234&is=6aa690b4&hm=7c153efcdd1feb1d36d0bb99955a2bfe12f26abea377fed02980521766f51620&format=jpeg"
+	const result = file._removeExpiryParams(url)
+	t.equal(result, "https://media.discordapp.net/attachments/1160894080998461480/1548665030898094240/855064e9-bdae-487a-a497-c662bd510487.png?format=jpeg")
+})

test/ooye-test-data.sql

@@ -168,7 +168,8 @@ INSERT INTO file (discord_url, mxc_url) VALUES
 ('https://cdn.discordapp.com/attachments/122155380120748034/1174514575220158545/the.yml', 'mxc://cadence.moe/HnQIYQmmlIKwOQsbFsIGpzPP'),
 ('https://cdn.discordapp.com/attachments/112760669178241024/1296237494987133070/100km.gif', 'mxc://cadence.moe/qDAotmebTfEIfsAIVCEZptLh'),
 ('https://cdn.discordapp.com/attachments/123/456/my_enemies.txt', 'mxc://cadence.moe/y89EOTRp2lbeOkgdsEleGOge'),
-('https://cdn.discordapp.com/emojis/1254940125948022915.webp', 'mxc://cadence.moe/bvVJFgOIyNcAknKCbmaHDktG');
+('https://cdn.discordapp.com/emojis/1254940125948022915.webp', 'mxc://cadence.moe/bvVJFgOIyNcAknKCbmaHDktG'),
+('https://media.discordapp.net/attachments/1160894080998461480/1548656385292509274/855064e9-bdae-487a-a497-c662bd510487.png?format=jpeg', 'mxc://cadence.moe/zXNXRvJwRDMIzZZVqHUXtGeD');
 
 INSERT INTO emoji (emoji_id, name, animated, mxc_url) VALUES
 ('230201364309868544', 'hippo', 0, 'mxc://cadence.moe/qWmbXeRspZRLPcjseyLmeyXC'),