Skip to content

Backend redis improvements #1450

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 25, 2025
Merged

Backend redis improvements #1450

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9f72ebf
feat: enhance Redis connection handling with URL validation and adjus…
Artuomka Nov 25, 2025
6a02795
fix: correct password and username assignment logic in Redis client c…
Artuomka Nov 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion backend/src/entities/connection/connection.controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ import {
import { TokenValidationResult } from './use-cases/validate-connection-token.use.case.js';
import { isTestConnectionUtil } from './utils/is-test-connection-util.js';
import { SkipThrottle } from '@nestjs/throttler';
import { isRedisConnectionUrl } from '@rocketadmin/shared-code/dist/src/data-access-layer/shared/create-data-access-object.js';

@UseInterceptors(SentryInterceptor)
@Controller()
Expand Down Expand Up @@ -243,7 +244,11 @@ export class ConnectionController {
@UserId() userId: string,
@MasterPassword() masterPwd: string,
): Promise<CreatedConnectionDTO> {
if (!createConnectionDto.password && !isConnectionTypeAgent(createConnectionDto.type)) {
if (
!createConnectionDto.password &&
!isConnectionTypeAgent(createConnectionDto.type) &&
!isRedisConnectionUrl(createConnectionDto.host)
) {
throw new BadRequestException(Messages.PASSWORD_MISSING);
}
if (createConnectionDto.masterEncryption && !masterPwd) {
Expand Down Expand Up @@ -709,6 +714,10 @@ export class ConnectionController {
return errors;
}

if (isRedisConnectionUrl(connectionData.host)) {
return errors;
}
Comment on lines +717 to +719
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] When a Redis connection URL is detected, the validation returns early without validating the port field. However, the subsequent code (lines 740-742) still expects port to be a valid number. This could lead to issues if someone provides a Redis URL but also provides an invalid port value.

Consider either:

  1. Validating that port is not provided when using a connection URL, or
  2. Skipping port validation specifically for Redis URLs by checking the connection type as well:
if (isRedisConnectionUrl(connectionData.host)) {
  return errors;
}

if (typeof connectionData.port !== 'number') errors.push(Messages.PORT_FORMAT_INCORRECT);

Or move the port validation before the early return.

Copilot uses AI. Check for mistakes.

if (!connectionData.username && connectionData.type !== ConnectionTypesEnum.redis)
errors.push(Messages.USERNAME_MISSING);

Expand Down
10 changes: 7 additions & 3 deletions backend/src/entities/connection/use-cases/test-connection.use.case.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ import { getRepository } from 'typeorm';
import AbstractUseCase from '../../../common/abstract-use.case.js';
import { IGlobalDatabaseContext } from '../../../common/application/global-database-context.interface.js';
import { BaseType } from '../../../common/data-injection.tokens.js';
import { getDataAccessObject } from '@rocketadmin/shared-code/dist/src/data-access-layer/shared/create-data-access-object.js';
import {
getDataAccessObject,
isRedisConnectionUrl,
} from '@rocketadmin/shared-code/dist/src/data-access-layer/shared/create-data-access-object.js';
import { Messages } from '../../../exceptions/text/messages.js';
import { processExceptionMessage } from '../../../exceptions/utils/process-exception-message.js';
import { isConnectionTypeAgent, slackPostMessage } from '../../../helpers/index.js';
Expand Down Expand Up @@ -96,7 +99,8 @@ export class TestConnectionUseCase
if (
!connectionData.password &&
(connectionData.host !== toUpdate.host || connectionData.port !== toUpdate.port) &&
!isConnectionTypeAgent(connectionData.type)
!isConnectionTypeAgent(connectionData.type) &&
!isRedisConnectionUrl(connectionData.host)
) {
return {
result: false,
Expand Down Expand Up @@ -125,7 +129,7 @@ export class TestConnectionUseCase
};
}
} else {
if (!connectionData.password) {
if (!connectionData.password && !isConnectionTypeAgent(connectionData.type) && !isRedisConnectionUrl(connectionData.host)) {
return {
result: false,
message: Messages.PASSWORD_MISSING,
Expand Down
9 changes: 6 additions & 3 deletions shared-code/src/data-access-layer/data-access-objects/data-access-object-redis.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { ValidateTableSettingsDS } from '../shared/data-structures/validate-tabl
import { FilterCriteriaEnum } from '../shared/enums/filter-criteria.enum.js';
import { IDataAccessObject } from '../shared/interfaces/data-access-object.interface.js';
import { BasicDataAccessObject } from './basic-data-access-object.js';
import { isRedisConnectionUrl } from '../shared/create-data-access-object.js';

export class DataAccessObjectRedis extends BasicDataAccessObject implements IDataAccessObject {
constructor(connection: ConnectionParams) {
Expand Down Expand Up @@ -857,6 +858,7 @@ export class DataAccessObjectRedis extends BasicDataAccessObject implements IDat
try {
if (!client) {
const shouldUseTLS = this.connection.ssl !== false;
const isConnectionUrl = isRedisConnectionUrl(this.connection.host);

const socketConfig: any = {
host: this.connection.host,
Expand All @@ -880,9 +882,10 @@ export class DataAccessObjectRedis extends BasicDataAccessObject implements IDat
}

client = createClient({
socket: socketConfig,
password: this.connection.password || undefined,
username: this.connection.username || undefined,
socket: isConnectionUrl ? undefined : socketConfig,
url: isConnectionUrl ? this.connection.host : undefined,
password: isConnectionUrl ? undefined : this.connection.password ? this.connection.password : undefined,
username: isConnectionUrl ? undefined : this.connection.username ? this.connection.username : undefined,
Comment on lines +887 to +888
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant ternary expression. The condition this.connection.password ? this.connection.password : undefined can be simplified to just this.connection.password || undefined or even simpler this.connection.password since empty strings are falsy in JavaScript. The same applies to line 888 for username.

Consider simplifying to:

password: isConnectionUrl ? undefined : this.connection.password || undefined,
username: isConnectionUrl ? undefined : this.connection.username || undefined,
Suggested change
password: isConnectionUrl ? undefined : this.connection.password ? this.connection.password : undefined,
username: isConnectionUrl ? undefined : this.connection.username ? this.connection.username : undefined,
password: isConnectionUrl ? undefined : this.connection.password,
username: isConnectionUrl ? undefined : this.connection.username,

Copilot uses AI. Check for mistakes.
database: database,
});

Expand Down
7 changes: 6 additions & 1 deletion shared-code/src/data-access-layer/shared/create-data-access-object.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ function buildConnectionParams(connectionParams: IUnknownConnectionParams): Conn

const sqlAndMongoRequiredKeys = ['type', 'host', 'port', 'username', 'password', 'database'];
const elasticAndDynamoAndRedisRequiredKeys = ['host', 'username', 'password'];
const redisRequiredKeys = ['host', 'port', 'password'];
const redisRequiredKeys = !isRedisConnectionUrl(connectionParams.host) ? ['host', 'port', 'password'] : ['host'];

switch (connectionParams.type) {
case ConnectionTypesEnum.postgres:
Expand Down Expand Up @@ -155,3 +155,8 @@ function buildConnectionParams(connectionParams: IUnknownConnectionParams): Conn
};
return connection;
}

export function isRedisConnectionUrl(host: string): boolean {
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The isRedisConnectionUrl function does not handle null or undefined values for the host parameter. If connectionParams.host is null or undefined, calling .test() on it will throw a runtime error. Consider adding a null check:

export function isRedisConnectionUrl(host: string): boolean {
  if (!host) {
    return false;
  }
  const redisUrlPattern = /^rediss?:\/\/.+/i;
  return redisUrlPattern.test(host);
}
Suggested change
export function isRedisConnectionUrl(host: string): boolean {
export function isRedisConnectionUrl(host: string): boolean {
if (!host) {
return false;
}

Copilot uses AI. Check for mistakes.
const redisUrlPattern = /^rediss?:\/\/.+/i;
return redisUrlPattern.test(host);
}
Loading