Skip to content

Commit f481b85

Browse files
dmatejdmatej
committed
Fixed file path to uri conversions
- Were missing on some places - That fact broke some tests on some Windows file systems which used "suspicious" file path/uri strings refused by java while the string should have always been an URI. - I pushed the conversion from getters to setters to make it clear what kind of URI/path was used - then we can do more appropriate coversion earlier and just once (however GlassFishProperties are still just a bunch of strings so there will still be redundant conversions for now). Signed-off-by: David Matějček <[email protected]>
1 parent 41ec342 commit f481b85

File tree

10 files changed

+194
-53
lines changed

10 files changed

+194
-53
lines changed

‎appserver/ant-tasks/src/main/java/org/glassfish/ant/embedded/tasks/Util.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
/*
2+
* Copyright (c) 2024 Contributors to the Eclipse Foundation
23
* Copyright (c) 2009, 2018 Oracle and/or its affiliates. All rights reserved.
34
*
45
* This program and the accompanying materials are made available under the
@@ -39,7 +40,7 @@ public class Util {
3940
private static GlassFishRuntime glassfishRuntime;
4041

4142
public static synchronized GlassFish startGlassFish(String serverID, String installRoot,
42-
String instanceRoot, String configFileURI,
43+
String instanceRoot, String configFile,
4344
boolean configFileReadOnly, int httpPort)
4445
throws GlassFishException {
4546
GlassFish glassfish = gfMap.get(serverID);
@@ -58,12 +59,12 @@ public static synchronized GlassFish startGlassFish(String serverID, String inst
5859
if (instanceRoot != null) {
5960
glassfishProperties.setInstanceRoot(instanceRoot);
6061
}
61-
if (configFileURI != null) {
62-
glassfishProperties.setConfigFileURI(configFileURI);
62+
if (configFile != null) {
63+
glassfishProperties.setConfigFileURI(new File(configFile).toURI().toString());
6364
glassfishProperties.setConfigFileReadOnly(configFileReadOnly);
6465
}
6566

66-
if (instanceRoot==null && configFileURI==null) {
67+
if (instanceRoot == null && configFile == null) {
6768
// only set port if embedded domain.xml is used
6869
if (httpPort != -1) {
6970
glassfishProperties.setPort("http-listener", httpPort);
@@ -92,7 +93,7 @@ public static void deploy(String app, String serverId, List<String> deployParams
9293
Deployer deployer = glassfish.getDeployer();
9394
final int len = deployParams.size();
9495
if (len > 0) {
95-
deployer.deploy(new File(app).toURI(), deployParams.toArray(new String[len]));
96+
deployer.deploy(new File(app).toURI(), deployParams.toArray(String[]::new));
9697
System.out.println("Deployed [" + app + "] with parameters " + deployParams);
9798
} else {
9899
deployer.deploy(new File(app).toURI());

‎appserver/ejb/ejb-container/src/main/java/org/glassfish/ejb/embedded/EJBContainerProviderImpl.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2022, 2023 Contributors to the Eclipse Foundation.
2+
* Copyright (c) 2022, 2024 Contributors to the Eclipse Foundation.
33
* Copyright (c) 2009, 2020 Oracle and/or its affiliates. All rights reserved.
44
*
55
* This program and the accompanying materials are made available under the
@@ -184,8 +184,9 @@ private Locations createContainer(Map<?, ?> properties, Locations l) throws EJBE
184184
_logger.info("[EJBContainerProviderImpl] Using instance location: " + l.instance_root.getCanonicalPath());
185185
glassFishProperties.setInstanceRoot(l.instance_root.getCanonicalPath());
186186
} else if (l.domain_file != null) {
187-
_logger.info("[EJBContainerProviderImpl] Using config file location: " + l.domain_file.toURI().toString());
188-
glassFishProperties.setConfigFileURI(l.domain_file.toURI().toString());
187+
String cfgFileUri = l.domain_file.toURI().toString();
188+
_logger.info("[EJBContainerProviderImpl] Using config file location: " + cfgFileUri);
189+
glassFishProperties.setConfigFileURI(cfgFileUri);
189190
}
190191
addWebContainerIfRequested(properties, glassFishProperties);
191192

‎appserver/tests/embedded/basic/config/src/test/java/org/glassfish/tests/embedded/basic/config/ConfigTest.java

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -43,33 +43,34 @@
4343

4444
public class ConfigTest {
4545

46-
public static final Logger logger = Logger.getLogger(ConfigTest.class.getName());
46+
private static final Logger LOG = Logger.getLogger(ConfigTest.class.getName());
4747

48-
static GlassFishRuntime runtime;
48+
private static GlassFishRuntime runtime;
4949

5050
@BeforeAll
5151
static void bootStrap() throws GlassFishException {
52-
runtime = GlassFishRuntime.bootstrap();
52+
runtime = GlassFishRuntime.bootstrap();
5353
}
5454

55+
5556
@ParameterizedTest
5657
@ArgumentsSource(ConfigFileArgumentsProvider.class)
57-
void testConfigFile(String configFile) throws GlassFishException, IOException, URISyntaxException {
58+
void testConfigFile(String configFile) throws Exception {
5859
GlassFishProperties gfp = new GlassFishProperties();
5960
gfp.setConfigFileURI(configFile);
6061

6162
GlassFish instance1 = runtime.newGlassFish(gfp);
62-
logger.info(() -> "Instance1 created" + instance1);
63+
LOG.info(() -> "Instance1 created" + instance1);
6364
instance1.start();
64-
logger.info("Instance1 started #1");
65+
LOG.info("Instance1 started #1");
6566

6667
final String domainName = instance1.getService(Domain.class).getName();
6768
assertTrue("myDomain".equals(domainName), "Domain name is " + domainName);
6869

6970
instance1.stop();
70-
logger.info("Instance1 stopped #1");
71+
LOG.info("Instance1 stopped #1");
7172
instance1.dispose();
72-
logger.info("Instance1 disposed");
73+
LOG.info("Instance1 disposed");
7374
checkDisposed();
7475
}
7576

@@ -91,7 +92,7 @@ private String fileInClassPath() throws URISyntaxException, IOException {
9192
// throws exception if the temp dir is not cleaned out.
9293
private void checkDisposed() {
9394
String instanceRoot = System.getProperty("com.sun.aas.instanceRoot");
94-
logger.info(() -> "Checking whether " + instanceRoot + " is disposed or not");
95+
LOG.info(() -> "Checking whether " + instanceRoot + " is disposed or not");
9596
if (new File(instanceRoot).exists()) {
9697
throw new RuntimeException("Directory " + instanceRoot +
9798
" is not cleaned up after glassfish.dispose()");

‎nucleus/common/simple-glassfish-api/pom.xml

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,4 +30,22 @@
3030
<packaging>glassfish-jar</packaging>
3131

3232
<name>Simple Public APIs of Glassfish</name>
33+
34+
<dependencies>
35+
<dependency>
36+
<groupId>org.junit.jupiter</groupId>
37+
<artifactId>junit-jupiter-engine</artifactId>
38+
</dependency>
39+
</dependencies>
40+
41+
<build>
42+
<testResources>
43+
<testResource>
44+
<directory>src/test/resources</directory>
45+
<includes>
46+
<include>**/*</include>
47+
</includes>
48+
</testResource>
49+
</testResources>
50+
</build>
3351
</project>

‎nucleus/common/simple-glassfish-api/src/main/java/org/glassfish/embeddable/GlassFishProperties.java

Lines changed: 59 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public class GlassFishProperties {
3838

3939
/** Key for specifying which instance root (aka domain dir) GlassFish should run with. */
4040
public static final String INSTANCE_ROOT_PROP_NAME = "com.sun.aas.instanceRoot";
41-
/** Key for specifying which configuration file (domain.xml) GlassFish should run with. */
41+
/** Key for specifying absolute URI which configuration file (domain.xml) GlassFish should run with. */
4242
public static final String CONFIG_FILE_URI_PROP_NAME = "org.glassfish.embeddable.configFileURI";
4343
/**
4444
* Key for specifying whether the specified configuration file (domain.xml) or config/domain.xml
@@ -130,35 +130,82 @@ public String getInstanceRoot() {
130130
return gfProperties.getProperty(INSTANCE_ROOT_PROP_NAME);
131131
}
132132

133+
133134
/**
134135
* Optionally set the location of configuration file (i.e., domain.xml) using
135136
* which the GlassFish should run.
136-
* <p/>
137+
* <p>
138+
* If the parameter does not start with <code>file:</code>, it is resolved as a local file path
139+
* relative to the current directory.
140+
* <p>
137141
* Unless specified, the configuration file is operated on read only mode.
138142
* To writeback any changes, call {@link #setConfigFileReadOnly(boolean)} with 'false'.
139143
*
140-
* @param configFileURI Location of configuration file. File path, or full URI with the schema, e.g. file:
144+
* @param configFileURI Location of configuration file. File path, or full URI with the schema,
145+
* e.g. <code>file:</code>
141146
*/
147+
@Deprecated(forRemoval = true, since = "7.0.20")
142148
public void setConfigFileURI(String configFileURI) {
143-
gfProperties.setProperty(CONFIG_FILE_URI_PROP_NAME, configFileURI);
149+
if (configFileURI == null) {
150+
gfProperties.remove(CONFIG_FILE_URI_PROP_NAME);
151+
return;
152+
}
153+
if (configFileURI.startsWith("file:")) {
154+
setConfigFile(URI.create(configFileURI));
155+
} else {
156+
setConfigFile(new File(configFileURI).toURI());
157+
}
144158
}
145159

146160
/**
147-
* Get the configurationFileURI set using {@link #setConfigFileURI(String)}
161+
* Optionally set the location of configuration file (i.e., domain.xml) using
162+
* which the GlassFish should run. The file will be converted to the absolute URI.
163+
* <p>
164+
* Unless specified, the configuration file is operated on read only mode.
165+
* To writeback any changes, call {@link #setConfigFileReadOnly(boolean)} with 'false'.
148166
*
149-
* @return The configurationFileURI set using {@link #setConfigFileURI(String)}
167+
* @param configFile Location of configuration file. File path, or full URI with the schema,
168+
* e.g. <code>file:</code>
150169
*/
151-
public String getConfigFileURI() {
152-
return gfProperties.getProperty(CONFIG_FILE_URI_PROP_NAME);
170+
public void setConfigFile(File configFile) {
171+
if (configFile == null) {
172+
gfProperties.remove(CONFIG_FILE_URI_PROP_NAME);
173+
return;
174+
}
175+
setConfigFile(configFile.toURI());
153176
}
154177

155178
/**
156-
* Get the absolute URI, with schema, set using {@link #setConfigFileURI(String)}. Internally uses {@link #filePathToAbsoluteURI(java.lang.String)}.
179+
* Optionally set the location of configuration file (i.e., domain.xml) using
180+
* which the GlassFish should run. The parameter must be an absolute URI or null.
181+
* <p>
182+
* Unless specified, the configuration file is operated on read only mode.
183+
* To writeback any changes, call {@link #setConfigFileReadOnly(boolean)} with 'false'.
157184
*
158-
* @return The configurationFileURI set using {@link #setConfigFileURI(String)} converted to URI
185+
* @param configFileURI Location of configuration file. File path, or full URI with the schema,
186+
* e.g. <code>file:</code>
187+
* @throws IllegalArgumentException If the parameter is not an absolute URI or null.
159188
*/
160-
public URI getAbsoluteConfigFileURI() {
161-
return filePathToAbsoluteURI(getConfigFileURI());
189+
public void setConfigFile(URI configFileURI) {
190+
if (configFileURI == null) {
191+
gfProperties.remove(CONFIG_FILE_URI_PROP_NAME);
192+
return;
193+
}
194+
if (configFileURI.isAbsolute()) {
195+
gfProperties.setProperty(CONFIG_FILE_URI_PROP_NAME, configFileURI.toString());
196+
return;
197+
}
198+
throw new IllegalArgumentException("The URI is not absolute: " + configFileURI);
199+
}
200+
201+
/**
202+
* Get the absolute configurationFileURI set using {@link #setConfigFileURI(String)}
203+
*
204+
* @return The configurationFileURI set using {@link #setConfigFileURI(String)}
205+
*/
206+
public URI getConfigFileURI() {
207+
String uri = gfProperties.getProperty(CONFIG_FILE_URI_PROP_NAME);
208+
return uri == null ? null : URI.create(uri);
162209
}
163210

164211
/**
@@ -255,22 +302,4 @@ public int getPort(String networkListener) {
255302
}
256303
return port;
257304
}
258-
259-
/**
260-
* Converts to absolute URI with absolute path.
261-
* If the filePath doesn't contain schema, it will add file: schema
262-
*
263-
* @param filePath Path to create the URI
264-
* @return absolute URI
265-
*/
266-
public static URI filePathToAbsoluteURI(String filePath) {
267-
if (filePath == null) {
268-
return null;
269-
}
270-
URI uri = URI.create(filePath);
271-
if (!uri.isAbsolute()) {
272-
return new File(filePath).getAbsoluteFile().toURI();
273-
}
274-
return uri;
275-
}
276305
}

‎nucleus/common/simple-glassfish-api/src/test/java/org/glassfish/embeddable/GlassFishPropertiesTest.java

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
/*
2+
* Copyright (c) 2024 Eclipse Foundation and/or its affiliates.
3+
*
4+
* This program and the accompanying materials are made available under the
5+
* terms of the Eclipse Public License v. 2.0, which is available at
6+
* http://www.eclipse.org/legal/epl-2.0.
7+
*
8+
* This Source Code may also be made available under the following Secondary
9+
* Licenses when the conditions for such availability set forth in the
10+
* Eclipse Public License v. 2.0 are satisfied: GNU General Public License,
11+
* version 2 with the GNU Classpath Exception, which is available at
12+
* https://www.gnu.org/software/classpath/license.html.
13+
*
14+
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
15+
*/
16+
17+
package org.glassfish.embeddable;
18+
19+
import java.io.File;
20+
import java.net.URI;
21+
import java.net.URISyntaxException;
22+
import java.net.URL;
23+
import java.net.URLEncoder;
24+
import java.nio.file.Path;
25+
26+
import org.junit.jupiter.api.Test;
27+
28+
import static java.nio.charset.StandardCharsets.UTF_8;
29+
import static org.junit.jupiter.api.Assertions.assertEquals;
30+
import static org.junit.jupiter.api.Assertions.assertNull;
31+
32+
public class GlassFishPropertiesTest {
33+
34+
private static final String DOMAIN_XML_ABS_WINDOWS = "file:/" + URLEncoder.encode("D:\\a\\domain.xml", UTF_8);
35+
private static final File DOMAIN_XML_ABS = getAbsoluteDomainXml();
36+
private static final Path CURRENT_DIR_ABS = new File(".").getAbsoluteFile().toPath();
37+
private static final File DOMAIN_XML_REL = CURRENT_DIR_ABS.relativize(DOMAIN_XML_ABS.toPath()).toFile();
38+
39+
@SuppressWarnings("removal")
40+
@Test
41+
public void setConfigFileURIString() {
42+
GlassFishProperties props = new GlassFishProperties();
43+
props.setConfigFileURI(DOMAIN_XML_ABS.getAbsolutePath());
44+
assertEquals(DOMAIN_XML_ABS.toURI(), props.getConfigFileURI());
45+
props.setConfigFileURI(DOMAIN_XML_REL.getPath());
46+
assertEquals(DOMAIN_XML_ABS.toURI(), props.getConfigFileURI());
47+
props.setConfigFileURI(DOMAIN_XML_ABS_WINDOWS);
48+
assertEquals(DOMAIN_XML_ABS_WINDOWS, props.getConfigFileURI().toString());
49+
props.setConfigFileURI(null);
50+
assertNull(props.getConfigFileURI());
51+
}
52+
53+
@Test
54+
public void setConfigFileURI() {
55+
GlassFishProperties props = new GlassFishProperties();
56+
props.setConfigFile(DOMAIN_XML_ABS.toURI());
57+
assertEquals(DOMAIN_XML_ABS.toURI(), props.getConfigFileURI());
58+
props.setConfigFile(DOMAIN_XML_REL.toURI());
59+
assertEquals(DOMAIN_XML_ABS.toURI(), props.getConfigFileURI());
60+
props.setConfigFile(URI.create(DOMAIN_XML_ABS_WINDOWS));
61+
assertEquals(DOMAIN_XML_ABS_WINDOWS, props.getConfigFileURI().toString());
62+
props.setConfigFile((URI) null);
63+
assertNull(props.getConfigFileURI());
64+
}
65+
66+
@Test
67+
public void setConfigFile() {
68+
GlassFishProperties props = new GlassFishProperties();
69+
props.setConfigFile(DOMAIN_XML_ABS);
70+
assertEquals(DOMAIN_XML_ABS.toURI(), props.getConfigFileURI());
71+
props.setConfigFile(DOMAIN_XML_REL);
72+
assertEquals(DOMAIN_XML_ABS.toURI(), props.getConfigFileURI());
73+
props.setConfigFile((File) null);
74+
assertNull(props.getConfigFileURI());
75+
}
76+
77+
private static File getAbsoluteDomainXml() {
78+
URL url = GlassFishPropertiesTest.class.getClassLoader().getResource("domain.xml");
79+
try {
80+
return new File(url.toURI());
81+
} catch (URISyntaxException e) {
82+
throw new IllegalStateException(e);
83+
}
84+
}
85+
}

‎nucleus/common/simple-glassfish-api/src/test/resources/domain.xml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<domain>
3+
</domain>

‎nucleus/core/bootstrap/src/main/java/com/sun/enterprise/glassfish/bootstrap/embedded/EmbeddedGlassFishRuntime.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ private String createTempInstanceRoot(GlassFishProperties gfProps) throws Except
199199
}
200200

201201
// If the user has specified a custom domain.xml then copy it.
202-
URI configFileURI = gfProps.getAbsoluteConfigFileURI();
202+
URI configFileURI = gfProps.getConfigFileURI();
203203
if(configFileURI != null) {
204204
copy(configFileURI.toURL(), new File(instanceConfigDir, "domain.xml"), true);
205205
}

‎nucleus/core/kernel/src/main/java/org/glassfish/kernel/embedded/EmbeddedDomainXml.java

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2023 Contributors to the Eclipse Foundation
2+
* Copyright (c) 2023, 2024 Contributors to the Eclipse Foundation
33
* Copyright (c) 2009, 2018 Oracle and/or its affiliates. All rights reserved.
44
*
55
* This program and the accompanying materials are made available under the
@@ -24,6 +24,7 @@
2424

2525
import java.io.File;
2626
import java.io.IOException;
27+
import java.net.URI;
2728
import java.net.URL;
2829

2930
import org.glassfish.embeddable.GlassFishProperties;
@@ -47,13 +48,15 @@ protected URL getDomainXml(ServerEnvironmentImpl env) throws IOException {
4748

4849
static URL getDomainXml(StartupContext startupContext) throws IOException {
4950
String configFileURI = startupContext.getArguments().getProperty(GlassFishProperties.CONFIG_FILE_URI_PROP_NAME);
50-
if (configFileURI != null) { // user specified domain.xml
51-
return GlassFishProperties.filePathToAbsoluteURI(configFileURI).toURL();
51+
if (configFileURI != null) {
52+
// user specified domain.xml
53+
return URI.create(configFileURI).toURL();
5254
}
5355
String instanceRoot = startupContext.getArguments().getProperty(
5456
"com.sun.aas.instanceRoot");
5557
File domainXml = new File(instanceRoot, "config/domain.xml");
56-
if (domainXml.exists()) { // domain/config/domain.xml, if exists.
58+
if (domainXml.exists()) {
59+
// domain/config/domain.xml, if exists.
5760
return domainXml.toURI().toURL();
5861
}
5962
return EmbeddedDomainXml.class.getClassLoader().getResource(

0 commit comments

Comments
 (0)