From 610a18e7b3bc9680031a2ba608b89ee6fdec795c Mon Sep 17 00:00:00 2001 From: Volkan Yazici Date: Thu, 12 Jun 2025 17:10:57 +0000 Subject: [PATCH] 8358688: HttpClient: Simplify file streaming in RequestPublishers.FilePublisher Reviewed-by: dfuchs, jpai --- .../internal/net/http/RequestPublishers.java | 44 +++++-------------- .../FilePublisher/FilePublisherTest.java | 35 +++++++++++---- .../net/httpclient/RelayingPublishers.java | 4 +- 3 files changed, 39 insertions(+), 44 deletions(-) diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/RequestPublishers.java b/src/java.net.http/share/classes/jdk/internal/net/http/RequestPublishers.java index 305957ae804..dd5443c5035 100644 --- a/src/java.net.http/share/classes/jdk/internal/net/http/RequestPublishers.java +++ b/src/java.net.http/share/classes/jdk/internal/net/http/RequestPublishers.java @@ -25,7 +25,6 @@ package jdk.internal.net.http; -import java.io.FileInputStream; import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; @@ -35,6 +34,7 @@ import java.net.http.HttpRequest.BodyPublisher; import java.nio.ByteBuffer; import java.nio.charset.Charset; import java.nio.file.Files; +import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.util.ArrayList; import java.util.Collections; @@ -48,7 +48,6 @@ import java.util.concurrent.Flow; import java.util.concurrent.Flow.Publisher; import java.util.concurrent.atomic.AtomicReference; import java.util.concurrent.locks.ReentrantLock; -import java.util.function.Function; import java.util.function.Supplier; import jdk.internal.net.http.common.Demand; @@ -228,30 +227,16 @@ public final class RequestPublishers { private final Path path; private final long length; - private final Function inputStreamSupplier; /** * Factory for creating FilePublisher. */ public static FilePublisher create(Path path) throws FileNotFoundException { - boolean defaultFS = true; - try { - path.toFile().getPath(); - } catch (UnsupportedOperationException uoe) { - // path not associated with the default file system provider - defaultFS = false; - } - - // existence check must be after FS checks if (Files.notExists(path)) throw new FileNotFoundException(path + " not found"); - boolean finalDefaultFS = defaultFS; - Function inputStreamSupplier = (p) -> - createInputStream(p, finalDefaultFS); - long length; try { length = Files.size(path); @@ -259,26 +244,12 @@ public final class RequestPublishers { length = -1; } - return new FilePublisher(path, length, inputStreamSupplier); + return new FilePublisher(path, length); } - private static InputStream createInputStream(Path path, - boolean defaultFS) { - try { - return defaultFS - ? new FileInputStream(path.toFile()) - : Files.newInputStream(path); - } catch (IOException io) { - throw new UncheckedIOException(io); - } - } - - private FilePublisher(Path name, - long length, - Function inputStreamSupplier) { + private FilePublisher(Path name, long length) { path = name; this.length = length; - this.inputStreamSupplier = inputStreamSupplier; } @Override @@ -286,7 +257,14 @@ public final class RequestPublishers { InputStream is = null; Throwable t = null; try { - is = inputStreamSupplier.apply(path); + // Throw `FileNotFoundException` to match the specification of `BodyPublishers::ofFile + if (!Files.isRegularFile(path)) { + throw new FileNotFoundException(path + " (Not a regular file)"); + } + is = Files.newInputStream(path); + } catch (NoSuchFileException nsfe) { + // Throw `FileNotFoundException` to match the specification of `BodyPublishers::ofFile` + t = new FileNotFoundException(path + " (No such file or directory)"); } catch (UncheckedIOException | UndeclaredThrowableException ue) { t = ue.getCause(); } catch (Throwable th) { diff --git a/test/jdk/java/net/httpclient/FilePublisher/FilePublisherTest.java b/test/jdk/java/net/httpclient/FilePublisher/FilePublisherTest.java index 8bb679a2e74..174afaf3f65 100644 --- a/test/jdk/java/net/httpclient/FilePublisher/FilePublisherTest.java +++ b/test/jdk/java/net/httpclient/FilePublisher/FilePublisherTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2020, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -23,9 +23,9 @@ /* * @test - * @bug 8235459 - * @summary Confirm that HttpRequest.BodyPublishers#ofFile(Path) - * assumes the default file system + * @bug 8235459 8358688 + * @summary Verifies `HttpRequest.BodyPublishers#ofFile(Path)` against file + * systems that support `Path#toFile()` and also those that don't * @library /test/lib /test/jdk/java/net/httpclient/lib * @build jdk.httpclient.test.lib.common.HttpServerAdapters * jdk.test.lib.net.SimpleSSLContext @@ -39,11 +39,10 @@ import org.testng.annotations.DataProvider; import org.testng.annotations.Test; import javax.net.ssl.SSLContext; +import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.net.InetAddress; -import java.net.InetSocketAddress; import java.net.URI; import java.net.http.HttpClient; import java.net.http.HttpRequest; @@ -61,6 +60,7 @@ import static java.net.http.HttpClient.Builder.NO_PROXY; import static java.net.http.HttpClient.Version.HTTP_1_1; import static java.net.http.HttpClient.Version.HTTP_2; import static org.testng.Assert.assertEquals; +import static org.testng.Assert.fail; public class FilePublisherTest implements HttpServerAdapters { SSLContext sslContext; @@ -156,6 +156,26 @@ public class FilePublisherTest implements HttpServerAdapters { send(uriString, path, expectedMsg, sameClient); } + @Test + public void testFileNotFound() throws Exception { + out.printf("\n\n--- testFileNotFound(): starting\n"); + try (FileSystem fs = newZipFs()) { + Path fileInZip = fs.getPath("non-existent.txt"); + BodyPublishers.ofFile(fileInZip); + fail(); + } catch (FileNotFoundException e) { + out.println("Caught expected: " + e); + } + var path = Path.of("fileNotFound.txt"); + try { + Files.deleteIfExists(path); + BodyPublishers.ofFile(path); + fail(); + } catch (FileNotFoundException e) { + out.println("Caught expected: " + e); + } + } + private static final int ITERATION_COUNT = 3; private void send(String uriString, @@ -193,9 +213,6 @@ public class FilePublisherTest implements HttpServerAdapters { zipFs = newZipFs(); zipFsPath = zipFsFile(zipFs); - InetSocketAddress sa = - new InetSocketAddress(InetAddress.getLoopbackAddress(), 0); - httpTestServer = HttpServerAdapters.HttpTestServer.create(HTTP_1_1); httpTestServer.addHandler(new HttpEchoHandler(), "/http1/echo"); httpURI = "http://" + httpTestServer.serverAuthority() + "/http1/echo"; diff --git a/test/jdk/java/net/httpclient/RelayingPublishers.java b/test/jdk/java/net/httpclient/RelayingPublishers.java index c97d09df352..83d74cd13ae 100644 --- a/test/jdk/java/net/httpclient/RelayingPublishers.java +++ b/test/jdk/java/net/httpclient/RelayingPublishers.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2019, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -42,7 +42,7 @@ import static org.testng.Assert.assertEquals; * @test * @summary Verifies that some of the standard BodyPublishers relay exception * rather than throw it - * @bug 8226303 + * @bug 8226303 8358688 * @library /test/lib * @run testng/othervm RelayingPublishers */