Skip to content

Commit 436b824

Browse files
committed
prevent NPE when saving or loading credentials with missing optional fields
1 parent 859cc4f commit 436b824

2 files changed

Lines changed: 117 additions & 18 deletions

File tree

src/main/java/org/sap/cytoscape/internal/tasks/CyConnectTaskTunables.java

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -105,20 +105,20 @@ public CyConnectTaskTunables(){
105105

106106
Properties props = IOUtils.loadProperties();
107107

108-
this.host = props.getProperty("hdb.host");
109-
this.port = props.getProperty("hdb.port");
110-
this.advancedProperties = props.getProperty("hdb.advancedproperties");
111-
this.username = props.getProperty("hdb.username");
112-
this.password = props.getProperty("hdb.password");
108+
this.host = props.getProperty("hdb.host", "");
109+
this.port = props.getProperty("hdb.port", "");
110+
this.advancedProperties = props.getProperty("hdb.advancedproperties", "");
111+
this.username = props.getProperty("hdb.username", "");
112+
this.password = props.getProperty("hdb.password", "");
113113

114114
this.enableProxyConfiguration = Boolean.parseBoolean(props.getProperty("hdb.proxy.enabled", "false"));
115115
if(props.getProperty("hdb.proxy.type") != null){
116116
this.proxyType.setSelectedValue(props.getProperty("hdb.proxy.type"));
117117
}
118-
this.proxyHost = props.getProperty("hdb.proxy.host");
119-
this.proxyPort = props.getProperty("hdb.proxy.port");
120-
this.proxyUsername = props.getProperty("hdb.proxy.username");
121-
this.proxyPassword = props.getProperty("hdb.proxy.password");
118+
this.proxyHost = props.getProperty("hdb.proxy.host", "");
119+
this.proxyPort = props.getProperty("hdb.proxy.port", "");
120+
this.proxyUsername = props.getProperty("hdb.proxy.username", "");
121+
this.proxyPassword = props.getProperty("hdb.proxy.password", "");
122122

123123
// assume that the user still wants to store the password, if this
124124
// has been done before
@@ -164,20 +164,20 @@ public HanaConnectionCredentials getHanaConnectionCredentials(){
164164
public void saveToCacheFile() throws IOException {
165165

166166
Properties credProps = new Properties();
167-
credProps.setProperty("hdb.host", this.host);
168-
credProps.setProperty("hdb.advancedproperties", this.advancedProperties);
169-
credProps.setProperty("hdb.port", this.port);
170-
credProps.setProperty("hdb.username", this.username);
167+
credProps.setProperty("hdb.host", this.host == null ? "" : this.host);
168+
credProps.setProperty("hdb.advancedproperties", this.advancedProperties == null ? "" : this.advancedProperties);
169+
credProps.setProperty("hdb.port", this.port == null ? "" : this.port);
170+
credProps.setProperty("hdb.username", this.username == null ? "" : this.username);
171171

172172
credProps.setProperty("hdb.proxy.enabled", String.valueOf(this.enableProxyConfiguration));
173173
credProps.setProperty("hdb.proxy.type", this.proxyType.getSelectedValue());
174-
credProps.setProperty("hdb.proxy.host", this.proxyHost);
175-
credProps.setProperty("hdb.proxy.port", this.proxyPort);
176-
credProps.setProperty("hdb.proxy.username", this.proxyUsername);
174+
credProps.setProperty("hdb.proxy.host", this.proxyHost == null ? "" : this.proxyHost);
175+
credProps.setProperty("hdb.proxy.port", this.proxyPort == null ? "" : this.proxyPort);
176+
credProps.setProperty("hdb.proxy.username", this.proxyUsername == null ? "" : this.proxyUsername);
177177

178178
if (savePassword) {
179-
credProps.setProperty("hdb.password", this.password);
180-
credProps.setProperty("hdb.proxy.password", this.proxyPassword);
179+
credProps.setProperty("hdb.password", this.password == null ? "" : this.password);
180+
credProps.setProperty("hdb.proxy.password", this.proxyPassword == null ? "" : this.proxyPassword);
181181
} else {
182182
// overwrite previously saved passwords
183183
credProps.setProperty("hdb.password", "");
Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
import org.junit.After;
2+
import org.junit.Assert;
3+
import org.junit.Before;
4+
import org.junit.Test;
5+
import org.sap.cytoscape.internal.tasks.CyConnectTaskTunables;
6+
import org.sap.cytoscape.internal.utils.IOUtils;
7+
8+
import java.io.File;
9+
import java.io.IOException;
10+
import java.nio.file.Files;
11+
import java.util.Properties;
12+
13+
public class CyConnectTaskTunablesTest {
14+
15+
private String originalUserHome;
16+
private File tempHome;
17+
18+
@Before
19+
public void setUp() throws IOException {
20+
originalUserHome = System.getProperty("user.home");
21+
tempHome = Files.createTempDirectory("saphana_test_home").toFile();
22+
System.setProperty("user.home", tempHome.getAbsolutePath());
23+
}
24+
25+
@After
26+
public void tearDown() {
27+
System.setProperty("user.home", originalUserHome);
28+
deleteRecursively(tempHome);
29+
}
30+
31+
private void deleteRecursively(File f) {
32+
if (f.isDirectory()) {
33+
for (File child : f.listFiles()) deleteRecursively(child);
34+
}
35+
f.delete();
36+
}
37+
38+
/**
39+
* When the cache file exists but required keys (host, port, username) are absent,
40+
* the constructor must leave those fields as "" rather than null, so that
41+
* saveToCacheFile() does not NPE when it calls Properties.setProperty(key, null).
42+
*/
43+
@Test
44+
public void testConstructor_requiredKeysAbsentInCacheDoNotLeaveNullFields() throws IOException {
45+
// Write a completely empty cache file — all keys absent
46+
IOUtils.cacheProperties(new Properties());
47+
48+
CyConnectTaskTunables tunables = new CyConnectTaskTunables();
49+
50+
Assert.assertNotNull("host must not be null when key is absent from cache", tunables.host);
51+
Assert.assertNotNull("port must not be null when key is absent from cache", tunables.port);
52+
Assert.assertNotNull("username must not be null when key is absent from cache", tunables.username);
53+
}
54+
55+
/**
56+
* When the cache file exists but the hdb.password key is absent (e.g. written by an
57+
* older version of the plug-in), the constructor must not silently crash and must
58+
* leave the password field as "" rather than null.
59+
*/
60+
@Test
61+
public void testConstructor_passwordKeyAbsentInCacheDoesNotLeaveNullPassword() throws IOException {
62+
Properties cachedProps = new Properties();
63+
cachedProps.setProperty("hdb.host", "myhost.hana.cloud");
64+
cachedProps.setProperty("hdb.port", "443");
65+
cachedProps.setProperty("hdb.username", "myuser");
66+
// hdb.password key intentionally absent — simulates a cache written by an older version
67+
IOUtils.cacheProperties(cachedProps);
68+
69+
CyConnectTaskTunables tunables = new CyConnectTaskTunables();
70+
71+
Assert.assertEquals("myhost.hana.cloud", tunables.host);
72+
Assert.assertEquals("myuser", tunables.username);
73+
Assert.assertNotNull("password must not be null when hdb.password key is absent from cache", tunables.password);
74+
}
75+
76+
/**
77+
* saveToCacheFile() must not throw when optional fields (advancedProperties, proxy
78+
* host/port/username/password) are null. This is the normal case for a user who does
79+
* not configure a proxy or advanced JDBC properties.
80+
*/
81+
@Test
82+
public void testSaveToCacheFile_doesNotThrowWhenOptionalFieldsAreNull() throws IOException {
83+
CyConnectTaskTunables tunables = new CyConnectTaskTunables();
84+
// Simulate a user who filled in only the required fields
85+
tunables.host = "myhost.hana.cloud";
86+
tunables.port = "443";
87+
tunables.username = "myuser";
88+
tunables.password = "mypassword";
89+
tunables.savePassword = true;
90+
// advancedProperties, proxyHost, proxyPort, proxyUsername, proxyPassword left as null
91+
92+
tunables.saveToCacheFile();
93+
94+
// Verify the file was actually written and can be read back
95+
Properties loaded = IOUtils.loadProperties();
96+
Assert.assertEquals("myhost.hana.cloud", loaded.getProperty("hdb.host"));
97+
Assert.assertEquals("mypassword", loaded.getProperty("hdb.password"));
98+
}
99+
}

0 commit comments

Comments
 (0)