Skip to content

Commit 0dd2368

Browse files
committed
[bugfix] harden WindowsPathResource wrapper and Jetty startup diagnostics
- Symmetric equals via delegate unwrapping - Reorder wrapIfNeeded guards to prevent double-wrap on Windows - Use OSUtil.isWindows() consistently - Remove System.err from recordStartupFailure; rely on logger + test exception detail - Document instanceof PathResource limitation in WindowsPathResource javadoc
1 parent 74cccf6 commit 0dd2368

4 files changed

Lines changed: 19 additions & 16 deletions

File tree

exist-core/src/main/java/org/exist/jetty/JettyStart.java

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -135,20 +135,13 @@ private static void consoleOut(final String msg) {
135135
System.out.println(msg); //NOSONAR this has to go to the console
136136
}
137137

138-
private static void consoleErr(final String msg) {
139-
System.err.println(msg); //NOSONAR surfaced in Surefire output when test log4j root is OFF
140-
}
141-
142138
private synchronized void recordStartupFailure(final String detail, final Throwable cause) {
143139
webAppStartedSuccessfully = false;
144140
webAppStartupFailureDetail = detail;
145141
if (cause != null) {
146142
logger.fatal("Jetty startup failed: {}", detail, cause);
147-
consoleErr("Jetty startup failed: " + detail);
148-
cause.printStackTrace(System.err); //NOSONAR CI diagnostics when log4j is disabled in tests
149143
} else {
150144
logger.fatal("Jetty startup failed: {}", detail);
151-
consoleErr("Jetty startup failed: " + detail);
152145
}
153146
}
154147

@@ -856,8 +849,8 @@ public synchronized boolean isWebAppStartedSuccessfully() {
856849

857850
/**
858851
* When {@link #isWebAppStartedSuccessfully()} is {@code false}, holds the last startup failure
859-
* message for test diagnostics (also printed to {@code System.err} because module test log4j
860-
* configs often set {@code Root level="OFF"}).
852+
* message for test diagnostics (surfaced by {@link org.exist.test.ExistWebServer} in thrown
853+
* {@link IllegalStateException}s).
861854
*/
862855
public synchronized Optional<String> getWebAppStartupFailureDetail() {
863856
return Optional.ofNullable(webAppStartupFailureDetail);

exist-core/src/main/java/org/exist/jetty/WindowsPathResource.java

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@
2525
import org.eclipse.jetty.util.resource.PathResource;
2626
import org.eclipse.jetty.util.resource.Resource;
2727
import org.eclipse.jetty.util.resource.ResourceFactory;
28+
import org.exist.util.OSUtil;
2829

29-
import java.io.File;
3030
import java.net.URI;
3131
import java.nio.file.Path;
3232
import java.nio.file.Paths;
@@ -40,6 +40,10 @@
4040
* {@code path.resolve(uri.getPath())}, which throws {@link java.nio.file.InvalidPathException}
4141
* when the URI path is {@code /D:/...}. Exploded test webapps on Windows CI hit this in
4242
* {@code WebAppContext.getWebInf()}. Remove when upstream Jetty restores safe Windows resolve.
43+
* <p>
44+
* This class extends {@link Resource}, not {@link PathResource}. Jetty internals that use
45+
* {@code instanceof PathResource} will not treat wrapped resources as path resources; CI and
46+
* integration tests validate that the exploded-webapp startup path does not depend on that.
4347
*/
4448
public final class WindowsPathResource extends Resource {
4549

@@ -52,12 +56,15 @@ private WindowsPathResource(final Resource delegate, final ResourceFactory resou
5256
}
5357

5458
public static Resource wrapIfNeeded(final Resource resource, final ResourceFactory resourceFactory) {
55-
if (resource == null || File.separatorChar != '\\' || !(resource instanceof PathResource)) {
59+
if (resource == null || !OSUtil.isWindows()) {
5660
return resource;
5761
}
5862
if (resource instanceof WindowsPathResource) {
5963
return resource;
6064
}
65+
if (!(resource instanceof PathResource)) {
66+
return resource;
67+
}
6168
return new WindowsPathResource(resource, resourceFactory);
6269
}
6370

@@ -114,10 +121,13 @@ public boolean equals(final Object obj) {
114121
if (this == obj) {
115122
return true;
116123
}
117-
if (!(obj instanceof WindowsPathResource other)) {
124+
if (!(obj instanceof Resource other)) {
118125
return false;
119126
}
120-
return delegate.equals(other.delegate);
127+
if (other instanceof WindowsPathResource wrapped) {
128+
other = wrapped.delegate;
129+
}
130+
return delegate.equals(other);
121131
}
122132

123133
@Override

exist-core/src/main/java/org/exist/test/ExistWebServer.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@
6767
* <li>{@code jetty.home} — Jetty configuration directory ({@code exist-jetty-config/target/classes/...})</li>
6868
* </ul>
6969
* Startup failures throw {@link IllegalStateException} with detail from {@link org.exist.jetty.JettyStart}
70-
* (also printed to {@code System.err} when test log4j root is OFF).
70+
* ({@code webAppStartupFailureDetail} is included in the exception message).
7171
*/
7272
public class ExistWebServer extends ExternalResource {
7373

extensions/webdav/src/test/java/org/exist/jetty/WindowsPathResourceIT.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,9 @@
2424
import org.eclipse.jetty.util.resource.PathResource;
2525
import org.eclipse.jetty.util.resource.Resource;
2626
import org.eclipse.jetty.util.resource.ResourceFactory;
27+
import org.exist.util.OSUtil;
2728
import org.junit.Test;
2829

29-
import java.io.File;
3030
import java.nio.file.Files;
3131
import java.nio.file.InvalidPathException;
3232
import java.nio.file.Path;
@@ -44,7 +44,7 @@ public class WindowsPathResourceIT {
4444

4545
@Test
4646
public void resolveWebInfOnWindowsDriveUri() throws Exception {
47-
assumeTrue("Windows-only PathResource URI regression", File.separatorChar == '\\');
47+
assumeTrue("Windows-only PathResource URI regression", OSUtil.isWindows());
4848

4949
final ResourceFactory resourceFactory = ResourceFactory.root();
5050
final Path webapp = Files.createTempDirectory("webapp");

0 commit comments

Comments
 (0)