fix: don't stream back bytes for icons (#2486)

* decrease global check alerts

* fix: don't stream back bytes for icons

src/backend/src/filesystem/hl_operations/hl_readdir.js

@@ -18,7 +18,6 @@
  */
 const APIError = require('../../api/APIError');
 const { Context } = require('../../util/context');
-const { stream_to_buffer } = require('../../util/streamutil');
 const { get_apps, suggestedAppsForFsEntries } = require('../../helpers');
 const { ECMAP } = require('../ECMAP');
 const { TYPE_DIRECTORY, TYPE_SYMLINK } = require('../FSNodeContext');
@@ -131,25 +130,12 @@ class HLReadDir extends HLFilesystemOperation {
             const entry = await child.getSafeEntry();
             if ( !no_thumbs && entry.associated_app ) {
                 const svc_appIcon = this.context.get('services').get('app-icon');
-                const iconResult = await svc_appIcon.getIconStream({
-                    appIcon: entry.associated_app.icon,
+                const iconPath = svc_appIcon.getAppIconPath({
                     appUid: entry.associated_app.uid ?? entry.associated_app.uuid,
                     size: 64,
                 });
-
-                if ( iconResult.dataUrl ?? iconResult.data_url ) {
-                    entry.associated_app.icon = iconResult.dataUrl ?? iconResult.data_url;
-                } else {
-                    try {
-                        const buffer = await stream_to_buffer(iconResult.stream);
-                        const respDataUrl = `data:${iconResult.mime};base64,${buffer.toString('base64')}`;
-                        entry.associated_app.icon = respDataUrl;
-                    } catch (e) {
-                        const svc_error = this.context.get('services').get('error-service');
-                        svc_error.report('hl_readdir:icon-stream', {
-                            source: e,
-                        });
-                    }
+                if ( iconPath ) {
+                    entry.associated_app.icon = iconPath;
                 }
             }
             return entry;

src/backend/src/helpers.js

@@ -1919,13 +1919,12 @@ async function get_taskbar_items (user, { icon_size, no_icons } = {}) {
             delete item.icon;
         } else {
             const svc_appIcon = _servicesHolder.services.get('app-icon');
-            const iconResult = await svc_appIcon.getIconStream({
-                appIcon: item.icon,
+            const iconUrl = svc_appIcon.getSizedIconUrl({
                 appUid: item.uid,
                 size: icon_size,
             });
 
-            item.icon = await iconResult.get_data_url();
+            item.icon = iconUrl;;
         }
 
         // add to final object

src/backend/src/modules/apps/AppIconService.js

@@ -30,7 +30,6 @@ import { DB_WRITE } from '../../services/database/consts.js';
 import { Endpoint } from '../../util/expressutil.js';
 import { buffer_to_stream, stream_to_buffer } from '../../util/streamutil.js';
 import DEFAULT_APP_ICON from './default-app-icon.js';
-import IconResult from './lib/IconResult.js';
 
 const require = createRequire(import.meta.url);
 
@@ -88,7 +87,7 @@ export class AppIconService extends BaseService {
                     mime,
                     redirectUrl,
                     redirectCacheControl,
-                } = await this.getIconStream({ appUid, size, allowRedirect: true });
+                } = await this.#getIconStream({ appUid, size, allowRedirect: true });
 
                 if ( redirectUrl ) {
                     if ( redirectCacheControl ) {
@@ -109,35 +108,30 @@ export class AppIconService extends BaseService {
     }
 
     async iconifyApps ({ apps, size }) {
-        return await Promise.all(apps.map(async app => {
-            const iconResult = await this.getIconStream({
-                appIcon: app.icon,
+        return apps.map(app => {
+            const iconPath = this.getAppIconPath({
                 appUid: app.uid ?? app.uuid,
                 size,
             });
-
-            if ( iconResult.dataUrl ?? iconResult.data_url ) {
-                app.icon = iconResult.dataUrl ?? iconResult.data_url;
-                return app;
-            }
-
-            try {
-                const buffer = await stream_to_buffer(iconResult.stream);
-                const respDataUrl = `data:${iconResult.mime};base64,${buffer.toString('base64')}`;
-
-                app.icon = respDataUrl;
-            } catch (e) {
-                this.errors.report('get-launch-apps:icon-stream', {
-                    source: e,
-                });
+            if ( iconPath ) {
+                app.icon = iconPath;
             }
             return app;
-        }));
+        });
     }
 
-    async getIconStream (params) {
-        const result = await this.#getIconStream(params);
-        return new IconResult(result);
+    getAppIconPath ({ appUid, size }) {
+        const normalizedAppUid = this.normalizeAppUid(appUid);
+        if ( typeof normalizedAppUid !== 'string' || !normalizedAppUid ) {
+            return null;
+        }
+
+        const apiBaseUrl = String(config.api_base_url || '').replace(/\/+$/, '');
+        if ( ! apiBaseUrl ) {
+            return null;
+        }
+
+        return `${apiBaseUrl}/app-icon/${normalizedAppUid}/${size}`;
     }
 
     normalizeAppUid (appUid) {

src/backend/src/modules/apps/AppIconService.test.js

@@ -82,98 +82,44 @@ describe('AppIconService', () => {
         });
     });
 
-    describe('getIconStream', () => {
-        const createServiceInstance = () => new AppIconService({
-            services: { get: vi.fn() },
-            config: {},
-            name: 'app-icon',
-            args: {},
-        });
+    describe('icon URL mapping', () => {
+        it('builds a legacy app-icon path with normalized app uid', () => {
+            const service = Object.create(AppIconService.prototype);
 
-        it('redirects to puter-app-icons subsite sized file when requested size exists', async () => {
-            const legacyNode = {
-                exists: vi.fn().mockResolvedValue(true),
-            };
-            const legacyRoot = {
-                getChild: vi.fn().mockResolvedValue(legacyNode),
-            };
-
-            const service = createServiceInstance();
-            service.errors = { report: vi.fn() };
-            service.getAppIcons = vi.fn().mockResolvedValue(legacyRoot);
-            service.getSizedIconUrl = vi.fn().mockReturnValue('https://puter-app-icons.site.puter.localhost/app-abc-64.png');
-
-            const result = await service.getIconStream({
-                appUid: 'app-abc',
+            const result = service.getAppIconPath({
+                appUid: 'abc',
                 size: 64,
-                allowRedirect: true,
             });
 
-            expect(result.redirectUrl).toBe('https://puter-app-icons.site.puter.localhost/app-abc-64.png');
-            expect(result.redirectCacheControl).toContain('max-age=2592000');
+            expect(result).toBe(`${config.api_base_url}/app-icon/app-abc/64`);
         });
 
-        it('redirects to original and queues resize when requested size is missing', async () => {
-            const legacyNode = {
-                exists: vi.fn().mockResolvedValue(false),
-            };
-            const legacyRoot = {
-                getChild: vi.fn().mockResolvedValue(legacyNode),
-            };
-            const originalNode = {
-                exists: vi.fn().mockResolvedValue(true),
-            };
+        it('iconifyApps rewrites icons to the legacy app-icon endpoint path', async () => {
+            const service = Object.create(AppIconService.prototype);
+            const apps = [
+                { uid: 'app-abc', icon: 'data:image/png;base64,AA==' },
+                { uuid: 'def', icon: 'https://example.com/icon.png' },
+            ];
 
-            const service = createServiceInstance();
-            service.errors = { report: vi.fn() };
-            service.getAppIcons = vi.fn().mockResolvedValue(legacyRoot);
-            service.getOriginalIconLookup = vi.fn().mockResolvedValue({
-                node: originalNode,
-                isFlatOriginal: true,
-            });
-            service.getOriginalIconUrl = vi.fn().mockReturnValue('https://puter-app-icons.site.puter.localhost/app-abc.png');
-            service.queueMissingSizeFromOriginal = vi.fn();
-
-            const result = await service.getIconStream({
-                appUid: 'app-abc',
-                size: 128,
-                allowRedirect: true,
-            });
-
-            expect(result.redirectUrl).toBe('https://puter-app-icons.site.puter.localhost/app-abc.png');
-            expect(result.redirectCacheControl).toContain('max-age=604800');
-            expect(service.queueMissingSizeFromOriginal).toHaveBeenCalledWith({
-                appUid: 'app-abc',
+            const result = await service.iconifyApps({
+                apps,
                 size: 128,
             });
+
+            expect(result[0].icon).toBe(`${config.api_base_url}/app-icon/app-abc/128`);
+            expect(result[1].icon).toBe(`${config.api_base_url}/app-icon/app-def/128`);
         });
 
-        it('redirects to app icon URL when no cached icon exists and URL is eligible', async () => {
-            const redirectUrl = `https://dev-center-app-id.${config.static_hosting_domain}/raw-icon.png`;
+        it('iconifyApps leaves icon unchanged when app uid is missing', async () => {
+            const service = Object.create(AppIconService.prototype);
+            const apps = [{ icon: 'existing-icon' }];
 
-            const legacyNode = {
-                exists: vi.fn().mockResolvedValue(false),
-            };
-            const legacyRoot = {
-                getChild: vi.fn().mockResolvedValue(legacyNode),
-            };
-
-            const service = createServiceInstance();
-            service.errors = { report: vi.fn() };
-            service.getAppIcons = vi.fn().mockResolvedValue(legacyRoot);
-            service.getOriginalIconLookup = vi.fn().mockResolvedValue({
-                node: null,
-                isFlatOriginal: false,
+            const result = await service.iconifyApps({
+                apps,
+                size: 128,
             });
 
-            const result = await service.getIconStream({
-                appUid: 'app-abc',
-                appIcon: redirectUrl,
-                size: 256,
-                allowRedirect: true,
-            });
-
-            expect(result.redirectUrl).toBe(redirectUrl);
+            expect(result[0].icon).toBe('existing-icon');
         });
     });
 });

src/backend/src/modules/data-access/AppService.js

@@ -190,13 +190,13 @@ export default class AppService extends BaseService {
                 const icon_size = params.icon_size;
                 const svc_appIcon = this.context.get('services').get('app-icon');
                 try {
-                    const iconResult = await svc_appIcon.getIconStream({
+                    const iconPath = svc_appIcon.getAppIconPath({
                         appUid: row.uid,
-                        appIcon: row.icon,
                         size: icon_size,
                     });
-                    console.log('this is working it looks like');
-                    app.icon = await iconResult.get_data_url();
+                    if ( iconPath ) {
+                        app.icon = iconPath;
+                    }
                 } catch (e) {
                     const svc_error = this.context.get('services').get('error-service');
                     svc_error.report('AppES:read_transform', { source: e });
@@ -318,12 +318,13 @@ export default class AppService extends BaseService {
             const icon_size = params.icon_size;
             const svc_appIcon = this.context.get('services').get('app-icon');
             try {
-                const iconResult = await svc_appIcon.getIconStream({
+                const iconPath = svc_appIcon.getAppIconPath({
                     appUid: row.uid,
-                    appIcon: row.icon,
                     size: icon_size,
                 });
-                app.icon = await iconResult.get_data_url();
+                if ( iconPath ) {
+                    app.icon = iconPath;
+                }
             } catch (e) {
                 const svc_error = this.context.get('services').get('error-service');
                 svc_error.report('AppES:read_transform', { source: e });

src/backend/src/modules/data-access/AppService.test.js

@@ -316,9 +316,7 @@ describe('AppService', () => {
             mockDb.read.mockResolvedValue([mockRow]);
 
             const mockIconService = {
-                getIconStream: vi.fn().mockResolvedValue({
-                    get_data_url: vi.fn().mockResolvedValue('data:image/png;base64,abc123'),
-                }),
+                getAppIconPath: vi.fn().mockReturnValue('/app-icon/app-uid-123/64'),
             };
 
             appService.context = {
@@ -341,12 +339,11 @@ describe('AppService', () => {
                 params: { icon_size: 64 },
             });
 
-            expect(mockIconService.getIconStream).toHaveBeenCalledWith({
+            expect(mockIconService.getAppIconPath).toHaveBeenCalledWith({
                 appUid: 'app-uid-123',
-                appIcon: 'icon.png',
                 size: 64,
             });
-            expect(result.icon).toBe('data:image/png;base64,abc123');
+            expect(result.icon).toBe('/app-icon/app-uid-123/64');
         });
 
         it('should keep original icon when icon service throws', async () => {
@@ -358,7 +355,9 @@ describe('AppService', () => {
             };
 
             const mockIconService = {
-                getIconStream: vi.fn().mockRejectedValue(new Error('Icon fetch failed')),
+                getAppIconPath: vi.fn().mockImplementation(() => {
+                    throw new Error('Icon fetch failed');
+                }),
             };
 
             appService.context = {
@@ -480,9 +479,7 @@ describe('AppService', () => {
             mockDb.read.mockResolvedValue(mockRows);
 
             const mockIconService = {
-                getIconStream: vi.fn().mockImplementation(({ appUid }) => ({
-                    get_data_url: vi.fn().mockResolvedValue(`data:image/png;base64,${appUid}`),
-                })),
+                getAppIconPath: vi.fn().mockImplementation(({ appUid, size }) => `/app-icon/${appUid}/${size}`),
             };
 
             appService.context = {
@@ -504,9 +501,9 @@ describe('AppService', () => {
                 params: { icon_size: 32 },
             });
 
-            expect(mockIconService.getIconStream).toHaveBeenCalledTimes(2);
-            expect(result[0].icon).toBe('data:image/png;base64,app-1');
-            expect(result[1].icon).toBe('data:image/png;base64,app-2');
+            expect(mockIconService.getAppIconPath).toHaveBeenCalledTimes(2);
+            expect(result[0].icon).toBe('/app-icon/app-1/32');
+            expect(result[1].icon).toBe('/app-icon/app-2/32');
         });
 
         it('should return empty array when no apps exist', async () => {

src/backend/src/om/entitystorage/AppES.js

@@ -337,12 +337,13 @@ class AppES extends BaseES {
             if ( icon_size ) {
                 const svc_appIcon = this.context.get('services').get('app-icon');
                 try {
-                    const iconResult = await svc_appIcon.getIconStream({
+                    const iconPath = svc_appIcon.getAppIconPath({
                         appUid: await entity.get('uid'),
-                        appIcon: await entity.get('icon'),
                         size: icon_size,
                     });
-                    await entity.set('icon', await iconResult.get_data_url());
+                    if ( iconPath ) {
+                        await entity.set('icon', iconPath);
+                    }
                 } catch (e) {
                     const svc_error = this.context.get('services').get('error-service');
                     svc_error.report('AppES:read_transform', { source: e });

src/backend/src/om/entitystorage/SQLES.js

@@ -18,7 +18,6 @@
  */
 const { AdvancedBase } = require('@heyputer/putility');
 const { BaseES } = require('./BaseES');
-
 const APIError = require('../../api/APIError');
 const { Entity } = require('./Entity');
 const { WeakConstructorFeature } = require('../../traits/WeakConstructorFeature');
@@ -62,7 +61,7 @@ class SQLES extends BaseES {
                     return [` WHERE ${id_col} = ?`, [uid]];
                 }
 
-                if ( ! uid.hasOwnProperty('predicate') ) {
+                if ( ! Object.prototype.hasOwnProperty.call(uid, 'predicate') ) {
                     throw new Error('SQLES.read does not understand this input: ' +
                         'object with no predicate property');
                 }
@@ -164,7 +163,7 @@ class SQLES extends BaseES {
             }
         },
 
-        async delete (uid, extra) {
+        async delete (uid) {
             const id_prop = this.om.properties[this.om.primary_identifier];
             let id_col =
                 id_prop.descriptor.sql?.column_name ?? id_prop.name;
@@ -304,7 +303,7 @@ class SQLES extends BaseES {
                 }
 
                 if ( value && options.use_id ) {
-                    if ( value.hasOwnProperty('id') ) {
+                    if ( Object.prototype.hasOwnProperty.call(value, 'id') ) {
                         value = value.id;
                     }
                 }

src/backend/src/routers/get-launch-apps.js

@@ -22,17 +22,8 @@ import { get_apps } from '../helpers.js';
 import { DB_READ } from '../services/database/consts.js';
 
 const iconify_apps = async (context, { apps, size }) => {
-    return await Promise.all(apps.map(async app => {
-        const svc_appIcon = context.services.get('app-icon');
-        const iconResult = await svc_appIcon.getIconStream({
-            appIcon: app.icon,
-            appUid: app.uid ?? app.uuid,
-            size,
-        });
-
-        app.icon = await iconResult.get_data_url();
-        return app;
-    }));
+    const svc_appIcon = context.services.get('app-icon');
+    return await svc_appIcon.iconifyApps({ apps, size });
 };
 
 // -----------------------------------------------------------------------//

src/backend/src/services/MeteringService/MeteringService.ts

@@ -683,35 +683,37 @@ export class MeteringService {
     }
 
     async #checkRateOfChange () {
-        const globalUsage = await this.getGlobalUsage();
         const now = Date.now();
         const lastChange = await this.#superUserService.sudo(async () => {
             return this.#kvStore.get({ key: `${METRICS_PREFIX}:lastGlobalUsageCheck` }) as Promise<{ total: number, timestamp: number } | null>;
         });
 
-        const currTotal = globalUsage.total;
+        if ( !lastChange || (now - lastChange.timestamp) > 4 * 60 * 1000 ) {
+            // only checked if more than 4 minutes from last check
+            const globalUsage = await this.getGlobalUsage();
+            const currTotal = globalUsage.total;
 
-        if ( lastChange ) {
-            const timeDelta = now - lastChange.timestamp;
-            const usageDelta = currTotal - lastChange.total;
-            const usagePerMinute = (usageDelta / (timeDelta / 60000));
+            if ( lastChange ) {
+                const timeDelta = now - lastChange.timestamp;
+                const usageDelta = currTotal - lastChange.total;
+                const usagePerMinute = (usageDelta / (timeDelta / 60000));
 
-            if ( usagePerMinute > MeteringService.MAX_GLOBAL_USAGE_PER_MINUTE ) {
-                this.#alarmService.create('metering:excessiveGlobalUsageRate', `Global usage rate is excessive: ${usagePerMinute} micro-cents per minute`, {
-                    usagePerMinute,
-                    maxAllowedPerMinute: MeteringService.MAX_GLOBAL_USAGE_PER_MINUTE,
-                });
+                if ( usagePerMinute > MeteringService.MAX_GLOBAL_USAGE_PER_MINUTE ) {
+                    this.#alarmService.create('metering:excessiveGlobalUsageRate', `Global usage rate is excessive: ${usagePerMinute} micro-cents per minute`, {
+                        usagePerMinute,
+                        maxAllowedPerMinute: MeteringService.MAX_GLOBAL_USAGE_PER_MINUTE,
+                    });
+                }
             }
-        }
-        await this.#superUserService.sudo(async () => {
-            await this.#kvStore.set({
-                key: `${METRICS_PREFIX}:lastGlobalUsageCheck`,
-                value: {
-                    total: currTotal,
-                    timestamp: now,
-                },
+            await this.#superUserService.sudo(async () => {
+                await this.#kvStore.set({
+                    key: `${METRICS_PREFIX}:lastGlobalUsageCheck`,
+                    value: {
+                        total: currTotal,
+                        timestamp: now,
+                    },
+                });
             });
-        });
-
+        }
     }
 }