Skip to content

Commit 6ffdd9c

Browse files
refactor(server): unify URL configs when scheme is missing (#2944)
- Add URL normalization support for config options - Automatically prefix missing schemes (http://, https://) - Log warnings when auto-correcting user-provided values - Add comprehensive test coverage for normalization logic - Update config files to demonstrate the feature Changes: - ConfigOption: Add withUrlNormalization() builder method - ServerOptions: Apply normalization to REST, Gremlin, K8s URLs - HugeConfig: Implement lazy cache and normalization logic - Add ServerOptionsTest with 5 test cases - Simplify URLs in main and Docker config * repair --------- Co-authored-by: imbajin <jin@apache.org>
1 parent 88ad859 commit 6ffdd9c

File tree

8 files changed

+244
-13
lines changed

8 files changed

+244
-13
lines changed

docker/configs/server1-conf/rest-server.properties

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
# bind url
2-
restserver.url=http://127.0.0.1:8081
2+
restserver.url=127.0.0.1:8081
33
# gremlin server url, need to be consistent with host and port in gremlin-server.yaml
4-
gremlinserver.url=http://127.0.0.1:8181
4+
gremlinserver.url=127.0.0.1:8181
55

66
graphs=./conf/graphs
77

docker/configs/server2-conf/rest-server.properties

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
# bind url
2-
restserver.url=http://127.0.0.1:8082
2+
restserver.url=127.0.0.1:8082
33
# gremlin server url, need to be consistent with host and port in gremlin-server.yaml
4-
gremlinserver.url=http://127.0.0.1:8182
4+
gremlinserver.url=127.0.0.1:8182
55

66
graphs=./conf/graphs
77

docker/configs/server3-conf/rest-server.properties

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
# bind url
2-
restserver.url=http://127.0.0.1:8083
2+
restserver.url=127.0.0.1:8083
33
# gremlin server url, need to be consistent with host and port in gremlin-server.yaml
4-
gremlinserver.url=http://127.0.0.1:8183
4+
gremlinserver.url=127.0.0.1:8183
55

66
graphs=./conf/graphs
77

hugegraph-commons/hugegraph-common/src/main/java/org/apache/hugegraph/config/ConfigOption.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,22 @@
2020
import com.google.common.base.Predicate;
2121

2222
public class ConfigOption<T> extends TypedOption<T, T> {
23+
private boolean urlNormalize = false;
24+
private String defaultScheme = null;
25+
26+
public ConfigOption<T> withUrlNormalization(String scheme) {
27+
this.urlNormalize = true;
28+
this.defaultScheme = scheme;
29+
return this;
30+
}
31+
32+
public boolean needsUrlNormalization() {
33+
return this.urlNormalize;
34+
}
35+
36+
public String getDefaultScheme() {
37+
return this.defaultScheme;
38+
}
2339

2440
public ConfigOption(String name, String desc, T value) {
2541
this(name, desc, null, value);

hugegraph-commons/hugegraph-common/src/main/java/org/apache/hugegraph/config/HugeConfig.java

Lines changed: 94 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,9 @@ public class HugeConfig extends PropertiesConfiguration {
4343

4444
private static final Logger LOG = Log.logger(HugeConfig.class);
4545

46+
// Cache for URL normalization metadata (populated lazily per key)
47+
private static final Map<String, String> URL_NORMALIZATIONS = new HashMap<>();
48+
4649
private String configPath;
4750

4851
public HugeConfig(Configuration config) {
@@ -87,9 +90,17 @@ private void setLayoutIfNeeded(Configuration conf) {
8790
@SuppressWarnings("unchecked")
8891
public <T, R> R get(TypedOption<T, R> option) {
8992
Object value = this.getProperty(option.name());
93+
boolean fromDefault = false;
94+
9095
if (value == null) {
91-
return option.defaultValue();
96+
value = option.defaultValue();
97+
fromDefault = true;
9298
}
99+
100+
if (!fromDefault) {
101+
value = normalizeUrlOptionIfNeeded(option.name(), value);
102+
}
103+
93104
return (R) value;
94105
}
95106

@@ -213,4 +224,86 @@ private static Configuration loadConfigFile(File configFile) {
213224
e, configFile);
214225
}
215226
}
227+
228+
private static Object normalizeUrlOptionIfNeeded(String key, Object value) {
229+
if (value == null) {
230+
return null;
231+
}
232+
233+
String scheme = defaultSchemeFor(key);
234+
if (scheme == null) {
235+
return value;
236+
}
237+
238+
// Normalize URL options if configured with .withUrlNormalization()
239+
if (value instanceof String) {
240+
String original = (String) value;
241+
String normalized = prefixSchemeIfMissing(original, scheme);
242+
243+
if (!original.equals(normalized)) {
244+
LOG.warn("Config '{}' is missing scheme, auto-corrected to '{}'",
245+
key, normalized);
246+
}
247+
248+
return normalized;
249+
}
250+
251+
// If it ever hits here, it means config storage returned a non-string type;
252+
// leave it unchanged (safer than forcing toString()).
253+
return value;
254+
}
255+
256+
private static String defaultSchemeFor(String key) {
257+
// Check if we already cached this key's scheme
258+
if (URL_NORMALIZATIONS.containsKey(key)) {
259+
return URL_NORMALIZATIONS.get(key);
260+
}
261+
262+
// We don't know yet - look it up NOW from OptionSpace
263+
synchronized (URL_NORMALIZATIONS) {
264+
// Double-check after acquiring lock
265+
if (URL_NORMALIZATIONS.containsKey(key)) {
266+
return URL_NORMALIZATIONS.get(key);
267+
}
268+
269+
// Look up the option from OptionSpace
270+
TypedOption<?, ?> option = OptionSpace.get(key);
271+
String scheme = null;
272+
273+
if (option instanceof ConfigOption) {
274+
ConfigOption<?> configOption = (ConfigOption<?>) option;
275+
if (configOption.needsUrlNormalization()) {
276+
scheme = configOption.getDefaultScheme();
277+
}
278+
}
279+
280+
// Cache it for next time (even if null)
281+
URL_NORMALIZATIONS.put(key, scheme);
282+
return scheme;
283+
}
284+
}
285+
286+
private static String prefixSchemeIfMissing(String raw, String scheme) {
287+
if (raw == null) {
288+
return null;
289+
}
290+
String s = raw.trim();
291+
if (s.isEmpty()) {
292+
return s;
293+
}
294+
295+
int scIdx = s.indexOf("://");
296+
if (scIdx > 0) {
297+
// Normalize existing scheme to lowercase while preserving the rest
298+
String existingScheme = s.substring(0, scIdx).toLowerCase();
299+
String rest = s.substring(scIdx + 3); // skip the "://" delimiter
300+
return existingScheme + "://" + rest;
301+
}
302+
303+
String defaultScheme = scheme == null ? "" : scheme;
304+
if (!defaultScheme.isEmpty() && !defaultScheme.endsWith("://")) {
305+
defaultScheme = defaultScheme + "://";
306+
}
307+
return defaultScheme + s;
308+
}
216309
}

hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ public class ServerOptions extends OptionHolder {
3232
"The url for listening of graph server.",
3333
disallowEmpty(),
3434
"http://127.0.0.1:8080"
35-
);
35+
).withUrlNormalization("http://");
3636

3737
public static final ConfigOption<Integer> SERVER_EVENT_HUB_THREADS =
3838
new ConfigOption<>(
@@ -118,7 +118,7 @@ public class ServerOptions extends OptionHolder {
118118
"The url of gremlin server.",
119119
disallowEmpty(),
120120
"http://127.0.0.1:8182"
121-
);
121+
).withUrlNormalization("http://");
122122

123123
public static final ConfigOption<Integer> GREMLIN_SERVER_TIMEOUT =
124124
new ConfigOption<>(
@@ -270,15 +270,15 @@ public class ServerOptions extends OptionHolder {
270270
"to clients. only used when starting the server in k8s.",
271271
disallowEmpty(),
272272
"http://0.0.0.0:8080"
273-
);
273+
).withUrlNormalization("http://");
274274

275275
public static final ConfigOption<String> SERVER_K8S_URL =
276276
new ConfigOption<>(
277277
"server.k8s_url",
278278
"The url of k8s.",
279279
disallowEmpty(),
280280
"https://127.0.0.1:8888"
281-
);
281+
).withUrlNormalization("https://");
282282

283283
public static final ConfigOption<Boolean> SERVER_K8S_USE_CA =
284284
new ConfigOption<>(

hugegraph-server/hugegraph-dist/src/assembly/static/conf/rest-server.properties

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
# bind url
22
# could use '0.0.0.0' or specified (real)IP to expose external network access
3-
restserver.url=http://127.0.0.1:8080
3+
restserver.url=127.0.0.1:8080
44
#restserver.enable_graphspaces_filter=false
55
# gremlin server url, need to be consistent with host and port in gremlin-server.yaml
6-
#gremlinserver.url=http://127.0.0.1:8182
6+
#gremlinserver.url=127.0.0.1:8182
77

88
graphs=./conf/graphs
99

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with this
4+
* work for additional information regarding copyright ownership. The ASF
5+
* licenses this file to You under the Apache License, Version 2.0 (the
6+
* "License"); you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
13+
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
14+
* License for the specific language governing permissions and limitations
15+
* under the License.
16+
*/
17+
18+
package org.apache.hugegraph.unit.config;
19+
20+
import org.apache.commons.configuration2.PropertiesConfiguration;
21+
import org.apache.hugegraph.config.HugeConfig;
22+
import org.apache.hugegraph.config.OptionSpace;
23+
import org.apache.hugegraph.config.ServerOptions;
24+
import org.apache.hugegraph.testutil.Assert;
25+
import org.junit.BeforeClass;
26+
import org.junit.Test;
27+
28+
public class ServerOptionsTest {
29+
30+
@BeforeClass
31+
public static void init() {
32+
OptionSpace.register("server",
33+
ServerOptions.class.getName());
34+
}
35+
36+
@Test
37+
public void testUrlOptionNormalizeAddsDefaultScheme() {
38+
PropertiesConfiguration conf = new PropertiesConfiguration();
39+
conf.setProperty("restserver.url", "127.0.0.1:8080");
40+
conf.setProperty("gremlinserver.url", "127.0.0.1:8182");
41+
conf.setProperty("server.urls_to_pd", "0.0.0.0:8080");
42+
conf.setProperty("server.k8s_url", "127.0.0.1:8888");
43+
44+
HugeConfig config = new HugeConfig(conf);
45+
46+
Assert.assertEquals("http://127.0.0.1:8080",
47+
config.get(ServerOptions.REST_SERVER_URL));
48+
Assert.assertEquals("http://127.0.0.1:8182",
49+
config.get(ServerOptions.GREMLIN_SERVER_URL));
50+
Assert.assertEquals("http://0.0.0.0:8080",
51+
config.get(ServerOptions.SERVER_URLS_TO_PD));
52+
Assert.assertEquals("https://127.0.0.1:8888",
53+
config.get(ServerOptions.SERVER_K8S_URL));
54+
}
55+
56+
@Test
57+
public void testUrlNormalizationEdgeCases() {
58+
// Whitespace trimming
59+
PropertiesConfiguration conf = new PropertiesConfiguration();
60+
conf.setProperty("restserver.url", " 127.0.0.1:8080 ");
61+
HugeConfig config = new HugeConfig(conf);
62+
Assert.assertEquals("http://127.0.0.1:8080",
63+
config.get(ServerOptions.REST_SERVER_URL));
64+
65+
// Case normalization
66+
conf = new PropertiesConfiguration();
67+
conf.setProperty("restserver.url", "HTTP://127.0.0.1:8080");
68+
config = new HugeConfig(conf);
69+
Assert.assertEquals("http://127.0.0.1:8080",
70+
config.get(ServerOptions.REST_SERVER_URL));
71+
72+
// IPv6 without scheme
73+
conf = new PropertiesConfiguration();
74+
conf.setProperty("restserver.url", "[::1]:8080");
75+
config = new HugeConfig(conf);
76+
Assert.assertEquals("http://[::1]:8080",
77+
config.get(ServerOptions.REST_SERVER_URL));
78+
79+
// IPv6 with existing scheme
80+
conf = new PropertiesConfiguration();
81+
conf.setProperty("restserver.url", "http://[::1]:8080");
82+
config = new HugeConfig(conf);
83+
Assert.assertEquals("http://[::1]:8080",
84+
config.get(ServerOptions.REST_SERVER_URL));
85+
}
86+
87+
@Test
88+
public void testUrlNormalizationPreservesHostnameCase() {
89+
// Uppercase scheme + mixed-case hostname
90+
PropertiesConfiguration conf = new PropertiesConfiguration();
91+
conf.setProperty("restserver.url", "HTTP://MyServer:8080");
92+
HugeConfig config = new HugeConfig(conf);
93+
// Should lowercase ONLY the scheme, preserve "MyServer"
94+
Assert.assertEquals("http://MyServer:8080",
95+
config.get(ServerOptions.REST_SERVER_URL));
96+
97+
// Use server.k8s_url for HTTPS test (it defaults to https://)
98+
conf = new PropertiesConfiguration();
99+
conf.setProperty("server.k8s_url", "HTTPS://MyHost:8888");
100+
config = new HugeConfig(conf);
101+
Assert.assertEquals("https://MyHost:8888",
102+
config.get(ServerOptions.SERVER_K8S_URL));
103+
}
104+
105+
@Test
106+
public void testUrlNormalizationPreservesPathCase() {
107+
PropertiesConfiguration conf = new PropertiesConfiguration();
108+
conf.setProperty("restserver.url", "http://127.0.0.1:8080/SomePath/CaseSensitive");
109+
HugeConfig config = new HugeConfig(conf);
110+
Assert.assertEquals("http://127.0.0.1:8080/SomePath/CaseSensitive",
111+
config.get(ServerOptions.REST_SERVER_URL));
112+
}
113+
114+
@Test
115+
public void testHttpsSchemeIsNotDowngraded() {
116+
PropertiesConfiguration conf = new PropertiesConfiguration();
117+
conf.setProperty("restserver.url", "https://127.0.0.1:8080");
118+
HugeConfig config = new HugeConfig(conf);
119+
Assert.assertEquals("https://127.0.0.1:8080",
120+
config.get(ServerOptions.REST_SERVER_URL));
121+
}
122+
}

0 commit comments

Comments
 (0)