Fix: launch string too long in minecraft forge 1.19 (#1384)

* fix: version 1.19 launch string too long * fix: removed console log * fix: getJVMArguments112 removed (") * fix: remove quotes in the java launch string only on windows * fix: launch java process without shell only on win * fix: launch java process without shell only on win * fix: remove quotes in the java launch string only on windows (java path) * fix: remove quotes in the java launch string only on windows (Dlog4j) * fix: missing quotes * fix: my fault * fix: testing java libaries missing quotes on macos * fix: test * fix: fix replaceLibraryDirectory Co-authored-by: Davide Ceschia <10165264+killpowa@users.noreply.github.com>
gorilla-devs · Jul 4, 2022 · 56552c1 · 56552c1
1 parent 572db94
commit 56552c1
Show file tree

Hide file tree

Showing 3 changed files with 70 additions and 33 deletions.
diff --git a/src/app/desktop/utils/index.js b/src/app/desktop/utils/index.js
@@ -17,6 +17,7 @@ import {
 } from '../../../common/utils/constants';
 
 import {
+  addQuotes,
   removeDuplicates,
   sortByForgeVersionDesc
 } from '../../../common/utils';
@@ -483,13 +484,14 @@ export const getJVMArguments112 = (
   hideAccessToken,
   jvmOptions = []
 ) => {
+  const needsQuote = process.platform !== 'win32';
   const args = [];
   args.push('-cp');
 
   args.push(
     [...libraries, mcjar]
       .filter(l => !l.natives)
-      .map(l => `"${l.path}"`)
+      .map(l => `${addQuotes(needsQuote, l.path)}`)
       .join(process.platform === 'win32' ? ';' : ':')
   );
 
@@ -501,8 +503,15 @@ export const getJVMArguments112 = (
   args.push(`-Xmx${memory}m`);
   args.push(`-Xms${memory}m`);
   args.push(...jvmOptions);
-  args.push(`-Djava.library.path="${path.join(instancePath, 'natives')}"`);
-  args.push(`-Dminecraft.applet.TargetDirectory="${instancePath}"`);
+  args.push(
+    `-Djava.library.path=${addQuotes(
+      needsQuote,
+      path.join(instancePath, 'natives')
+    )}`
+  );
+  args.push(
+    `-Dminecraft.applet.TargetDirectory=${addQuotes(needsQuote, instancePath)}`
+  );
   if (mcJson.logging) {
     args.push(mcJson?.logging?.client?.argument || '');
   }
@@ -524,13 +533,13 @@ export const getJVMArguments112 = (
           val = mcJson.id;
           break;
         case 'game_directory':
-          val = `"${instancePath}"`;
+          val = `${addQuotes(needsQuote, instancePath)}`;
           break;
         case 'assets_root':
-          val = `"${assetsPath}"`;
+          val = `${addQuotes(needsQuote, assetsPath)}`;
           break;
         case 'game_assets':
-          val = `"${path.join(assetsPath, 'virtual', 'legacy')}"`;
+          val = `${path.join(assetsPath, 'virtual', 'legacy')}`;
           break;
         case 'assets_index_name':
           val = mcJson.assets;
@@ -559,6 +568,9 @@ export const getJVMArguments112 = (
       if (val != null) {
         mcArgs[i] = val;
       }
+      if (typeof args[i] === 'string' && !needsQuote) {
+        args[i] = args[i].replaceAll('"', '');
+      }
     }
   }
 
@@ -586,6 +598,7 @@ export const getJVMArguments113 = (
 ) => {
   const argDiscovery = /\${*(.*)}/;
   let args = mcJson.arguments.jvm.filter(v => !skipLibrary(v));
+  const needsQuote = process.platform !== 'win32';
 
   // if (process.platform === "darwin") {
   //   args.push("-Xdock:name=instancename");
@@ -594,7 +607,9 @@ export const getJVMArguments113 = (
 
   args.push(`-Xmx${memory}m`);
   args.push(`-Xms${memory}m`);
-  args.push(`-Dminecraft.applet.TargetDirectory="${instancePath}"`);
+  args.push(
+    `-Dminecraft.applet.TargetDirectory=${addQuotes(needsQuote, instancePath)}`
+  );
   if (mcJson.logging) {
     args.push(mcJson?.logging?.client?.argument || '');
   }
@@ -612,9 +627,9 @@ export const getJVMArguments113 = (
   for (let i = 0; i < args.length; i += 1) {
     if (typeof args[i] === 'object' && args[i].rules) {
       if (typeof args[i].value === 'string') {
-        args[i] = `"${args[i].value}"`;
+        args[i] = `${addQuotes(needsQuote, args[i].value)}`;
       } else if (typeof args[i].value === 'object') {
-        args.splice(i, 1, ...args[i].value.map(v => `"${v}"`));
+        args.splice(i, 1, ...args[i].value.map(v => `${v}`));
       }
       i -= 1;
     } else if (typeof args[i] === 'string') {
@@ -629,10 +644,10 @@ export const getJVMArguments113 = (
             val = mcJson.id;
             break;
           case 'game_directory':
-            val = `"${instancePath}"`;
+            val = `${addQuotes(needsQuote, instancePath)}`;
             break;
           case 'assets_root':
-            val = `"${assetsPath}"`;
+            val = `${addQuotes(needsQuote, assetsPath)}`;
             break;
           case 'assets_index_name':
             val = mcJson.assets;
@@ -658,7 +673,7 @@ export const getJVMArguments113 = (
           case 'natives_directory':
             val = args[i].replace(
               argDiscovery,
-              `"${path.join(instancePath, 'natives')}"`
+              `${addQuotes(needsQuote, path.join(instancePath, 'natives'))}`
             );
             break;
           case 'launcher_name':
@@ -670,7 +685,7 @@ export const getJVMArguments113 = (
           case 'classpath':
             val = [...libraries, mcjar]
               .filter(l => !l.natives)
-              .map(l => `"${l.path}"`)
+              .map(l => `${addQuotes(needsQuote, l.path)}`)
               .join(process.platform === 'win32' ? ';' : ':');
             break;
           default:
@@ -680,6 +695,7 @@ export const getJVMArguments113 = (
           args[i] = val;
         }
       }
+      if (!needsQuote) args[i] = args[i].replaceAll('"', '');
     }
   }
 

diff --git a/src/common/reducers/actions.js b/src/common/reducers/actions.js
@@ -116,10 +116,12 @@ import {
   downloadInstanceFiles
 } from '../../app/desktop/utils/downloader';
 import {
+  addQuotes,
   getFileMurmurHash2,
   getSize,
   makeInstanceRestorePoint,
-  removeDuplicates
+  removeDuplicates,
+  replaceLibraryDirectory
 } from '../utils';
 import { UPDATE_CONCURRENT_DOWNLOADS } from './settings/actionTypes';
 import { UPDATE_MODAL } from './modals/actionTypes';
@@ -2966,20 +2968,18 @@ export function launchInstance(instanceName, forceQuit = false) {
             mcJson.forge = { arguments: {} };
             mcJson.forge.arguments.jvm = forgeJson.version.arguments.jvm.map(
               arg => {
-                return arg
-                  .replace(/\${version_name}/g, mcJson.id)
-                  .replace(
-                    /=\${library_directory}/g,
-                    `="${_getLibrariesPath(state)}"`
-                  )
-                  .replace(
-                    /\${library_directory}/g,
-                    `${_getLibrariesPath(state)}`
-                  )
-                  .replace(
-                    /\${classpath_separator}/g,
-                    process.platform === 'win32' ? ';' : ':'
-                  );
+                return replaceLibraryDirectory(
+                  arg
+                    .replace(/\${version_name}/g, mcJson.id)
+                    .replace(
+                      /=\${library_directory}/g,
+                      `="${_getLibrariesPath(state)}"`
+                    ),
+                  _getLibrariesPath(state)
+                ).replace(
+                  /\${classpath_separator}/g,
+                  process.platform === 'win32' ? '";' : '":'
+                );
               }
             );
           }
@@ -3082,8 +3082,10 @@ export function launchInstance(instanceName, forceQuit = false) {
       loggingId || ''
     );
 
+    const needsQuote = process.platform !== 'win32';
+
     console.log(
-      `"${javaPath}" ${getJvmArguments(
+      `${addQuotes(needsQuote, javaPath)} ${getJvmArguments(
         libraries,
         mcMainFile,
         instancePath,
@@ -3099,7 +3101,7 @@ export function launchInstance(instanceName, forceQuit = false) {
         .replace(
           // eslint-disable-next-line no-template-curly-in-string
           '-Dlog4j.configurationFile=${path}',
-          `-Dlog4j.configurationFile="${loggingPath}"`
+          `-Dlog4j.configurationFile=${addQuotes(needsQuote, loggingPath)}`
         )
     );
 
@@ -3110,20 +3112,20 @@ export function launchInstance(instanceName, forceQuit = false) {
     let closed = false;
 
     const ps = spawn(
-      `"${javaPath}"`,
+      `${addQuotes(needsQuote, javaPath)}`,
       jvmArguments.map(v =>
         v
           .toString()
           .replace(...replaceRegex)
           .replace(
             // eslint-disable-next-line no-template-curly-in-string
             '-Dlog4j.configurationFile=${path}',
-            `-Dlog4j.configurationFile="${loggingPath}"`
+            `-Dlog4j.configurationFile=${addQuotes(needsQuote, loggingPath)}`
           )
       ),
       {
         cwd: instancePath,
-        shell: true
+        shell: process.platform !== 'win32'
       }
     );
 

diff --git a/src/common/utils/index.js b/src/common/utils/index.js
@@ -285,3 +285,22 @@ export const getSize = async dir => {
       .catch(e => reject(e));
   });
 };
+
+export const addQuotes = (needsQuote, string) => {
+  return needsQuote ? `"${string}"` : string;
+};
+
+export const replaceLibraryDirectory = (arg, librariesDir) => {
+  const parsedArg = arg.replace(/\${library_directory}/g, `"${librariesDir}`);
+  const regex = /\${classpath_separator}/g;
+  const isLibrariesArgString = arg.match(regex);
+  const splittedString = parsedArg.split(regex);
+  splittedString[splittedString.length - 1] = `${
+    splittedString[splittedString.length - 1]
+  }"`;
+
+  return isLibrariesArgString
+    ? // eslint-disable-next-line no-template-curly-in-string
+      splittedString.join('${classpath_separator}')
+    : arg;
+};