Skip to content

Commit dd900c4

Browse files
fmaginfmagin
committed
Fix NPE when config is not available yet and add tests for initial setup
This also fixes a few other issues that were hidden by the initial NPE.
1 parent 6a0d6ed commit dd900c4

13 files changed

Lines changed: 447 additions & 120 deletions

File tree

‎src/main/java/ai/reveng/toolkit/ghidra/binarysimilarity/ui/collectiondialog/BinaryTableModel.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,12 @@ public BinaryTableModel(PluginTool tool) {
2121

2222
@Override
2323
protected void doLoad(Accumulator<BinaryRowObject> accumulator, TaskMonitor monitor) throws CancelledException {
24-
serviceProvider.getService(GhidraRevengService.class)
25-
.getActiveAnalysisIDsFilter()
24+
var service = serviceProvider.getService(GhidraRevengService.class);
25+
if (service == null) return;
26+
service.getActiveAnalysisIDsFilter()
2627
.forEach(
2728
analysisResult -> accumulator.add(new BinaryRowObject(analysisResult, true))
2829
);
29-
3030
}
3131

3232
@Override

‎src/main/java/ai/reveng/toolkit/ghidra/binarysimilarity/ui/collectiondialog/CollectionTableModel.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,13 @@ public CollectionTableModel(PluginTool plugin) {
2626

2727
@Override
2828
protected void doLoad(Accumulator<CollectionRowObject> accumulator, TaskMonitor monitor){
29+
var service = serviceProvider.getService(GhidraRevengService.class);
30+
if (service == null) return;
2931
monitor.setProgress(0);
3032
monitor.setMessage("Loading collections");
31-
serviceProvider.getService(GhidraRevengService.class).getActiveCollections().forEach(collection -> {
33+
service.getActiveCollections().forEach(collection -> {
3234
accumulator.add(new CollectionRowObject(collection, true));
3335
});
34-
3536
}
3637

3738
@Override

‎src/main/java/ai/reveng/toolkit/ghidra/core/services/api/types/ApiInfo.java‎

Lines changed: 8 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,13 @@
11
package ai.reveng.toolkit.ghidra.core.services.api.types;
22

33
import ai.reveng.toolkit.ghidra.core.models.ReaiConfig;
4-
import ai.reveng.toolkit.ghidra.core.services.api.TypedApiImplementation;
5-
import ai.reveng.toolkit.ghidra.core.services.api.types.exceptions.InvalidAPIInfoException;
64
import com.google.gson.Gson;
75
import org.json.JSONException;
86

97
import java.io.FileNotFoundException;
108
import java.io.FileReader;
119
import java.net.URI;
1210
import java.nio.file.Path;
13-
import java.nio.file.Paths;
1411

1512
public record ApiInfo(
1613
URI hostURI,
@@ -21,21 +18,15 @@ public ApiInfo(String hostURI, String portalURI, String apiKey) {
2118
this(URI.create(hostURI), URI.create(portalURI), apiKey);
2219
}
2320

24-
public void checkCredentials() throws InvalidAPIInfoException {
25-
if (hostURI == null || apiKey == null){
26-
throw new InvalidAPIInfoException("hostURI and apiKey must not be null");
27-
}
28-
var api = new TypedApiImplementation(this);
29-
30-
// Throws InvalidAPIInfoException if authentication fails
31-
api.authenticate();
32-
}
33-
3421
public static ApiInfo fromConfig(Path configFilePath) throws FileNotFoundException {
35-
// Read and parse the config file as JSON
36-
FileReader reader = new FileReader(configFilePath.toString());
37-
Gson gson = new Gson();
38-
ReaiConfig config = gson.fromJson(reader, ReaiConfig.class);
22+
ReaiConfig config;
23+
try (FileReader reader = new FileReader(configFilePath.toString())) {
24+
config = new Gson().fromJson(reader, ReaiConfig.class);
25+
} catch (FileNotFoundException e) {
26+
throw e;
27+
} catch (java.io.IOException e) {
28+
throw new RuntimeException("Failed to read config file: " + configFilePath, e);
29+
}
3930
var apikey = config.getPluginSettings().getApiKey();
4031
var hostname = config.getPluginSettings().getHostname();
4132
var portalHostname = config.getPluginSettings().getPortalHostname();
@@ -46,15 +37,4 @@ public static ApiInfo fromConfig(Path configFilePath) throws FileNotFoundExcepti
4637
}
4738
return new ApiInfo(hostname, portalHostname, apikey);
4839
}
49-
50-
public static ApiInfo fromConfig() throws FileNotFoundException {
51-
String uHome = System.getProperty("user.home");
52-
String cDir = ".reai";
53-
String cFileName = "reai.json";
54-
Path configDirPath = Paths.get(uHome, cDir);
55-
Path configFilePath = configDirPath.resolve(cFileName);
56-
57-
return fromConfig(configFilePath);
58-
59-
}
6040
}

‎src/main/java/ai/reveng/toolkit/ghidra/core/ui/wizard/CredentialValidator.java‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
package ai.reveng.toolkit.ghidra.core.ui.wizard;
2+
3+
import ai.reveng.toolkit.ghidra.core.services.api.TypedApiImplementation;
4+
import ai.reveng.toolkit.ghidra.core.services.api.types.ApiInfo;
5+
import ai.reveng.toolkit.ghidra.core.services.api.types.exceptions.InvalidAPIInfoException;
6+
7+
/**
8+
* Strategy for validating API credentials.
9+
* Production code uses {@link #defaultValidator()} which makes a real API call;
10+
* tests can supply a mock that avoids network calls.
11+
*/
12+
@FunctionalInterface
13+
public interface CredentialValidator {
14+
void validate(ApiInfo apiInfo) throws InvalidAPIInfoException;
15+
16+
/** Default validator that makes a real API call. */
17+
static CredentialValidator defaultValidator() {
18+
return apiInfo -> {
19+
if (apiInfo.hostURI() == null || apiInfo.apiKey() == null) {
20+
throw new InvalidAPIInfoException("hostURI and apiKey must not be null");
21+
}
22+
var api = new TypedApiImplementation(apiInfo);
23+
api.authenticate();
24+
};
25+
}
26+
}

‎src/main/java/ai/reveng/toolkit/ghidra/core/ui/wizard/SetupWizardManager.java‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
import java.io.IOException;
55
import java.nio.file.Files;
66
import java.nio.file.Path;
7-
import java.nio.file.Paths;
87
import java.util.ArrayList;
98
import java.util.List;
109

@@ -55,11 +54,11 @@ protected void doFinish() throws IllegalPanelStateException {
5554
tool.getOptions(REAI_OPTIONS_CATEGORY).setString(ReaiPluginPackage.OPTION_KEY_MODEL, model);
5655
tool.getOptions(REAI_OPTIONS_CATEGORY).setString(REAI_WIZARD_RUN_PREF, "true");
5756

58-
String uHome = System.getProperty("user.home");
59-
String cDir = ".reai";
60-
String cFileName = "reai.json";
61-
Path configDirPath = Paths.get(uHome, cDir);
62-
Path configFilePath = configDirPath.resolve(cFileName);
57+
String configFileOverride = (String) getState().get(SetupWizardStateKey.CONFIGFILE);
58+
Path configFilePath = configFileOverride != null
59+
? Path.of(configFileOverride)
60+
: ReaiPluginPackage.DEFAULT_CONFIG_PATH;
61+
Path configDirPath = configFilePath.getParent();
6362

6463
// check that our .reai directory exists
6564
if (!Files.exists(configDirPath)) {

‎src/main/java/ai/reveng/toolkit/ghidra/core/ui/wizard/SetupWizardStateKey.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,5 @@ public enum SetupWizardStateKey {
77
CREDENTIALS_VALIDATED,
88
MODEL,
99
CONFIGFILE,
10+
CREDENTIAL_VALIDATOR,
1011
}

‎src/main/java/ai/reveng/toolkit/ghidra/core/ui/wizard/panels/UserCredentialsPanel.java‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import ai.reveng.toolkit.ghidra.core.services.api.types.ApiInfo;
66
import ai.reveng.toolkit.ghidra.core.services.api.types.exceptions.InvalidAPIInfoException;
77
import ai.reveng.toolkit.ghidra.core.services.logging.ReaiLoggingService;
8+
import ai.reveng.toolkit.ghidra.core.ui.wizard.CredentialValidator;
89
import ai.reveng.toolkit.ghidra.core.ui.wizard.SetupWizardStateKey;
910
import docking.wizard.AbstractMageJPanel;
1011
import docking.wizard.IllegalPanelStateException;
@@ -26,6 +27,7 @@ public class UserCredentialsPanel extends AbstractMageJPanel<SetupWizardStateKey
2627
private JTextField tfApiHostname;
2728
private JTextField tfPortalHostname;
2829
private Boolean credentialsValidated = false;
30+
private CredentialValidator credentialValidator = CredentialValidator.defaultValidator();
2931

3032
private ReaiLoggingService loggingService;
3133

@@ -73,6 +75,7 @@ public void changedUpdate(DocumentEvent e) {
7375
gbc.fill = GridBagConstraints.HORIZONTAL;
7476
gbc.weightx = 1.0;
7577
tfApiKey = new JTextField(30);
78+
tfApiKey.setName("apiKey");
7679
tfApiKey.getDocument().addDocumentListener(documentListener);
7780
tfApiKey.setToolTipText("API key from your account settings");
7881
userDetailsPanel.add(tfApiKey, gbc);
@@ -89,6 +92,7 @@ public void changedUpdate(DocumentEvent e) {
8992
gbc.fill = GridBagConstraints.HORIZONTAL;
9093
gbc.weightx = 1.0;
9194
tfApiHostname = new JTextField(30);
95+
tfApiHostname.setName("apiHostname");
9296
tfApiHostname.getDocument().addDocumentListener(documentListener);
9397
tfApiHostname.setToolTipText("URL hosting the RevEng.AI Server");
9498
tfApiHostname.setText("https://api.reveng.ai");
@@ -106,6 +110,7 @@ public void changedUpdate(DocumentEvent e) {
106110
gbc.fill = GridBagConstraints.HORIZONTAL;
107111
gbc.weightx = 1.0;
108112
tfPortalHostname = new JTextField(30);
113+
tfPortalHostname.setName("portalHostname");
109114
tfPortalHostname.getDocument().addDocumentListener(documentListener);
110115
tfPortalHostname.setToolTipText("URL hosting the RevEng.AI Portal");
111116
tfPortalHostname.setText("https://portal.reveng.ai");
@@ -158,16 +163,13 @@ public void mouseClicked(java.awt.event.MouseEvent e) {
158163
runTestsButton.addActionListener(e -> {
159164
var apiInfo = new ApiInfo(tfApiHostname.getText(), tfPortalHostname.getText(), tfApiKey.getText());
160165
try {
161-
apiInfo.checkCredentials();
166+
credentialValidator.validate(apiInfo);
162167
credentialsValidated = true;
163-
// TODO: Get the user for this key once the API exists
164168
notifyListenersOfValidityChanged();
165-
166169
} catch (InvalidAPIInfoException ex) {
167170
credentialsValidated = false;
168171
notifyListenersOfStatusMessage("Problem with user info:\n" + ex.getMessage());
169172
}
170-
171173
});
172174
userDetailsPanel.add(runTestsButton, gbc);
173175
}
@@ -186,6 +188,12 @@ public WizardPanelDisplayability getPanelDisplayabilityAndUpdateState(WizardStat
186188

187189
@Override
188190
public void enterPanel(WizardState<SetupWizardStateKey> state) throws IllegalPanelStateException {
191+
// Pick up credential validator from state if provided (e.g. by tests)
192+
CredentialValidator validator = (CredentialValidator) state.get(SetupWizardStateKey.CREDENTIAL_VALIDATOR);
193+
if (validator != null) {
194+
this.credentialValidator = validator;
195+
}
196+
189197
// Populate fields with existing state information if present
190198
String existingApiKey = (String) state.get(SetupWizardStateKey.API_KEY);
191199
String existingHostname = (String) state.get(SetupWizardStateKey.HOSTNAME);
@@ -213,11 +221,6 @@ public void enterPanel(WizardState<SetupWizardStateKey> state) throws IllegalPan
213221
}
214222
}
215223

216-
private void validateCredentialsFromState() {
217-
// This method is no longer used - validation state is preserved from wizard state
218-
// Keeping for backwards compatibility but it should not be called
219-
loggingService.warn("validateCredentialsFromState() called - this should not happen with new validation logic");
220-
}
221224

222225
@Override
223226
public void leavePanel(WizardState<SetupWizardStateKey> state) {

‎src/main/java/ai/reveng/toolkit/ghidra/plugins/BinarySimilarityPlugin.java‎

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
66
* You may obtain a copy of the License at
7-
*
7+
*
88
* http://www.apache.org/licenses/LICENSE-2.0
9-
*
9+
*
1010
* Unless required by applicable law or agreed to in writing, software
1111
* distributed under the License is distributed on an "AS IS" BASIS,
1212
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -64,10 +64,11 @@ public class BinarySimilarityPlugin extends ProgramPlugin {
6464
private final AIDecompilationdWindow decompiledWindow;
6565
private final SimilarFunctionsWindow similarFunctionsWindow;
6666

67-
private GhidraRevengService apiService;
68-
6967
public final static String REVENG_AI_NAMESPACE = "RevEng.AI";
7068

69+
private GhidraRevengService getApiService() {
70+
return tool.getService(GhidraRevengService.class);
71+
}
7172

7273
@Override
7374
protected void locationChanged(ProgramLocation loc) {
@@ -78,6 +79,11 @@ protected void locationChanged(ProgramLocation loc) {
7879
return;
7980
}
8081

82+
var apiService = getApiService();
83+
if (apiService == null) {
84+
return;
85+
}
86+
8187
// If no program, or not attached to a complete analysis, do not trigger location change events
8288
var program = loc.getProgram();
8389
if (program == null || apiService.getAnalysedProgram(program).isEmpty()) {
@@ -90,7 +96,7 @@ protected void locationChanged(ProgramLocation loc) {
9096

9197
/**
9298
* Plugin constructor.
93-
*
99+
*
94100
* @param tool The plugin tool that this plugin is added to.
95101
*/
96102
public BinarySimilarityPlugin(PluginTool tool) {
@@ -110,19 +116,13 @@ public BinarySimilarityPlugin(PluginTool tool) {
110116
collectionsComponent.addToTool();
111117
}
112118

113-
/// In `init()` the services are guaranteed to be available
114-
@Override
115-
public void init() {
116-
super.init();
117-
118-
apiService = tool.getService(GhidraRevengService.class);
119-
}
120-
121119
private void setupActions() {
122120
new ActionBuilder("Auto Unstrip", this.getName())
123121
.menuGroup(ReaiPluginPackage.NAME)
124122
.menuPath(ReaiPluginPackage.MENU_GROUP_NAME, "Auto Unstrip")
125123
.enabledWhen(context -> {
124+
var apiService = getApiService();
125+
if (apiService == null) return false;
126126
var program = tool.getService(ProgramManager.class).getCurrentProgram();
127127
if (program != null) {
128128
return apiService.getKnownProgram(program).isPresent();
@@ -132,6 +132,7 @@ private void setupActions() {
132132
}
133133
)
134134
.onAction(context -> {
135+
var apiService = getApiService();
135136
var program = tool.getService(ProgramManager.class).getCurrentProgram();
136137
if (apiService.getAnalysedProgram(program).isEmpty()) {
137138
Msg.showError(this, null, ReaiPluginPackage.WINDOW_PREFIX + "Auto Unstrip",
@@ -155,6 +156,8 @@ private void setupActions() {
155156
.menuGroup(ReaiPluginPackage.NAME)
156157
.menuPath(ReaiPluginPackage.MENU_GROUP_NAME, "Function Matching")
157158
.enabledWhen(context -> {
159+
var apiService = getApiService();
160+
if (apiService == null) return false;
158161
var program = tool.getService(ProgramManager.class).getCurrentProgram();
159162
if (program != null) {
160163
return apiService.getAnalysedProgram(program).isPresent();
@@ -164,6 +167,7 @@ private void setupActions() {
164167
}
165168
)
166169
.onAction(context -> {
170+
var apiService = getApiService();
167171
var program = tool.getService(ProgramManager.class).getCurrentProgram();
168172
var knownProgram = apiService.getAnalysedProgram(program);
169173
if (knownProgram.isEmpty()){
@@ -181,6 +185,8 @@ private void setupActions() {
181185
new ActionBuilder("Match function", this.getName())
182186
.withContext(ProgramLocationActionContext.class)
183187
.enabledWhen(context -> {
188+
var apiService = getApiService();
189+
if (apiService == null) return false;
184190
var func = context.getProgram().getFunctionManager().getFunctionContaining(context.getAddress());
185191
return func != null
186192
// Exclude thunks and external functions because we do not support them in the portal
@@ -189,6 +195,7 @@ private void setupActions() {
189195
&& apiService.getAnalysedProgram(context.getProgram()).isPresent();
190196
})
191197
.onAction(context -> {
198+
var apiService = getApiService();
192199
// We know analysed program is present due to enabledWhen
193200
var knownProgram = apiService.getAnalysedProgram(context.getProgram()).get();
194201

@@ -206,6 +213,8 @@ private void setupActions() {
206213
new ActionBuilder("AI Decompilation", this.getName())
207214
.withContext(ProgramLocationActionContext.class)
208215
.enabledWhen(context -> {
216+
var apiService = getApiService();
217+
if (apiService == null) return false;
209218
var func = context.getProgram().getFunctionManager().getFunctionContaining(context.getAddress());
210219
return func != null
211220
// Exclude thunks and external functions because we do not support them in the portal
@@ -214,6 +223,7 @@ private void setupActions() {
214223
&& apiService.getAnalysedProgram(context.getProgram()).isPresent();
215224
})
216225
.onAction(context -> {
226+
var apiService = getApiService();
217227
var func = context.getProgram().getFunctionManager().getFunctionContaining(context.getAddress());
218228
var analysedProgram = apiService.getAnalysedProgram(context.getProgram()).get();
219229
var functionWithId = analysedProgram.getIDForFunction(func);
@@ -237,11 +247,14 @@ private void setupActions() {
237247
new ActionBuilder("View function in portal", this.getName())
238248
.withContext(ProgramLocationActionContext.class)
239249
.enabledWhen(context -> {
250+
var apiService = getApiService();
251+
if (apiService == null) return false;
240252
var func = context.getProgram().getFunctionManager().getFunctionContaining(context.getAddress());
241253
return func != null
242254
&& apiService.getAnalysedProgram(context.getProgram()).isPresent();
243255
})
244256
.onAction(context -> {
257+
var apiService = getApiService();
245258
var func = context.getProgram().getFunctionManager().getFunctionContaining(context.getAddress());
246259
var analysedProgram = apiService.getAnalysedProgram(context.getProgram()).get();
247260
var functionWithID = analysedProgram.getIDForFunction(func);
@@ -263,6 +276,8 @@ private void setupActions() {
263276

264277
@Override
265278
public void readDataState(SaveState saveState) {
279+
var apiService = getApiService();
280+
if (apiService == null) return;
266281
int[] rawCollectionIDs = saveState.getInts("collectionIDs", new int[0]);
267282
var restoredCollections = Arrays.stream(rawCollectionIDs)
268283
.mapToObj(TypedApiInterface.CollectionID::new)
@@ -273,6 +288,8 @@ public void readDataState(SaveState saveState) {
273288

274289
@Override
275290
public void writeDataState(SaveState saveState) {
291+
var apiService = getApiService();
292+
if (apiService == null) return;
276293
int[] collectionIDs = apiService.getActiveCollections().stream().map(Collection::collectionID).mapToInt(TypedApiInterface.CollectionID::id).toArray();
277294
saveState.putInts("collectionIDs", collectionIDs);
278295
}

0 commit comments

Comments
 (0)