Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
8 changes: 8 additions & 0 deletions app/src/main/java/io/netbird/client/MainActivity.java
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,14 @@ public void deselectRoute(String route) throws Exception {
mBinder.deselectRoute(route);
}

@Override
public String debugBundle(boolean anonymize) throws Exception {
if (mBinder == null) {
throw new Exception("VPN service not connected");
}
return mBinder.debugBundle(anonymize);
Comment on lines +418 to +421
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Use a local binder snapshot to avoid a check-then-use race.

mBinder is read twice; it can change between the null-check and call, especially since this path is triggered off the UI thread. Capture once, then use that reference.

💡 Suggested fix
 `@Override`
 public String debugBundle(boolean anonymize) throws Exception {
-    if (mBinder == null) {
+    VPNService.MyLocalBinder binder = mBinder;
+    if (binder == null) {
         throw new Exception("VPN service not connected");
     }
-    return mBinder.debugBundle(anonymize);
+    return binder.debugBundle(anonymize);
 }
📝 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
if (mBinder == null) {
throw new Exception("VPN service not connected");
}
return mBinder.debugBundle(anonymize);
VPNService.MyLocalBinder binder = mBinder;
if (binder == null) {
throw new Exception("VPN service not connected");
}
return binder.debugBundle(anonymize);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/src/main/java/io/netbird/client/MainActivity.java` around lines 418 -
421, The code performs a check-then-use on the field mBinder causing a potential
race; capture a local snapshot (e.g., BinderType binder = mBinder) before the
null check and then use that local variable when calling debugBundle(anonymize)
in MainActivity to avoid mBinder changing between the null check and the call;
if the local snapshot is null, throw the same Exception("VPN service not
connected") so behavior is unchanged.

}

@Override
public void addRouteChangeListener(RouteChangeListener listener) {
if (mBinder == null) {
Expand Down
2 changes: 2 additions & 0 deletions app/src/main/java/io/netbird/client/ServiceAccessor.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,6 @@ public interface ServiceAccessor {

void addRouteChangeListener(RouteChangeListener listener);
void removeRouteChangeListener(RouteChangeListener listener);

String debugBundle(boolean anonymize) throws Exception;
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package io.netbird.client.ui.advanced;

import android.app.Activity;
import android.content.Context;
import android.content.SharedPreferences;
import android.os.Bundle;
Expand All @@ -20,7 +19,6 @@
import io.netbird.client.R;
import io.netbird.client.databinding.ComponentSwitchBinding;
import io.netbird.client.databinding.FragmentAdvancedBinding;
import io.netbird.client.tool.Logcat;
import io.netbird.client.tool.Preferences;
import io.netbird.client.tool.ProfileManagerWrapper;

Expand Down Expand Up @@ -103,28 +101,7 @@ public View onCreateView(@NonNull LayoutInflater inflater,
setPreSharedKey(presharedKey, inflater.getContext());
});

// Enable trace logs
Preferences preferences = new Preferences(inflater.getContext());
binding.switchTraceLog.setChecked(preferences.isTraceLogEnabled());

// Handle trace log switch toggle
binding.switchTraceLog.setOnCheckedChangeListener((buttonView, isChecked) -> {
if (isChecked) {
preferences.enableTraceLog();
} else {
preferences.disableTraceLog();
}
});

// Make parent layout clickable to toggle switch (for TV remote)
binding.traceLogLayout.setOnClickListener(v -> {
binding.switchTraceLog.toggle();
});

// Handle "Share Logs" button click
binding.buttonShareLogs.setOnClickListener(v -> {
shareLog();
});

// Rosenpass settings
try {
Expand Down Expand Up @@ -362,17 +339,4 @@ private boolean hasPreSharedKey(Context context) {
}
}

private void shareLog() {
Activity activity = getActivity();
if (activity == null) {
return;
}

try {
Logcat logcat = new Logcat(activity);
logcat.dump();
} catch (Exception e) {
Log.e(LOGTAG, "failed to dump log", e);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
package io.netbird.client.ui.troubleshoot;

import android.app.Activity;
import android.content.ClipData;
import android.content.ClipboardManager;
import android.content.Context;
import android.os.Bundle;
import android.util.Log;
import android.view.LayoutInflater;
import android.view.View;
import android.view.ViewGroup;
import android.widget.Toast;

import androidx.annotation.NonNull;
import androidx.fragment.app.Fragment;

import io.netbird.client.R;
import io.netbird.client.ServiceAccessor;
import io.netbird.client.databinding.FragmentTroubleshootBinding;
import io.netbird.client.tool.Preferences;

public class TroubleshootFragment extends Fragment {

private static final String LOGTAG = "TroubleshootFragment";
private FragmentTroubleshootBinding binding;

@Override
public View onCreateView(@NonNull LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) {
binding = FragmentTroubleshootBinding.inflate(inflater, container, false);

Preferences preferences = new Preferences(inflater.getContext());
binding.switchTraceLog.setChecked(preferences.isTraceLogEnabled());

binding.switchTraceLog.setOnCheckedChangeListener((buttonView, isChecked) -> {
if (isChecked) {
preferences.enableTraceLog();
} else {
preferences.disableTraceLog();
}
});

binding.traceLogLayout.setOnClickListener(v -> {
binding.switchTraceLog.toggle();
});

binding.anonymizeLayout.setOnClickListener(v -> {
binding.switchAnonymize.toggle();
});

binding.buttonDebugBundle.setOnClickListener(v -> {
generateDebugBundle();
});

return binding.getRoot();
}

@Override
public void onDestroyView() {
super.onDestroyView();
binding = null;
}

private void generateDebugBundle() {
Activity activity = getActivity();
if (activity == null || !(activity instanceof ServiceAccessor)) {
return;
}

boolean anonymize = binding.switchAnonymize.isChecked();
binding.buttonDebugBundle.setEnabled(false);
new Thread(() -> {
try {
String key = ((ServiceAccessor) activity).debugBundle(anonymize);
activity.runOnUiThread(() -> {
binding.buttonDebugBundle.setEnabled(true);
ClipboardManager clipboard = (ClipboardManager) activity.getSystemService(Context.CLIPBOARD_SERVICE);
ClipData clip = ClipData.newPlainText("Debug bundle key", key);
clipboard.setPrimaryClip(clip);
Toast.makeText(activity, "Debug bundle key copied to clipboard", Toast.LENGTH_SHORT).show();
});
} catch (Exception e) {
Log.e(LOGTAG, "failed to create debug bundle", e);
activity.runOnUiThread(() -> {
binding.buttonDebugBundle.setEnabled(true);
Toast.makeText(activity, "Failed to create debug bundle: " + e.getMessage(), Toast.LENGTH_LONG).show();
});
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}).start();
}
}
11 changes: 11 additions & 0 deletions app/src/main/res/drawable/ic_menu_troubleshoot.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<vector xmlns:android="http://schemas.android.com/apk/res/android"
android:width="24dp"
android:height="24dp"
android:viewportWidth="24"
android:viewportHeight="24"
android:tint="?attr/colorControlNormal">
<!-- Material Design "bug_report" outlined icon -->
<path
android:fillColor="#000000"
android:pathData="M20,8h-2.81c-0.45,-0.78 -1.07,-1.45 -1.82,-1.96L17,4.41 15.59,3l-2.17,2.17C12.96,5.06 12.49,5 12,5s-0.96,0.06 -1.41,0.17L8.41,3 7,4.41l1.62,1.63C7.88,6.55 7.26,7.22 6.81,8H4v2h2.09C6.04,10.33 6,10.66 6,11v1H4v2h2v1c0,0.34 0.04,0.67 0.09,1H4v2h2.81c1.04,1.79 2.97,3 5.19,3s4.15,-1.21 5.19,-3H20v-2h-2.09c0.05,-0.33 0.09,-0.66 0.09,-1v-1h2v-2h-2v-1c0,-0.34 -0.04,-0.67 -0.09,-1H20V8zM16,15c0,2.21 -1.79,4 -4,4s-4,-1.79 -4,-4v-4c0,-2.21 1.79,-4 4,-4s4,1.79 4,4v4zM10,14h4v2h-4v-2zM10,10h4v2h-4v-2z" />
</vector>
59 changes: 1 addition & 58 deletions app/src/main/res/layout/fragment_advanced.xml
Original file line number Diff line number Diff line change
Expand Up @@ -76,63 +76,6 @@
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toBottomOf="@id/preshared_key" />

<View
android:id="@+id/separator"
android:layout_width="0dp"
android:layout_height="1dp"
android:layout_marginTop="24dp"
android:background="@drawable/separator"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toBottomOf="@id/btn_save" />

<LinearLayout
android:id="@+id/trace_log_layout"
android:layout_width="0dp"
android:layout_height="wrap_content"
android:layout_marginTop="24dp"
android:background="@drawable/focus_highlight"
android:clickable="true"
android:focusable="true"
android:focusableInTouchMode="false"
android:gravity="center_vertical"
android:orientation="horizontal"
android:padding="12dp"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toBottomOf="@id/separator">

<TextView
android:id="@+id/switchLabel"
android:layout_width="0dp"
android:layout_height="wrap_content"
android:layout_weight="1"
android:text="@string/advanced_tracelog"
android:textColor="@color/nb_txt_light"
android:textSize="15sp" />

<com.google.android.material.switchmaterial.SwitchMaterial
android:id="@+id/switch_trace_log"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:focusable="false" />
</LinearLayout>

<Button
android:id="@+id/button_share_logs"
android:layout_width="0dp"
android:layout_height="wrap_content"
android:layout_marginTop="16dp"
android:background="@drawable/btn_bg_orange"
android:focusable="true"
android:focusableInTouchMode="false"
android:foreground="@drawable/focus_highlight"
android:text="@string/advanced_share_logs"
android:textAllCaps="false"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toBottomOf="@id/trace_log_layout" />

<View
android:id="@+id/separator_rosenpass"
android:layout_width="0dp"
Expand All @@ -141,7 +84,7 @@
android:background="@drawable/separator"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toBottomOf="@id/button_share_logs" />
app:layout_constraintTop_toBottomOf="@id/btn_save" />

<LinearLayout
android:id="@+id/layout_rosenpas"
Expand Down
102 changes: 102 additions & 0 deletions app/src/main/res/layout/fragment_troubleshoot.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
<?xml version="1.0" encoding="utf-8"?>
<ScrollView xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:app="http://schemas.android.com/apk/res-auto"
xmlns:tools="http://schemas.android.com/tools"
android:layout_width="match_parent"
android:layout_height="match_parent"
android:fillViewport="true"
tools:context=".ui.troubleshoot.TroubleshootFragment">

<LinearLayout
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:gravity="center_horizontal">

<androidx.constraintlayout.widget.ConstraintLayout
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:maxWidth="@dimen/fragment_max_width"
android:padding="40dp">

<LinearLayout
android:id="@+id/trace_log_layout"
android:layout_width="0dp"
android:layout_height="wrap_content"
android:background="@drawable/focus_highlight"
android:clickable="true"
android:focusable="true"
android:focusableInTouchMode="false"
android:gravity="center_vertical"
android:orientation="horizontal"
android:padding="12dp"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent">

<TextView
android:id="@+id/switchLabel"
android:layout_width="0dp"
android:layout_height="wrap_content"
android:layout_weight="1"
android:text="@string/advanced_tracelog"
android:textColor="@color/nb_txt_light"
android:textSize="15sp" />

<com.google.android.material.switchmaterial.SwitchMaterial
android:id="@+id/switch_trace_log"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:focusable="false" />
</LinearLayout>

<LinearLayout
android:id="@+id/anonymize_layout"
android:layout_width="0dp"
android:layout_height="wrap_content"
android:layout_marginTop="16dp"
android:background="@drawable/focus_highlight"
android:clickable="true"
android:focusable="true"
android:focusableInTouchMode="false"
android:gravity="center_vertical"
android:orientation="horizontal"
android:padding="12dp"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toBottomOf="@id/trace_log_layout">

<TextView
android:layout_width="0dp"
android:layout_height="wrap_content"
android:layout_weight="1"
android:text="@string/troubleshoot_anonymize"
android:textColor="@color/nb_txt_light"
android:textSize="15sp" />

<com.google.android.material.switchmaterial.SwitchMaterial
android:id="@+id/switch_anonymize"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:focusable="false" />
Comment thread
pappz marked this conversation as resolved.
</LinearLayout>

<Button
android:id="@+id/button_debug_bundle"
android:layout_width="0dp"
android:layout_height="wrap_content"
android:layout_marginTop="24dp"
android:background="@drawable/btn_bg_orange"
android:focusable="true"
android:focusableInTouchMode="false"
android:foreground="@drawable/focus_highlight"
android:text="@string/troubleshoot_debug_bundle"
android:textAllCaps="false"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toBottomOf="@id/anonymize_layout" />

</androidx.constraintlayout.widget.ConstraintLayout>

</LinearLayout>

</ScrollView>
16 changes: 10 additions & 6 deletions app/src/main/res/menu/activity_main_drawer.xml
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,20 @@
android:icon="@drawable/ic_menu_advanced"
android:title="@string/menu_advanced" />
<item
android:id="@+id/nav_about"
android:icon="@drawable/ic_menu_about"
android:title="@string/menu_about" />
android:id="@+id/nav_change_server"
android:icon="@drawable/ic_menu_change_server"
android:title="@string/menu_change_server" />
<item
android:id="@+id/nav_docs"
android:icon="@drawable/ic_menu_docs"
android:title="@string/menu_docs" />
<item
android:id="@+id/nav_change_server"
android:icon="@drawable/ic_menu_change_server"
android:title="@string/menu_change_server" />
android:id="@+id/nav_troubleshoot"
android:icon="@drawable/ic_menu_troubleshoot"
android:title="@string/menu_troubleshoot" />
<item
android:id="@+id/nav_about"
android:icon="@drawable/ic_menu_about"
android:title="@string/menu_about" />
</group>
</menu>
Loading
Loading