server: better performace for mass delivery

This should hopefully relieve some of the massive hammering on the
database when mass delivery jobs are running.

However, this also means that instance blocks are applied with a
slight delay on delivery queue. Since the settings page in the
native frontend already warns about this, I think it should be fine.
And the maximum time an instance block would be delayed would be
somewhere around 5min which IMO is also tolerable.

Changelog: Changed

packages/backend/src/queue/initialize.ts

@@ -23,6 +23,19 @@ export function initialize<T>(name: string, limitPerSec = -1): Bull.Queue<T> {
 function apBackoff(attemptsMade: number /*, err: Error */): number {
 	const baseDelay = MINUTE;
 	const maxBackoff = 8 * HOUR;
+	/*
+	attempt | average seconds + up to 2% random offset
+	0  |     0
+	1  |    60 = 1min
+	2  |   180 = 3min
+	3  |   420 = 7min
+	4  |   900 = 15min
+	5  |  1860 = 31min
+	6  |  3780 = 63min
+	7  |  7620 = 127min ~= 2.1h
+	8  | 15300 = 4.25h
+	>8 | 28800 = 8h
+	*/
 	let backoff = (Math.pow(2, attemptsMade) - 1) * baseDelay;
 	backoff = Math.min(backoff, maxBackoff);
 	backoff += Math.round(backoff * Math.random() * 0.2);

packages/backend/src/queue/processors/deliver.ts

@@ -17,7 +17,18 @@ export default async (job: Bull.Job<DeliverJobData>) => {
 	const { host } = new URL(job.data.to);
 	const puny = toPuny(host);
 
-	if (await shouldSkipInstance(puny)) return 'skip';
+	// for the first few tries (where most attempts will be made)
+	// we assume that inserting deliver jobs took care of this check
+	// only on later attempts do we actually do it, to ease database
+	// performance. this might cause a slight delay of a few minutes
+	// for instance blocks being applied
+	//
+	// with apBackoff, attempt 2 happens ~4min after the initial try, while
+	// attempt 3 happens ~11 min after the initial try, which seems like a
+	// good tradeoff between database and blocks being applied reasonably quick
+	if (job.attemptsMade >= 3 && await shouldSkipInstance(puny)) {
+		return 'skip';
+	}
 
 	const keypair = await getUserKeypair(job.data.user.id);
 

packages/backend/src/remote/activitypub/request.ts

@@ -4,6 +4,7 @@ import { getUserKeypair } from '@/misc/keypair-store.js';
 import { User } from '@/models/entities/user.js';
 import { UserKeypair } from '@/models/entities/user-keypair.js';
 import { getResponse } from '@/misc/fetch.js';
+import { getApId } from './type.js';
 import { createSignedPost, createSignedGet } from './ap-request.js';
 import { apRequestChart, federationChart, instanceChart } from '@/services/chart/index.js';
 
@@ -54,6 +55,39 @@ export async function request(user: { id: User['id'] }, url: string, object: any
 	}
 }
 
+// Determine that the content type actually is an activitypub object.
+//
+// This defends against the possibility of a user of a remote instance uploading
+// something that looks like an ActivityPub object and thus masquerading as any
+// other user on that same instance. It in turn depends on the server not
+// returning that content as an ActivityPub MIME type.
+//
+// Ref: GHSA-jhrq-qvrm-qr36
+function isActivitypub(_contentType: string): boolean {
+	const contentType = _contentType.toLowerCase()
+	if (contentType.startsWith('application/activity+json')) {
+		return true;
+	}
+	if (contentType.startsWith('application/ld+json')) {
+		// oh lord, actually parsing the MIME type
+		// Ref: <urn:ietf:rfc:2045> § 5.1
+		// Ref: <https://www.iana.org/assignments/media-types/application/ld+json>
+		let start = contentType.indexOf('profile="');
+		if (start === -1) return false; // profile is required for our purposes
+
+		start += 'profile="'.length;
+		let end = contentType.indexOf('"', start);
+		if (end === -1) return false; // malformed MIME type
+
+		let profiles = contentType.substring(start, end).split(/\s+/);
+		if (profiles.includes('https://www.w3.org/ns/activitystreams')) {
+			return true;
+		}
+	}
+
+	return false;
+};
+
 /**
  * Get AP object with http-signature
  * @param user http-signature user
@@ -63,7 +97,7 @@ export async function signedGet(_url: string, user: { id: User['id'] }): Promise
 	let url = _url;
 	const keypair = await getUserKeypair(user.id);
 
-	for (let redirects = 0; redirects < 3; redirects++) {
+	for (let redirects = 3; redirects > 0; redirects--) {
 		const req = createSignedGet({
 			key: {
 				privateKeyPem: keypair.privateKey,
@@ -85,45 +119,35 @@ export async function signedGet(_url: string, user: { id: User['id'] }): Promise
 		if (res.status >= 300 && res.status < 400) {
 			// Have been redirected, need to make a new signature.
 			// Use Location header and fetched URL as the base URL.
-			url = new URL(res.headers.get('Location'), url).href;
+			let newUrl = new URL(res.headers.get('Location'), url);
+			// Check that we have not been redirected to a different host.
+			if (newUrl.host !== new URL(url).host) {
+				throw new Error('cross-origin redirect not allowed');
+			}
+			url = newUrl.href;
 		} else {
-			// Determine that the content type we got back actually is an activitypub object.
-			//
-			// This defends against the possibility of a user of a remote instance uploading
-			// something that looks like an ActivityPub object and thus masquerading as any
-			// other user on that same instance. It in turn depends on the server not
-			// returning that content as an ActivityPub MIME type.
-			//
-			// Ref: GHSA-jhrq-qvrm-qr36
-			const isActivitypub = (_contentType) => {
-				const contentType = _contentType.toLowerCase()
-				if (contentType.startsWith('application/activity+json')) {
-					return true;
-				}
-				if (contentType.startsWith('application/ld+json')) {
-					// oh lord, actually parsing the MIME type
-					// Ref: <urn:ietf:rfc:2045> § 5.1
-					// Ref: <https://www.iana.org/assignments/media-types/application/ld+json>
-					let start = contentType.indexOf('profile="');
-					if (start === -1) return false; // profile is required for our purposes
-
-					start += 'profile="'.length;
-					let end = contentType.indexOf('"', start);
-					if (end === -1) return false; // malformed MIME type
-
-					let profiles = contentType.substring(start, end).split(/\s+/);
-					if (profiles.includes('https://www.w3.org/ns/activitystreams')) {
-						return true;
-					}
-				}
-
-				return false;
-			};
-
 			if (!isActivitypub(res.headers.get('Content-Type'))) {
 				throw new Error('invalid response content type');
 			}
-			return await res.json();
+
+			const data = await res.json();
+			// In theory, activitypub allows for `id` to be null for ephemeral
+			// objects, but we wouldn't be fetching those with signed get, since
+			// they are... ephemeral.
+			const id = new URL(getApId(data));
+			if (id.href !== url.href) {
+				// if the id and fetched url mismatch, treat it as if it was a redirect
+				// SECURITY: this is to prevent impersonation via improper media files
+				url = id;
+				// if this kind of "redirect" happens, there should be at most one more
+				// redirect since we now have the canonical url. setting to 2 because it
+				// will be decremented to 1 right away by the for loop.
+				if (redirects > 2) {
+					redirects = 2;
+				}
+			} else {
+				return data;
+			}
 		}
 	}