Skip to content

Commit dc4937b

Browse files
ulbidarkscout
ulbi
authored and
darkscout
committed
[timescaledb] Fix TimescaleDB JDBC URL validation (openhab#20689)
* [timescaledb] Fix for openhab#20676 TimescaleDB JDBC URL validation Signed-off-by: René Ulbricht <rene_ulbricht@outlook.com>
1 parent 4e8dd4a commit dc4937b

6 files changed

Lines changed: 99 additions & 4 deletions

File tree

‎bundles/org.openhab.persistence.timescaledb/README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ Configure via `$OPENHAB_CONF/services/timescaledb.cfg` or in the UI under `Setti
4949

5050
| Property | Default | Required | Description |
5151
|------------------------|-----------|:--------:|-----------------------------------------------------------|
52-
| `url` | | Yes | JDBC URL, e.g. `jdbc:postgresql://localhost:5432/openhab` |
52+
| `url` | | Yes | Database URL, e.g. `jdbc:postgresql://localhost:5432/openhab` or `postgresql://localhost:5432/openhab` (`jdbc:` is added automatically if missing) |
5353
| `user` | `openhab` | No | Database user |
5454
| `password` | | Yes | Database password |
5555
| `chunkInterval` | `7 days` | No | TimescaleDB chunk interval for the hypertable |

‎bundles/org.openhab.persistence.timescaledb/src/main/java/org/openhab/persistence/timescaledb/internal/TimescaleDBPersistenceService.java‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ public TimescaleDBPersistenceService(final @Reference ItemRegistry itemRegistry,
114114

115115
@Activate
116116
public void activate(final Map<String, Object> config) {
117-
String url = (String) config.getOrDefault("url", "");
117+
String url = normalizeJdbcUrl((String) config.getOrDefault("url", ""));
118118
if (url.isBlank()) {
119119
LOGGER.warn("TimescaleDB persistence not configured: missing 'url'. "
120120
+ "Configure org.openhab.timescaledb:url.");
@@ -447,6 +447,14 @@ private static HikariDataSource createDataSource(String url, String user, String
447447
return new HikariDataSource(cfg);
448448
}
449449

450+
static String normalizeJdbcUrl(String url) {
451+
String trimmed = url.trim();
452+
if (trimmed.regionMatches(true, 0, "postgresql://", 0, "postgresql://".length())) {
453+
return "jdbc:postgresql://" + trimmed.substring("postgresql://".length());
454+
}
455+
return trimmed;
456+
}
457+
450458
static int parseIntConfig(Map<String, Object> config, String key, int defaultValue) {
451459
Object val = config.get(key);
452460
if (val == null) {

‎bundles/org.openhab.persistence.timescaledb/src/main/resources/OH-INF/config/timescaledb.xml‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,8 @@
1717
</parameter-group>
1818

1919
<parameter name="url" type="text" required="true" groupName="connection">
20-
<context>url</context>
2120
<label>JDBC URL</label>
22-
<description>JDBC connection URL, e.g. jdbc:postgresql://localhost:5432/openhab</description>
21+
<description>Database URL, e.g. [jdbc]:postgresql://localhost:5432/openhab</description>
2322
</parameter>
2423

2524
<parameter name="user" type="text" required="false" groupName="connection">

‎bundles/org.openhab.persistence.timescaledb/src/test/java/org/openhab/persistence/timescaledb/internal/BundleManifestTest.java‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,14 @@
2424
import java.util.Set;
2525
import java.util.jar.Manifest;
2626

27+
import javax.xml.parsers.DocumentBuilderFactory;
28+
2729
import org.eclipse.jdt.annotation.DefaultLocation;
2830
import org.eclipse.jdt.annotation.NonNullByDefault;
2931
import org.junit.jupiter.api.Test;
32+
import org.w3c.dom.Document;
33+
import org.w3c.dom.Element;
34+
import org.w3c.dom.NodeList;
3035

3136
/**
3237
* Verifies structural requirements of the OSGi bundle to ensure it deploys and integrates
@@ -103,6 +108,37 @@ void addonXmlExists() {
103108
+ "Create src/main/resources/OH-INF/addon/addon.xml.");
104109
}
105110

111+
@Test
112+
void jdbcUrlParameterDoesNotUseUrlContext() throws Exception {
113+
Path configPath = Path.of("src/main/resources/OH-INF/config/timescaledb.xml");
114+
assertTrue(Files.exists(configPath), "OH-INF/config/timescaledb.xml is missing.");
115+
116+
DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
117+
dbf.setNamespaceAware(true);
118+
Document doc = dbf.newDocumentBuilder().parse(configPath.toFile());
119+
120+
Element urlParameter = null;
121+
NodeList parameters = doc.getElementsByTagName("parameter");
122+
for (int i = 0; i < parameters.getLength(); i++) {
123+
Element parameter = (Element) parameters.item(i);
124+
if ("url".equals(parameter.getAttribute("name"))) {
125+
urlParameter = parameter;
126+
break;
127+
}
128+
}
129+
130+
assertNotNull(urlParameter, "timescaledb.xml must define a parameter named 'url'.");
131+
assertEquals("text", urlParameter.getAttribute("type"),
132+
"The 'url' parameter must remain a text field for JDBC URLs.");
133+
134+
NodeList contexts = urlParameter.getElementsByTagName("context");
135+
for (int i = 0; i < contexts.getLength(); i++) {
136+
String context = contexts.item(i).getTextContent().trim();
137+
assertNotEquals("url", context, "The JDBC URL parameter must not use context 'url': "
138+
+ "MainUI enforces http/https URL semantics and rejects jdbc:postgresql://... values.");
139+
}
140+
}
141+
106142
/**
107143
* Parses the OSGi {@code Import-Package} header value into a list of package names,
108144
* stripping directives and attributes (e.g. {@code version="[1.0,2)"}).

‎bundles/org.openhab.persistence.timescaledb/src/test/java/org/openhab/persistence/timescaledb/internal/TimescaleDBContainerTest.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -718,6 +718,25 @@ void serviceActivateInitializesschemaandschedulesjob() throws Exception {
718718
assertNull(jobField.get(service), "Downsampling job must be null after deactivate()");
719719
}
720720

721+
@Test
722+
@Order(70)
723+
void serviceActivateAcceptsPostgresqlUrlWithoutJdbcPrefix() throws Exception {
724+
MetadataRegistry mr = mock(MetadataRegistry.class);
725+
when(mr.getAll()).thenReturn(Collections.emptyList());
726+
TimescaleDBPersistenceService service = new TimescaleDBPersistenceService(mock(ItemRegistry.class), mr,
727+
new TimescaleDBMetadataService(mr));
728+
729+
String urlWithoutJdbc = DB.getJdbcUrl().replaceFirst("^jdbc:", "");
730+
service.activate(Map.of("url", urlWithoutJdbc, "user", DB.getUsername(), "password", DB.getPassword()));
731+
732+
var dsField = TimescaleDBPersistenceService.class.getDeclaredField("dataSource");
733+
dsField.setAccessible(true);
734+
assertNotNull(dsField.get(service), "DataSource must be initialized for postgresql:// URL");
735+
736+
service.deactivate();
737+
assertNull(dsField.get(service), "DataSource must be null after deactivate()");
738+
}
739+
721740
@Test
722741
@Order(71)
723742
void serviceStoreandqueryViaserviceinterface() throws Exception {

‎bundles/org.openhab.persistence.timescaledb/src/test/java/org/openhab/persistence/timescaledb/internal/TimescaleDBPersistenceServiceTest.java‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -385,6 +385,21 @@ void activateInvalidurlDatasourceremainsnull() throws Exception {
385385
realService.deactivate();
386386
}
387387

388+
@Test
389+
void activatePostgresqlUrlWithoutJdbcPrefixIsAcceptedAndNormalized() throws Exception {
390+
var realService = new TimescaleDBPersistenceService(mock(ItemRegistry.class), mock(MetadataRegistry.class),
391+
new TimescaleDBMetadataService(mock(MetadataRegistry.class)));
392+
// Same as invalid URL test, but without jdbc: prefix to verify normalization path.
393+
realService.activate(
394+
Map.of("url", "postgresql://localhost:19999/invalid", "password", "x", "connectTimeout", "500"));
395+
396+
var dsField = TimescaleDBPersistenceService.class.getDeclaredField("dataSource");
397+
dsField.setAccessible(true);
398+
assertTrue(null == dsField.get(realService),
399+
"dataSource must be null when connection fails after URL normalization");
400+
realService.deactivate();
401+
}
402+
388403
@Test
389404
@SuppressWarnings("unchecked")
390405
void deactivateWithscheduledjobCancelsjob() throws Exception {
@@ -446,6 +461,24 @@ void parseIntConfigInvalidvalueReturnsdefault() {
446461
assertEquals(3, TimescaleDBPersistenceService.parseIntConfig(Map.of("k", "notanumber"), "k", 3));
447462
}
448463

464+
@Test
465+
void normalizeJdbcUrlAddsJdbcPrefixForPostgresqlUrls() {
466+
assertEquals("jdbc:postgresql://localhost:5432/openhab",
467+
TimescaleDBPersistenceService.normalizeJdbcUrl("postgresql://localhost:5432/openhab"));
468+
assertEquals("jdbc:postgresql://localhost:5432/openhab",
469+
TimescaleDBPersistenceService.normalizeJdbcUrl("PostgreSQL://localhost:5432/openhab"));
470+
}
471+
472+
@Test
473+
void normalizeJdbcUrlLeavesAlreadyJdbcAndOtherUrlsUnchanged() {
474+
assertEquals("jdbc:postgresql://localhost:5432/openhab",
475+
TimescaleDBPersistenceService.normalizeJdbcUrl("jdbc:postgresql://localhost:5432/openhab"));
476+
assertEquals("jdbc:mysql://localhost:3306/openhab",
477+
TimescaleDBPersistenceService.normalizeJdbcUrl("jdbc:mysql://localhost:3306/openhab"));
478+
assertEquals("jdbc:postgresql://localhost:5432/openhab",
479+
TimescaleDBPersistenceService.normalizeJdbcUrl(" jdbc:postgresql://localhost:5432/openhab "));
480+
}
481+
449482
// ------------------------------------------------------------------
450483
// runDownsampleNow / ConsoleCommandExtension
451484
// ------------------------------------------------------------------

0 commit comments

Comments
 (0)