mod download issue

[PhilipJFryFAF]

when trying to download the common mod tools mod i (and another user who reported the bug) receive the following error

java.util.concurrent.CompletionException: java.io.IOException: Server returned HTTP response code: 400 for URL: http://content.faforever.com/mods/Common Mod Tools.v0001.zip
	at java.base/java.util.concurrent.CompletableFuture.encodeThrowable(Unknown Source)
	at java.base/java.util.concurrent.CompletableFuture.completeThrowable(Unknown Source)
	at java.base/java.util.concurrent.CompletableFuture$UniRun.tryFire(Unknown Source)
	at java.base/java.util.concurrent.CompletableFuture.postComplete(Unknown Source)
	at java.base/java.util.concurrent.CompletableFuture.completeExceptionally(Unknown Source)
	at com.faforever.client.task.CompletableTask.lambda$new$1(CompletableTask.java:17)
	at javafx.base/com.sun.javafx.event.CompositeEventHandler.dispatchBubblingEvent(Unknown Source)
	at javafx.base/com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(Unknown Source)
	at javafx.base/com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(Unknown Source)
	at javafx.base/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(Unknown Source)
	at javafx.base/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(Unknown Source)
	at javafx.base/com.sun.javafx.event.EventUtil.fireEventImpl(Unknown Source)
	at javafx.base/com.sun.javafx.event.EventUtil.fireEvent(Unknown Source)
	at javafx.base/javafx.event.Event.fireEvent(Unknown Source)
	at javafx.graphics/javafx.concurrent.EventHelper.fireEvent(Unknown Source)
	at javafx.graphics/javafx.concurrent.Task.fireEvent(Unknown Source)
	at javafx.graphics/javafx.concurrent.Task.setState(Unknown Source)
	at javafx.graphics/javafx.concurrent.Task$TaskCallable.lambda$call$2(Unknown Source)
	at javafx.graphics/com.sun.javafx.application.PlatformImpl.lambda$runLater$9(Unknown Source)
	at java.base/java.security.AccessController.doPrivileged(Native Method)
	at javafx.graphics/com.sun.javafx.application.PlatformImpl.lambda$runLater$10(Unknown Source)
	at javafx.graphics/com.sun.glass.ui.InvokeLaterDispatcher$Future.run(Unknown Source)
	at javafx.graphics/com.sun.glass.ui.win.WinApplication._runLoop(Native Method)
	at javafx.graphics/com.sun.glass.ui.win.WinApplication.lambda$runLoop$3(Unknown Source)
	at java.base/java.lang.Thread.run(Unknown Source)
Caused by: java.io.IOException: Server returned HTTP response code: 400 for URL: http://content.faforever.com/mods/Common Mod Tools.v0001.zip
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(Unknown Source)
	at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(Unknown Source)
	at java.base/java.lang.reflect.Constructor.newInstance(Unknown Source)
	at java.base/sun.net.www.protocol.http.HttpURLConnection$10.run(Unknown Source)
	at java.base/sun.net.www.protocol.http.HttpURLConnection$10.run(Unknown Source)
	at java.base/java.security.AccessController.doPrivileged(Native Method)
	at java.base/sun.net.www.protocol.http.HttpURLConnection.getChainedException(Unknown Source)
	at java.base/sun.net.www.protocol.http.HttpURLConnection.getInputStream0(Unknown Source)
	at java.base/sun.net.www.protocol.http.HttpURLConnection.getInputStream(Unknown Source)
	at com.faforever.client.mod.InstallModTask.call(InstallModTask.java:66)
	at com.faforever.client.mod.InstallModTask.call(InstallModTask.java:32)
	at javafx.graphics/javafx.concurrent.Task$TaskCallable.call(Unknown Source)
	at java.base/java.util.concurrent.FutureTask.run(Unknown Source)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source)
	... 1 more
Caused by: java.io.IOException: Server returned HTTP response code: 400 for URL: http://content.faforever.com/mods/Common Mod Tools.v0001.zip
	at java.base/sun.net.www.protocol.http.HttpURLConnection.getInputStream0(Unknown Source)
	at java.base/sun.net.www.protocol.http.HttpURLConnection.getInputStream(Unknown Source)
	at java.base/sun.net.www.protocol.http.HttpURLConnection.getHeaderField(Unknown Source)
	at java.base/java.net.URLConnection.getHeaderFieldLong(Unknown Source)
	at java.base/java.net.URLConnection.getContentLengthLong(Unknown Source)
	at java.base/java.net.URLConnection.getContentLength(Unknown Source)
	at com.faforever.client.mod.InstallModTask.call(InstallModTask.java:63)
	... 6 more

downloading the mod works when doing so via the http://content.faforever.com/mods/Common Mod Tools.v0001.zip or via the legacy client
i assume the client

[Wizek]

I have the same issue. I think the issue is that " " (whitespace) is not encoded to be "%20" in the request URL.

[PhilipJFryFAF]

https://cdn.discordapp.com/attachments/202928463076786176/607247319678713897/unknown.png
another user on discord ran into that issue today

[Wizek]

It seems this is the critical part in the code:

downlords-faf-client/src/main/java/com/faforever/client/mod/InstallModTask.java

Lines 52 to 63 in b289e1d

	protected Void call() throws Exception {
	Objects.requireNonNull(url, "url has not been set");
	Path tempFile = Files.createTempFile(preferencesService.getCacheDirectory(), "mod", null);
	logger.info("Downloading mod {} to {}", url, tempFile);
	updateTitle(i18n.get("downloadingModTask.downloading", url));
	Files.createDirectories(tempFile.getParent());
	URLConnection urlConnection = url.openConnection();
	int contentLength = urlConnection.getContentLength();

I don't have much Java experience -- but I do have some with other languages, and maybe all we need here is to use encode? https://www.geeksforgeeks.org/java-net-urlencoder-class-java/

I gave it a try in a repl, and as expected, the spaces are encoded, but not to "%20"s, instead they turned to "+"es. Which would be fine if the server conformed to url decoding standards, which it seems it doesn't do, since:

• http://content.faforever.com/mods/Supreme%20Economy%202.2.v0002.zip works, while
• http://content.faforever.com/mods/Supreme+Economy+2.2.v0002.zip doesn't work, gives 404.

Okay, so I'm onto a bit more hacky solution to do search and replace of white-spaces.

This might do the trick:

System.out.println(URLEncoder.encode(url).replaceAll("\\+", "%20"));

This way everything is encoded, and we give special consideration to whitespace to be %20 and not +.

Maybe I can try to send in a pull request based on this.

[Wizek]

Started a PR here: #1381

[Katharsas]

Since Guava is already used, you could use UrlEscapers.urlFragmentEscaper().escape(input);.
And i think we also use Apache commons, so URIUtil.encodeQuery(input) would probably work too.
(see https://stackoverflow.com/questions/4737841/urlencoder-not-able-to-translate-space-character)

I think the real underlying problem is that com.faforever.client.api.dto.ModVersion uses URL incorrectly (or whatever code does the dto conversion to the dto object), since URL.encode was never meant to encode non query parameter part of a URL. This is maybe a cross-cutting issue with all DTO classes that use URL.
@Brutus5000

Whereever the URL object gets created from string (maybe some preconfigured converter), there is the actual bug.

[Katharsas]

This is a cross-cutting issue im pretty sure. Most likely, all classes inside com.faforever.client.api.dto are affected by wrong URL object from json conversion.