Fixed infinite recursion in avatar & user resolvers. (#9178)

PR 9073 introduced an infinite recursion bug. If a user had a custom avatar that they had uploaded, the user resolver would fetch their avatar, which would now fetch the user that owned that avatar, which would fetch the avatar, ad infinitum. Added some query params to skip the population of the user or avatar if instructed to do so by internal resolver fetches.
EtherealEngine · Oct 31, 2023 · c780204 · c780204
1 parent ec71481
commit c780204
Show file tree

Hide file tree

Showing 7 changed files with 31 additions and 10 deletions.
diff --git a/packages/engine/src/schemas/user/avatar.schema.ts b/packages/engine/src/schemas/user/avatar.schema.ts
@@ -65,7 +65,7 @@ export const avatarSchema = Type.Object(
 )
 export type AvatarType = Static<typeof avatarSchema>
 
-export type AvatarDatabaseType = Omit<AvatarType, 'user' | 'modelResource' | 'thumbnailResource'>
+export type AvatarDatabaseType = Omit<AvatarType, 'modelResource' | 'thumbnailResource'>
 
 // Schema for creating new entries
 // export const avatarDataSchema = Type.Pick(
@@ -105,7 +105,10 @@ export const avatarQuerySchema = Type.Intersect(
       }
     }),
     // Add additional query properties here
-    Type.Object({ action: Type.Optional(Type.String()) }, { additionalProperties: false })
+    Type.Object(
+      { action: Type.Optional(Type.String()), skipUser: Type.Optional(Type.Boolean()) },
+      { additionalProperties: false }
+    )
   ],
   { additionalProperties: false }
 )

diff --git a/packages/engine/src/schemas/user/user.schema.ts b/packages/engine/src/schemas/user/user.schema.ts
@@ -113,7 +113,8 @@ export const userQuerySchema = Type.Intersect(
     // Add additional query properties here
     Type.Object(
       {
-        search: Type.Optional(Type.String())
+        search: Type.Optional(Type.String()),
+        skipAvatar: Type.Optional(Type.Boolean())
       },
       { additionalProperties: false }
     )

diff --git a/packages/server-core/src/scope/scope/scope.hooks.ts b/packages/server-core/src/scope/scope/scope.hooks.ts
@@ -39,14 +39,14 @@ import { HookContext } from '../../../declarations'
 import enableClientPagination from '../../hooks/enable-client-pagination'
 import verifyScope from '../../hooks/verify-scope'
 import verifyScopeAllowingSelf from '../../hooks/verify-scope-allowing-self'
+import { ScopeService } from './scope.class'
 import {
   scopeDataResolver,
   scopeExternalResolver,
   scopePatchResolver,
   scopeQueryResolver,
   scopeResolver
-} from '../../scope/scope/scope.resolvers'
-import { ScopeService } from './scope.class'
+} from './scope.resolvers'
 
 /**
  * Check and maintain existing scopes

diff --git a/packages/server-core/src/user/avatar/avatar.hooks.ts b/packages/server-core/src/user/avatar/avatar.hooks.ts
@@ -41,6 +41,7 @@ import { userPath } from '@etherealengine/engine/src/schemas/user/user.schema'
 import { HookContext } from '../../../declarations'
 import disallowNonId from '../../hooks/disallow-non-id'
 import isAction from '../../hooks/is-action'
+import persistQuery from '../../hooks/persist-query'
 import verifyScope from '../../hooks/verify-scope'
 import { AvatarService } from './avatar.class'
 import {
@@ -179,10 +180,12 @@ export default {
     all: [() => schemaHooks.validateQuery(avatarQueryValidator), schemaHooks.resolveQuery(avatarQueryResolver)],
     find: [
       iffElse(isAction('admin'), verifyScope('admin', 'admin'), ensureUserAccessibleAvatars),
+      persistQuery,
       discardQuery('action'),
+      discardQuery('skipUser'),
       sortByUserName
     ],
-    get: [],
+    get: [persistQuery, discardQuery('skipUser')],
     create: [
       () => schemaHooks.validateData(avatarDataValidator),
       schemaHooks.resolveData(avatarDataResolver),

diff --git a/packages/server-core/src/user/avatar/avatar.resolvers.ts b/packages/server-core/src/user/avatar/avatar.resolvers.ts
@@ -41,8 +41,13 @@ export const avatarResolver = resolve<AvatarType, HookContext>({
 
 export const avatarExternalResolver = resolve<AvatarType, HookContext>({
   user: virtual(async (avatar, context) => {
+    if (context.arguments && context.arguments.length > 0 && context.arguments[1]?.actualQuery?.skipUser) return {}
     if (avatar.userId) {
-      return context.app.service(userPath).get(avatar.userId)
+      try {
+        return await context.app.service(userPath).get(avatar.userId, { query: { skipAvatar: true } })
+      } catch (err) {
+        return {}
+      }
     }
   }),
   modelResource: virtual(async (avatar, context) => {

diff --git a/packages/server-core/src/user/user/user.hooks.ts b/packages/server-core/src/user/user/user.hooks.ts
@@ -46,6 +46,7 @@ import { HookContext } from '../../../declarations'
 import { createSkippableHooks } from '../../hooks/createSkippableHooks'
 import disallowNonId from '../../hooks/disallow-non-id'
 import persistData from '../../hooks/persist-data'
+import persistQuery from '../../hooks/persist-query'
 import verifyScope from '../../hooks/verify-scope'
 import getFreeInviteCode from '../../util/get-free-invite-code'
 import { UserService } from './user.class'
@@ -267,9 +268,11 @@ export default createSkippableHooks(
       all: [() => schemaHooks.validateQuery(userQueryValidator), schemaHooks.resolveQuery(userQueryResolver)],
       find: [
         iff(isProvider('external'), verifyScope('admin', 'admin'), verifyScope('user', 'read'), handleUserSearch),
-        iff(isProvider('external'), discardQuery('search', '$sort.accountIdentifier') as any)
+        iff(isProvider('external'), discardQuery('search', '$sort.accountIdentifier') as any),
+        persistQuery,
+        discardQuery('skipAvatar')
       ],
-      get: [],
+      get: [persistQuery, discardQuery('skipAvatar')],
       create: [
         iff(isProvider('external'), verifyScope('admin', 'admin'), verifyScope('user', 'write')),
         () => schemaHooks.validateData(userDataValidator),

diff --git a/packages/server-core/src/user/user/user.resolvers.ts b/packages/server-core/src/user/user/user.resolvers.ts
@@ -95,7 +95,13 @@ export const userResolver = resolve<UserType, HookContext>({
 
 export const userExternalResolver = resolve<UserType, HookContext>({
   avatar: virtual(async (user, context) => {
-    if (context.event !== 'removed' && user.avatarId) return await context.app.service(avatarPath).get(user.avatarId)
+    if (context.arguments && context.arguments.length > 0 && context.arguments[1]?.actualQuery?.skipAvatar) return {}
+    if (context.event !== 'removed' && user.avatarId)
+      try {
+        return await context.app.service(avatarPath).get(user.avatarId, { query: { skipUser: true } })
+      } catch (err) {
+        return {}
+      }
   }),
   userSetting: virtual(async (user, context) => {
     const userSetting = (await context.app.service(userSettingPath).find({