Skip to content

Fix shell script parameter duplication and improve consistency#2622

Merged
lvca merged 2 commits intomainfrom
copilot/fix-duplicate-java-parameters
Oct 9, 2025
Merged

Fix shell script parameter duplication and improve consistency#2622
lvca merged 2 commits intomainfrom
copilot/fix-duplicate-java-parameters

Conversation

Copy link
Contributor

Copilot AI commented Oct 9, 2025

Problem

The server.sh bootup script was duplicating Java command-line arguments, and there were inconsistencies across the shell scripts that could lead to service misconfiguration.

Critical Issue: Duplicate Arguments in server.sh

The server.sh script was using both $* and "$@" together on line 103, causing all command-line arguments to be duplicated when passed to the Java process:

# Before (buggy):
exec "$JAVA" ... $ARGS $* "$@" com.arcadedb.server.ArcadeDBServer
                          ^^^ ^^^^
                          Both expand to all arguments!

# Example issue:
./server.sh --config=custom.yaml --debug
# Would pass: --config=custom.yaml --debug --config=custom.yaml --debug
# to the Java process, causing misconfiguration

Solution

1. Fixed server.sh argument duplication (Line 103)

  • Removed duplicate $* parameter expansion
  • Now correctly uses only "$@" to pass arguments once

2. Added ARCADEDB_OPTS_MEMORY initialization to console.sh

  • Added defensive initialization check (lines 49-52)
  • Ensures consistency with server.sh behavior
  • Prevents potential shell expansion errors when variable is unset

3. Fixed restore.sh ARCADEDB_HOME validation (Line 35)

  • Changed from checking bin/console.sh to bin/restore.sh
  • Each script now validates its own existence correctly

4. Added ARCADEDB_OPTS_MEMORY initialization to restore.sh

  • Added defensive initialization check (lines 49-53)
  • Maintains consistency across all three scripts

5. Fixed restore.sh argument expansion (Line 70)

  • Changed from $* to "$@" for proper argument handling
  • Now correctly handles file paths with spaces

Impact

Before:

# server.sh duplicated all arguments
./server.sh --config=server.yaml --debug
→ java ... --config=server.yaml --debug --config=server.yaml --debug ...

# restore.sh failed with spaces in paths
./restore.sh --file "my database backup.zip"
→ Could break due to improper argument expansion

After:

# Arguments passed correctly, once
./server.sh --config=server.yaml --debug
→ java ... --config=server.yaml --debug ...

# Properly handles paths with spaces
./restore.sh --file "my database backup.zip"
→ Works correctly with quoted arguments

Testing

All scripts have been validated:

  • ✅ Syntax validation passed for all scripts
  • ✅ No duplicate arguments in any script
  • ✅ Consistent variable initialization across all scripts
  • ✅ Proper argument handling with "$@"
  • ✅ Each script validates its own existence

Backward Compatibility

100% backward compatible - These changes fix bugs and improve robustness without breaking existing functionality:

  • Removing duplicate arguments fixes incorrect behavior
  • Variable initialization is defensive (no change if already set)
  • ARCADEDB_HOME check is more correct but functionally equivalent
  • Argument expansion improvements maintain compatibility while fixing edge cases

No migration, configuration changes, or special deployment steps required.

Files Changed

  • package/src/main/scripts/server.sh - Fixed duplicate argument expansion
  • package/src/main/scripts/console.sh - Added ARCADEDB_OPTS_MEMORY initialization
  • package/src/main/scripts/restore.sh - Fixed ARCADEDB_HOME check, added variable initialization, fixed argument expansion

Total: 3 files changed, 12 insertions(+), 3 deletions(-)

Original prompt

This section details on the original issue you should resolve

<issue_title>Bootup shell script is duplicating Java parameters</issue_title>
<issue_description>## What

The server.sh is duplicating the java parameters unnecessarily. There's also an environment variable which is not sanitised and could lead to service misconfiguration.

I would suggest to review the other scripts, console.sh and restore.sh in order to keep it coherent.</issue_description>

<agent_instructions>Check if the change can impact existing installations
</agent_instructions>

Comments on the Issue (you are @copilot in this section)

Fixes #2621


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

- server.sh: Remove duplicate argument expansion ($* and "$@")
- console.sh: Add ARCADEDB_OPTS_MEMORY initialization for consistency
- restore.sh: Fix ARCADEDB_HOME check (console.sh -> restore.sh)
- restore.sh: Add ARCADEDB_OPTS_MEMORY initialization
- restore.sh: Fix argument expansion ($* -> "$@")

Co-authored-by: lvca <[email protected]>
Copilot AI changed the title [WIP] Fix duplication of Java parameters in server.sh Fix shell script parameter duplication and improve consistency Oct 9, 2025
Copilot AI requested a review from lvca October 9, 2025 16:20
Copy link
Contributor

@carlos-rodrigues-8x8 carlos-rodrigues-8x8 left a comment

Choose a reason for hiding this comment

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

Seems pretty good. I'm just wondering if we need $ARGS on scripts.

@lvca
Copy link
Contributor

lvca commented Oct 9, 2025

No sure why we have $ARGS, I guess you can use it from docker or managed environment to pass arguments easily?

@lvca lvca marked this pull request as ready for review October 9, 2025 18:05
@lvca lvca merged commit 0c1618a into main Oct 9, 2025
8 of 9 checks passed
@lvca lvca deleted the copilot/fix-duplicate-java-parameters branch October 9, 2025 18:05
@codacy-production
Copy link

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
Report missing for f5649951
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (f564995) Report Missing Report Missing Report Missing
Head commit (4564ec2) 72884 46119 63.28%

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#2622) 0 0 ∅ (not applicable)

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

Footnotes

  1. Codacy didn't receive coverage data for the commit, or there was an error processing the received data. Check your integration for errors and validate that your coverage setup is correct.

@carlos-rodrigues-8x8
Copy link
Contributor

No sure why we have $ARGS, I guess you can use it from docker or managed environment to pass arguments easily?

We can use it in any context, docker, k8s, baremetal, my point is if we already have ARCADEDB_SETTINGS and $@ seems a bit overkill.

robfrank pushed a commit that referenced this pull request Nov 10, 2025
* Initial plan

* Fix shell script parameter duplication and improve consistency

- server.sh: Remove duplicate argument expansion ($* and "$@")
- console.sh: Add ARCADEDB_OPTS_MEMORY initialization for consistency
- restore.sh: Fix ARCADEDB_HOME check (console.sh -> restore.sh)
- restore.sh: Add ARCADEDB_OPTS_MEMORY initialization
- restore.sh: Fix argument expansion ($* -> "$@")

Co-authored-by: lvca <[email protected]>

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: lvca <[email protected]>
(cherry picked from commit 0c1618a)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bootup shell script is duplicating Java parameters

3 participants