Skip to content

fix: trim db urls and remove special characters during backup restore #36201

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
pratapaprasanna merged 1 commit into from
Sep 13, 2024
Merged

fix: trim db urls and remove special characters during backup restore #36201

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions deploy/docker/fs/opt/appsmith/utils/bin/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ function getDburl() {
let env_array = fs.readFileSync(Constants.ENV_PATH, 'utf8').toString().split("\n");
for (let i in env_array) {
if (env_array[i].startsWith("APPSMITH_MONGODB_URI") || env_array[i].startsWith("APPSMITH_DB_URL")) {
dbUrl = env_array[i].toString().split("=")[1];
dbUrl = env_array[i].toString().split("=")[1].trim();
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 9, 2024

Choose a reason for hiding this comment

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

Ensure consistent handling of undefined or null values.

The line dbUrl = env_array[i].toString().split("=")[1].trim(); assumes that the split operation results in at least two elements. If the environment variable is malformed and does not contain an "=", this will result in an undefined value, which will cause an error when trim() is called.

To prevent runtime errors, consider checking if the split operation results in the expected number of elements before calling trim():

- dbUrl = env_array[i].toString().split("=")[1].trim();
+ const parts = env_array[i].toString().split("=");
+ dbUrl = parts.length > 1 ? parts[1].trim() : '';
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
dbUrl = env_array[i].toString().split("=")[1].trim();
const parts = env_array[i].toString().split("=");
dbUrl = parts.length > 1 ? parts[1].trim() : '';

break; // Break early when the desired line is found
}
}
Expand All @@ -48,7 +48,7 @@ function getDburl() {
let dbEnvUrl = process.env.APPSMITH_DB_URL || process.env.APPSMITH_MONGO_DB_URI;
// Make sure dbEnvUrl takes precedence over dbUrl
if (dbEnvUrl && dbEnvUrl !== "undefined") {
dbUrl = dbEnvUrl;
dbUrl = dbEnvUrl.trim();
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 9, 2024

Choose a reason for hiding this comment

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

Good use of trim() to ensure clean database URLs.

The addition of trim() on the line dbUrl = dbEnvUrl.trim(); is a good practice to ensure that any leading or trailing whitespace is removed from the database URL. This change directly addresses the issue outlined in the PR summary and should help prevent errors related to improperly formatted URIs in backup operations.

However, as mentioned in the PR summary, there's a need to remove special characters as well. Consider extending this functionality to also filter out any unwanted characters that might cause parsing issues:

- dbUrl = dbEnvUrl.trim();
+ dbUrl = dbEnvUrl.trim().replace(/[^a-zA-Z0-9:/.@]+/g, '');

This regex will remove any characters that are not alphanumeric, colon, slash, period, or at symbol, which are typically safe for URIs.

Committable suggestion was skipped due to low confidence.

}
return dbUrl;
}
Expand Down