Skip to content

Commit

Permalink
lint: fixes + baseline
Browse files Browse the repository at this point in the history
Signed-off-by: Alon Blayer-Gat <[email protected]>
  • Loading branch information
Alon Blayer-Gat committed Oct 21, 2023
1 parent 3dc3fd9 commit 3c128d6
Show file tree
Hide file tree
Showing 33 changed files with 5,936 additions and 327 deletions.
38 changes: 1 addition & 37 deletions .idea/inspectionProfiles/Default.xml

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions tunnel/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ android {
lint {
disable += "LongLogTag"
disable += "NewApi"
baseline = file("lint-baseline.xml")
}
publishing {
singleVariant("release") {
Expand Down
15 changes: 15 additions & 0 deletions tunnel/lint-baseline.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<?xml version="1.0" encoding="UTF-8"?>
<issues format="6" by="lint 8.3.0-alpha09" type="baseline" client="gradle" dependencies="false" name="AGP (8.3.0-alpha09)" variant="all" version="8.3.0-alpha09">

<issue
id="UnsafeDynamicallyLoadedCode"
message="Dynamically loading code using `load` is risky, please use `loadLibrary` instead when possible"
errorLine1=" System.load(f.getAbsolutePath());"
errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~">
<location
file="src/main/java/com/wireguard/android/util/SharedLibraryLoader.java"
line="80"
column="17"/>
</issue>

</issues>
21 changes: 12 additions & 9 deletions tunnel/src/main/java/com/wireguard/android/backend/GoBackend.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@

import android.content.Context;
import android.content.Intent;
import android.os.Build;
import android.net.VpnService.Builder;
import android.os.Build.VERSION;
import android.os.Build.VERSION_CODES;
import android.os.ParcelFileDescriptor;
import android.system.OsConstants;
import android.util.Log;
Expand All @@ -24,14 +26,14 @@
import com.wireguard.util.NonNullForAll;

import java.net.InetAddress;
import java.time.Instant;
import java.util.Collections;
import java.util.Set;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.FutureTask;
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.regex.Pattern;

import androidx.annotation.Nullable;
import androidx.collection.ArraySet;
Expand All @@ -43,6 +45,7 @@
@NonNullForAll
public final class GoBackend implements Backend {
private static final int DNS_RESOLUTION_RETRIES = 10;
private static final Pattern COMPILE = Pattern.compile("\\n");
private static final String TAG = "WireGuard/GoBackend";
@Nullable private static AlwaysOnCallback alwaysOnCallback;
private static GhettoCompletableFuture<VpnService> vpnService = new GhettoCompletableFuture<>();
Expand Down Expand Up @@ -127,7 +130,7 @@ public Statistics getStatistics(final Tunnel tunnel) {
long rx = 0;
long tx = 0;
long latestHandshakeMSec = 0;
for (final String line : config.split("\\n")) {
for (final String line : COMPILE.split(config)) {
if (line.startsWith("public_key=")) {
if (key != null)
stats.add(key, rx, tx, latestHandshakeMSec);
Expand Down Expand Up @@ -212,7 +215,7 @@ public State setState(final Tunnel tunnel, State state, @Nullable final Config c
if (currentTunnel != null)
setStateInternal(currentTunnel, null, State.DOWN);
try {
setStateInternal(tunnel, config, state);
setStateInternal(tunnel, config, State.UP);
} catch (final Exception e) {
if (originalTunnel != null)
setStateInternal(originalTunnel, originalConfig, State.UP);
Expand All @@ -232,7 +235,7 @@ private void setStateInternal(final Tunnel tunnel, @Nullable final Config config
if (config == null)
throw new BackendException(Reason.TUNNEL_MISSING_CONFIG);

if (VpnService.prepare(context) != null)
if (android.net.VpnService.prepare(context) != null)
throw new BackendException(Reason.VPN_NOT_AUTHORIZED);

final VpnService service;
Expand Down Expand Up @@ -278,7 +281,7 @@ private void setStateInternal(final Tunnel tunnel, @Nullable final Config config
final String goConfig = config.toWgUserspaceString();

// Create the vpn tunnel with android API
final VpnService.Builder builder = service.getBuilder();
final Builder builder = service.getBuilder();
builder.setSession(tunnel.getName());

for (final String excludedApplication : config.getInterface().getExcludedApplications())
Expand Down Expand Up @@ -313,9 +316,9 @@ private void setStateInternal(final Tunnel tunnel, @Nullable final Config config

builder.setMtu(config.getInterface().getMtu().orElse(1280));

if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q)
if (VERSION.SDK_INT >= VERSION_CODES.Q)
builder.setMetered(false);
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M)
if (VERSION.SDK_INT >= VERSION_CODES.M)
service.setUnderlyingNetworks(null);

builder.setBlocking(true);
Expand All @@ -338,7 +341,7 @@ private void setStateInternal(final Tunnel tunnel, @Nullable final Config config
Log.w(TAG, "Tunnel already down");
return;
}
int handleToClose = currentTunnelHandle;
final int handleToClose = currentTunnelHandle;
currentTunnel = null;
currentTunnelHandle = -1;
currentConfig = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,10 @@
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Objects;
import java.util.Set;
import java.util.regex.Pattern;

import androidx.annotation.Nullable;

Expand All @@ -41,6 +43,7 @@

@NonNullForAll
public final class WgQuickBackend implements Backend {
private static final Pattern COMPILE = Pattern.compile("\\t");
private static final String TAG = "WireGuard/WgQuickBackend";
private final File localTemporaryDir;
private final RootShell rootShell;
Expand Down Expand Up @@ -90,7 +93,7 @@ public Statistics getStatistics(final Tunnel tunnel) {
return stats;
}
for (final String line : output) {
final String[] parts = line.split("\\t");
final String[] parts = COMPILE.split(line);
if (parts.length != 8)
continue;
try {
Expand Down Expand Up @@ -127,9 +130,9 @@ public State setState(final Tunnel tunnel, State state, @Nullable final Config c
if (state == State.UP) {
toolsInstaller.ensureToolsAvailable();
if (!multipleTunnels && originalState == State.DOWN) {
final List<Pair<Tunnel, Config>> rewind = new LinkedList<>();
final Collection<Pair<Tunnel, Config>> rewind = new LinkedList<>();
try {
for (final Map.Entry<Tunnel, Config> entry : runningConfigsSnapshot.entrySet()) {
for (final Entry<Tunnel, Config> entry : runningConfigsSnapshot.entrySet()) {
setStateInternal(entry.getKey(), entry.getValue(), State.DOWN);
rewind.add(Pair.create(entry.getKey(), entry.getValue()));
}
Expand All @@ -153,7 +156,7 @@ public State setState(final Tunnel tunnel, State state, @Nullable final Config c
setStateInternal(tunnel, originalConfig, State.UP);
}
if (!multipleTunnels && originalState == State.DOWN) {
for (final Map.Entry<Tunnel, Config> entry : runningConfigsSnapshot.entrySet()) {
for (final Entry<Tunnel, Config> entry : runningConfigsSnapshot.entrySet()) {
setStateInternal(entry.getKey(), entry.getValue(), State.UP);
}
}
Expand Down
20 changes: 11 additions & 9 deletions tunnel/src/main/java/com/wireguard/android/util/RootShell.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
import java.io.OutputStreamWriter;
import java.nio.charset.StandardCharsets;
import java.util.Collection;
import java.util.Locale;
import java.util.Objects;
import java.util.UUID;

import androidx.annotation.Nullable;
Expand Down Expand Up @@ -46,16 +48,16 @@ public RootShell(final Context context) {
final String packageName = context.getPackageName();
if (packageName.contains("'"))
throw new RuntimeException("Impossibly invalid package name contains a single quote");
preamble = String.format("export CALLING_PACKAGE='%s' PATH=\"%s:$PATH\" TMPDIR='%s'; magisk --sqlite \"UPDATE policies SET notification=0, logging=0 WHERE uid=%d\" >/dev/null 2>&1; id -u\n",
preamble = String.format(Locale.getDefault(), "export CALLING_PACKAGE='%s' PATH=\"%s:$PATH\" TMPDIR='%s'; magisk --sqlite \"UPDATE policies SET notification=0, logging=0 WHERE uid=%d\" >/dev/null 2>&1; id -u\n",
packageName, localBinaryDir, localTemporaryDir, android.os.Process.myUid());
}

private static boolean isExecutableInPath(final String name) {
private static boolean isExecutableInPath() {
final String path = System.getenv("PATH");
if (path == null)
return false;
for (final String dir : path.split(":"))
if (new File(dir, name).canExecute())
if (new File(dir, RootShell.SU).canExecute())
return true;
return false;
}
Expand Down Expand Up @@ -91,17 +93,17 @@ public int run(@Nullable final Collection<String> output, final String command)
final String script = "echo " + marker + "; echo " + marker + " >&2; (" + command +
"); ret=$?; echo " + marker + " $ret; echo " + marker + " $ret >&2\n";
Log.v(TAG, "executing: " + command);
stdin.write(script);
Objects.requireNonNull(stdin).write(script);
stdin.flush();
String line;
int errnoStdout = Integer.MIN_VALUE;
int errnoStderr = Integer.MAX_VALUE;
int markersSeen = 0;
while ((line = stdout.readLine()) != null) {
while ((line = Objects.requireNonNull(stdout).readLine()) != null) {
if (line.startsWith(marker)) {
++markersSeen;
if (line.length() > marker.length() + 1) {
errnoStdout = Integer.valueOf(line.substring(marker.length() + 1));
errnoStdout = Integer.parseInt(line.substring(marker.length() + 1));
break;
}
} else if (markersSeen > 0) {
Expand All @@ -110,11 +112,11 @@ public int run(@Nullable final Collection<String> output, final String command)
Log.v(TAG, "stdout: " + line);
}
}
while ((line = stderr.readLine()) != null) {
while ((line = Objects.requireNonNull(stderr).readLine()) != null) {
if (line.startsWith(marker)) {
++markersSeen;
if (line.length() > marker.length() + 1) {
errnoStderr = Integer.valueOf(line.substring(marker.length() + 1));
errnoStderr = Integer.parseInt(line.substring(marker.length() + 1));
break;
}
} else if (markersSeen > 2) {
Expand All @@ -131,7 +133,7 @@ public int run(@Nullable final Collection<String> output, final String command)
}

public void start() throws IOException, RootShellException {
if (!isExecutableInPath(SU))
if (!isExecutableInPath())
throw new RootShellException(Reason.NO_ROOT_ACCESS);
synchronized (lock) {
if (isRunning())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ public boolean extract() throws IOException {
for (int i = 0; i < files.length; ++i) {
files[i] = new File(localBinaryDir, EXECUTABLES[i]);
tempFiles[i] = new File(localBinaryDir, EXECUTABLES[i] + ".tmp");
allExist &= files[i].exists();
allExist = allExist && files[i].exists();
}
if (allExist)
return false;
Expand Down Expand Up @@ -148,7 +148,7 @@ private int installMagisk() throws RootShellException, IOException {
script.append("printf 'id=wireguard\nname=WireGuard Command Line Tools\nversion=1.0\nversionCode=1\nauthor=zx2c4\ndescription=Command line tools for WireGuard\nminMagisk=1500\n' > /data/adb/modules/wireguard/module.prop; ");
script.append("touch /data/adb/modules/wireguard/auto_mount; ");
for (final String name : EXECUTABLES) {
final File destination = new File("/data/adb/modules/wireguard" + INSTALL_DIR, name);
final File destination = new File("/data/adb/modules/wireguard%s".formatted(INSTALL_DIR), name);
script.append(String.format("cp '%s' '%s'; chmod 755 '%s'; chcon 'u:object_r:system_file:s0' '%s' || true; ",
new File(localBinaryDir, name), destination, destination, destination));
}
Expand Down
3 changes: 1 addition & 2 deletions tunnel/src/main/java/com/wireguard/config/Config.java
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,8 @@ public static Config parse(final BufferedReader reader)

@Override
public boolean equals(final Object obj) {
if (!(obj instanceof Config))
if (!(obj instanceof final Config other))
return false;
final Config other = (Config) obj;
return interfaze.equals(other.interfaze) && peers.equals(other.peers);
}

Expand Down
7 changes: 5 additions & 2 deletions tunnel/src/main/java/com/wireguard/config/InetAddresses.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@

package com.wireguard.config;

import android.os.Build.VERSION;
import android.os.Build.VERSION_CODES;

import com.wireguard.util.NonNullForAll;

import java.lang.reflect.Method;
Expand All @@ -28,7 +31,7 @@ public final class InetAddresses {
static {
Method m = null;
try {
if (android.os.Build.VERSION.SDK_INT < android.os.Build.VERSION_CODES.Q)
if (VERSION.SDK_INT < VERSION_CODES.Q)
// noinspection JavaReflectionMemberAccess
m = InetAddress.class.getMethod("parseNumericAddress", String.class);
} catch (final Exception ignored) {
Expand Down Expand Up @@ -59,7 +62,7 @@ public static InetAddress parse(final String address) throws ParseException {
if (address.isEmpty())
throw new ParseException(InetAddress.class, address, "Empty address");
try {
if (android.os.Build.VERSION.SDK_INT >= android.os.Build.VERSION_CODES.Q)
if (VERSION.SDK_INT >= VERSION_CODES.Q)
return android.net.InetAddresses.parseNumericAddress(address);
else if (PARSER_METHOD != null)
return (InetAddress) PARSER_METHOD.invoke(null, address);
Expand Down
3 changes: 1 addition & 2 deletions tunnel/src/main/java/com/wireguard/config/InetEndpoint.java
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,8 @@ public static InetEndpoint parse(final String endpoint) throws ParseException {

@Override
public boolean equals(final Object obj) {
if (!(obj instanceof InetEndpoint))
if (!(obj instanceof final InetEndpoint other))
return false;
final InetEndpoint other = (InetEndpoint) obj;
return host.equals(other.host) && port == other.port;
}

Expand Down
3 changes: 1 addition & 2 deletions tunnel/src/main/java/com/wireguard/config/InetNetwork.java
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,8 @@ public static InetNetwork parse(final String network) throws ParseException {

@Override
public boolean equals(final Object obj) {
if (!(obj instanceof InetNetwork))
if (!(obj instanceof final InetNetwork other))
return false;
final InetNetwork other = (InetNetwork) obj;
return address.equals(other.address) && mask == other.mask;
}

Expand Down
3 changes: 1 addition & 2 deletions tunnel/src/main/java/com/wireguard/config/Interface.java
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,8 @@ public static Interface parse(final Iterable<? extends CharSequence> lines)

@Override
public boolean equals(final Object obj) {
if (!(obj instanceof Interface))
if (!(obj instanceof final Interface other))
return false;
final Interface other = (Interface) obj;
return addresses.equals(other.addresses)
&& dnsServers.equals(other.dnsServers)
&& dnsSearchDomains.equals(other.dnsSearchDomains)
Expand Down
3 changes: 1 addition & 2 deletions tunnel/src/main/java/com/wireguard/config/Peer.java
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,8 @@ public static Peer parse(final Iterable<? extends CharSequence> lines)

@Override
public boolean equals(final Object obj) {
if (!(obj instanceof Peer))
if (!(obj instanceof final Peer other))
return false;
final Peer other = (Peer) obj;
return allowedIps.equals(other.allowedIps)
&& endpoint.equals(other.endpoint)
&& persistentKeepalive.equals(other.persistentKeepalive)
Expand Down
2 changes: 1 addition & 1 deletion tunnel/src/main/java/com/wireguard/crypto/Key.java
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ public byte[] getBytes() {
public int hashCode() {
int ret = 0;
for (int i = 0; i < key.length / 4; ++i)
ret ^= (key[i * 4 + 0] >> 0) + (key[i * 4 + 1] >> 8) + (key[i * 4 + 2] >> 16) + (key[i * 4 + 3] >> 24);
ret ^= (key[i * 4] >> 0) + (key[i * 4 + 1] >> 8) + (key[i * 4 + 2] >> 16) + (key[i * 4 + 3] >> 24);
return ret;
}

Expand Down
2 changes: 2 additions & 0 deletions tunnel/tools/libwg-go/jni.c
Original file line number Diff line number Diff line change
Expand Up @@ -69,3 +69,5 @@ JNIEXPORT jstring JNICALL Java_com_wireguard_android_backend_GoBackend_wgVersion
free(version);
return ret;
}


1 change: 1 addition & 0 deletions ui/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ android {
disable += "LongLogTag"
warning += "MissingTranslation"
warning += "ImpliedQuantity"
baseline = file("lint-baseline.xml")
}
}

Expand Down
Loading

0 comments on commit 3c128d6

Please sign in to comment.