# HG changeset patch # User chegar # Date 1520431599 0 # Node ID 82a9340bdda6e4892105baeeed82833c403e9ccf # Parent 0fe17c3f9b4fda7642066524fa61c58750d61232 http-client-branch: rework file permissions to use limited doPriv diff -r 0fe17c3f9b4f -r 82a9340bdda6 src/java.net.http/share/classes/java/net/http/HttpRequest.java --- a/src/java.net.http/share/classes/java/net/http/HttpRequest.java Wed Mar 07 13:00:11 2018 +0000 +++ b/src/java.net.http/share/classes/java/net/http/HttpRequest.java Wed Mar 07 14:06:39 2018 +0000 @@ -28,23 +28,16 @@ import java.io.FileNotFoundException; import java.io.InputStream; import java.net.URI; -import java.net.URLPermission; import java.nio.ByteBuffer; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; -import java.nio.file.Files; import java.nio.file.Path; -import java.security.AccessController; -import java.security.PrivilegedAction; import java.time.Duration; import java.util.Iterator; import java.util.Objects; import java.util.Optional; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.Executor; import java.util.concurrent.Flow; import java.util.function.Supplier; -import java.net.http.HttpResponse.BodyHandler; import jdk.internal.net.http.HttpRequestBuilderImpl; import jdk.internal.net.http.RequestPublishers; import static java.nio.charset.StandardCharsets.UTF_8; @@ -616,10 +609,6 @@ return new RequestPublishers.ByteArrayPublisher(buf, offset, length); } - private static String pathForSecurityCheck(Path path) { - return path.toFile().getPath(); - } - /** * A request body publisher that takes data from the contents of a File. * @@ -636,12 +625,7 @@ */ public static BodyPublisher ofFile(Path path) throws FileNotFoundException { Objects.requireNonNull(path); - SecurityManager sm = System.getSecurityManager(); - if (sm != null) - sm.checkRead(pathForSecurityCheck(path)); - if (Files.notExists(path)) - throw new FileNotFoundException(path + " not found"); - return new RequestPublishers.FilePublisher(path); + return RequestPublishers.FilePublisher.create(path); } /** diff -r 0fe17c3f9b4f -r 82a9340bdda6 src/java.net.http/share/classes/java/net/http/HttpResponse.java --- a/src/java.net.http/share/classes/java/net/http/HttpResponse.java Wed Mar 07 13:00:11 2018 +0000 +++ b/src/java.net.http/share/classes/java/net/http/HttpResponse.java Wed Mar 07 14:06:39 2018 +0000 @@ -33,7 +33,6 @@ import java.nio.charset.Charset; import java.nio.channels.FileChannel; import java.nio.charset.StandardCharsets; -import java.nio.file.Files; import java.nio.file.OpenOption; import java.nio.file.Path; import java.util.List; @@ -163,11 +162,6 @@ public HttpClient.Version version(); - private static String pathForSecurityCheck(Path path) { - return path.toFile().getPath(); - } - - /** * A handler for response bodies. The class {@link BodyHandlers BodyHandlers} * provides implementations of many common body handlers. @@ -504,13 +498,7 @@ // these options make no sense, since the FileChannel is not exposed throw new IllegalArgumentException("invalid openOptions: " + opts); } - - SecurityManager sm = System.getSecurityManager(); - if (sm != null) { - String fn = pathForSecurityCheck(file); - sm.checkWrite(fn); - } - return new PathBodyHandler(file, opts); + return PathBodyHandler.create(file, opts); } /** @@ -561,9 +549,13 @@ * is not a directory, is not writable, or if an invalid set * of open options are specified * @throws SecurityException If a security manager has been installed - * and it denies {@linkplain SecurityManager#checkRead(String) - * read access} or {@linkplain SecurityManager#checkWrite(String) - * write access} to the directory. + * and it denies + * {@linkplain SecurityManager#checkRead(String) read access} + * to the directory, or it denies + * {@linkplain SecurityManager#checkWrite(String) write access} + * to the directory, or it denies + * {@linkplain SecurityManager#checkWrite(String) write access} + * to the files within the directory. */ public static BodyHandler ofFileDownload(Path directory, OpenOption... openOptions) { @@ -572,22 +564,7 @@ if (opts.contains(DELETE_ON_CLOSE)) { throw new IllegalArgumentException("invalid option: " + DELETE_ON_CLOSE); } - - SecurityManager sm = System.getSecurityManager(); - if (sm != null) { - String fn = pathForSecurityCheck(directory); - sm.checkWrite(fn); - sm.checkRead(fn); - } - - if (Files.notExists(directory)) - throw new IllegalArgumentException("non-existent directory: " + directory); - if (!Files.isDirectory(directory)) - throw new IllegalArgumentException("not a directory: " + directory); - if (!Files.isWritable(directory)) - throw new IllegalArgumentException("non-writable directory: " + directory); - - return new FileDownloadBodyHandler(directory, opts); + return FileDownloadBodyHandler.create(directory, opts); } /** @@ -1058,13 +1035,7 @@ // these options make no sense, since the FileChannel is not exposed throw new IllegalArgumentException("invalid openOptions: " + opts); } - - SecurityManager sm = System.getSecurityManager(); - if (sm != null) { - String fn = pathForSecurityCheck(file); - sm.checkWrite(fn); - } - return new PathSubscriber(file, opts); + return PathSubscriber.create(file, opts); } /** diff -r 0fe17c3f9b4f -r 82a9340bdda6 src/java.net.http/share/classes/jdk/internal/net/http/RequestPublishers.java --- a/src/java.net.http/share/classes/jdk/internal/net/http/RequestPublishers.java Wed Mar 07 13:00:11 2018 +0000 +++ b/src/java.net.http/share/classes/jdk/internal/net/http/RequestPublishers.java Wed Mar 07 14:06:39 2018 +0000 @@ -27,11 +27,14 @@ import java.io.File; import java.io.FileInputStream; +import java.io.FileNotFoundException; +import java.io.FilePermission; import java.io.IOException; import java.io.InputStream; import java.io.UncheckedIOException; import java.nio.ByteBuffer; import java.nio.charset.Charset; +import java.nio.file.Files; import java.nio.file.Path; import java.security.AccessControlContext; import java.security.AccessController; @@ -217,22 +220,72 @@ } } + /** + * Publishes the content of a given file. + * + * Privileged actions are performed within a limited doPrivileged that only + * asserts the specific, read, file permission that was checked during the + * construction of this FilePublisher. + */ public static class FilePublisher implements BodyPublisher { + + private static final FilePermission[] EMPTY_FILE_PERMISSIONS = new FilePermission[0]; + private final File file; + private final FilePermission[] filePermissions; + + private static String pathForSecurityCheck(Path path) { + return path.toFile().getPath(); + } - public FilePublisher(Path name) { + /** + * Factory for creating FilePublisher. + * + * Permission checks are performed here before construction of the + * FilePublisher. Permission checking and construction are deliberately + * and tightly co-located. + */ + public static FilePublisher create(Path path) throws FileNotFoundException { + FilePermission filePermission = null; + SecurityManager sm = System.getSecurityManager(); + if (sm != null) { + String fn = pathForSecurityCheck(path); + FilePermission readPermission = new FilePermission(fn, "read"); + sm.checkPermission(readPermission); + filePermission = readPermission; + } + + // existence check must be after permission checks + if (Files.notExists(path)) + throw new FileNotFoundException(path + " not found"); + + return new FilePublisher(path, filePermission); + } + + private FilePublisher(Path name, FilePermission filePermission) { + assert filePermission != null ? filePermission.getActions().equals("read") : true; file = name.toFile(); + this.filePermissions = filePermission == null ? EMPTY_FILE_PERMISSIONS + : new FilePermission[] { filePermission }; } @Override public void subscribe(Flow.Subscriber subscriber) { InputStream is; - try { - PrivilegedExceptionAction pa = - () -> new FileInputStream(file); - is = AccessController.doPrivileged(pa); - } catch (PrivilegedActionException pae) { - throw new UncheckedIOException((IOException)pae.getCause()); + if (System.getSecurityManager() == null) { + try { + is = new FileInputStream(file); + } catch (IOException ioe) { + throw new UncheckedIOException(ioe); + } + } else { + try { + PrivilegedExceptionAction pa = + () -> new FileInputStream(file); + is = AccessController.doPrivileged(pa, null, filePermissions); + } catch (PrivilegedActionException pae) { + throw new UncheckedIOException((IOException) pae.getCause()); + } } PullPublisher publisher = new PullPublisher<>(() -> new StreamIterator(is)); @@ -241,8 +294,12 @@ @Override public long contentLength() { - PrivilegedAction pa = () -> file.length(); - return AccessController.doPrivileged(pa); + if (System.getSecurityManager() == null) { + return file.length(); + } else { + PrivilegedAction pa = () -> file.length(); + return AccessController.doPrivileged(pa, null, filePermissions); + } } } diff -r 0fe17c3f9b4f -r 82a9340bdda6 src/java.net.http/share/classes/jdk/internal/net/http/ResponseBodyHandlers.java --- a/src/java.net.http/share/classes/jdk/internal/net/http/ResponseBodyHandlers.java Wed Mar 07 13:00:11 2018 +0000 +++ b/src/java.net.http/share/classes/jdk/internal/net/http/ResponseBodyHandlers.java Wed Mar 07 14:06:39 2018 +0000 @@ -25,13 +25,15 @@ package jdk.internal.net.http; +import java.io.File; +import java.io.FilePermission; import java.io.IOException; import java.io.UncheckedIOException; import java.net.URI; +import java.nio.file.Files; import java.nio.file.OpenOption; import java.nio.file.Path; import java.nio.file.Paths; -import java.nio.file.StandardOpenOption; import java.util.List; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentMap; @@ -50,21 +52,49 @@ private ResponseBodyHandlers() { } + private static final String pathForSecurityCheck(Path path) { + return path.toFile().getPath(); + } + /** * A Path body handler. */ public static class PathBodyHandler implements BodyHandler{ private final Path file; - private final List openOptions; + private final List openOptions; // immutable list + private final FilePermission filePermission; - public PathBodyHandler(Path file, List openOptions) { + /** + * Factory for creating PathBodyHandler. + * + * Permission checks are performed here before construction of the + * PathBodyHandler. Permission checking and construction are + * deliberately and tightly co-located. + */ + public static PathBodyHandler create(Path file, + List openOptions) { + FilePermission filePermission = null; + SecurityManager sm = System.getSecurityManager(); + if (sm != null) { + String fn = pathForSecurityCheck(file); + FilePermission writePermission = new FilePermission(fn, "write"); + sm.checkPermission(writePermission); + filePermission = writePermission; + } + return new PathBodyHandler(file, openOptions, filePermission); + } + + private PathBodyHandler(Path file, + List openOptions, + FilePermission filePermission) { this.file = file; this.openOptions = openOptions; + this.filePermission = filePermission; } @Override public BodySubscriber apply(int statusCode, HttpHeaders headers) { - return new PathSubscriber(file, openOptions); + return new PathSubscriber(file, openOptions, filePermission); } } @@ -118,10 +148,51 @@ public static class FileDownloadBodyHandler implements BodyHandler { private final Path directory; private final List openOptions; + private final FilePermission[] filePermissions; // may be null - public FileDownloadBodyHandler(Path directory, List openOptions) { + /** + * Factory for creating FileDownloadBodyHandler. + * + * Permission checks are performed here before construction of the + * FileDownloadBodyHandler. Permission checking and construction are + * deliberately and tightly co-located. + */ + public static FileDownloadBodyHandler create(Path directory, + List openOptions) { + FilePermission filePermissions[] = null; + SecurityManager sm = System.getSecurityManager(); + if (sm != null) { + String fn = pathForSecurityCheck(directory); + FilePermission writePermission = new FilePermission(fn, "write"); + String writePathPerm = fn + File.separatorChar + "*"; + FilePermission writeInDirPermission = new FilePermission(writePathPerm, "write"); + sm.checkPermission(writeInDirPermission); + FilePermission readPermission = new FilePermission(fn, "read"); + sm.checkPermission(readPermission); + + // read permission is only needed before determine the below checks + // only write permission is required when downloading to the file + filePermissions = new FilePermission[] { writePermission, writeInDirPermission }; + } + + // existence, etc, checks must be after permission checks + if (Files.notExists(directory)) + throw new IllegalArgumentException("non-existent directory: " + directory); + if (!Files.isDirectory(directory)) + throw new IllegalArgumentException("not a directory: " + directory); + if (!Files.isWritable(directory)) + throw new IllegalArgumentException("non-writable directory: " + directory); + + return new FileDownloadBodyHandler(directory, openOptions, filePermissions); + + } + + private FileDownloadBodyHandler(Path directory, + List openOptions, + FilePermission... filePermissions) { this.directory = directory; this.openOptions = openOptions; + this.filePermissions = filePermissions; } /** The "attachment" disposition-type and separator. */ @@ -204,7 +275,7 @@ "Resulting file, " + file.toString() + ", outside of given directory"); } - return new PathSubscriber(file, openOptions); + return new PathSubscriber(file, openOptions, filePermissions); } } } diff -r 0fe17c3f9b4f -r 82a9340bdda6 src/java.net.http/share/classes/jdk/internal/net/http/ResponseSubscribers.java --- a/src/java.net.http/share/classes/jdk/internal/net/http/ResponseSubscribers.java Wed Mar 07 13:00:11 2018 +0000 +++ b/src/java.net.http/share/classes/jdk/internal/net/http/ResponseSubscribers.java Wed Mar 07 14:06:39 2018 +0000 @@ -26,6 +26,7 @@ package jdk.internal.net.http; import java.io.BufferedReader; +import java.io.FilePermission; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; @@ -109,32 +110,80 @@ } + /** + * A Subscriber that writes the flow of data to a given file. + * + * Privileged actions are performed within a limited doPrivileged that only + * asserts the specific, write, file permissions that were checked during + * the construction of this PathSubscriber. + */ public static class PathSubscriber implements BodySubscriber { + private static final FilePermission[] EMPTY_FILE_PERMISSIONS = new FilePermission[0]; + private final Path file; + private final OpenOption[] options; + private final FilePermission[] filePermissions; private final CompletableFuture result = new MinimalFuture<>(); - private final OpenOption[] options; private volatile Flow.Subscription subscription; private volatile FileChannel out; - public PathSubscriber(Path file, List options) { + private static final String pathForSecurityCheck(Path path) { + return path.toFile().getPath(); + } + + /** + * Factory for creating PathSubscriber. + * + * Permission checks are performed here before construction of the + * PathSubscriber. Permission checking and construction are deliberately + * and tightly co-located. + */ + public static PathSubscriber create(Path file, + List options) { + FilePermission filePermission = null; + SecurityManager sm = System.getSecurityManager(); + if (sm != null) { + String fn = pathForSecurityCheck(file); + FilePermission writePermission = new FilePermission(fn, "write"); + sm.checkPermission(writePermission); + filePermission = writePermission; + } + return new PathSubscriber(file, options, filePermission); + } + + // pp so handler implementations in the same package can construct + /*package-private*/ PathSubscriber(Path file, + List options, + FilePermission... filePermissions) { this.file = file; this.options = options.stream().toArray(OpenOption[]::new); + this.filePermissions = + filePermissions == null ? EMPTY_FILE_PERMISSIONS : filePermissions; } @Override public void onSubscribe(Flow.Subscription subscription) { this.subscription = subscription; - try { - PrivilegedExceptionAction pa = - () -> FileChannel.open(file, options); - out = AccessController.doPrivileged(pa); - } catch (PrivilegedActionException pae) { - Throwable t = pae.getCause() != null ? pae.getCause() : pae; - result.completeExceptionally(t); - subscription.cancel(); - return; + if (System.getSecurityManager() == null) { + try { + out = FileChannel.open(file, options); + } catch (IOException ioe) { + result.completeExceptionally(ioe); + return; + } + } else { + try { + PrivilegedExceptionAction pa = + () -> FileChannel.open(file, options); + out = AccessController.doPrivileged(pa, null, filePermissions); + } catch (PrivilegedActionException pae) { + Throwable t = pae.getCause() != null ? pae.getCause() : pae; + result.completeExceptionally(t); + subscription.cancel(); + return; + } } subscription.request(1); } diff -r 0fe17c3f9b4f -r 82a9340bdda6 test/jdk/java/net/httpclient/AsFileDownloadTest.java --- a/test/jdk/java/net/httpclient/AsFileDownloadTest.java Wed Mar 07 13:00:11 2018 +0000 +++ b/test/jdk/java/net/httpclient/AsFileDownloadTest.java Wed Mar 07 14:06:39 2018 +0000 @@ -23,7 +23,7 @@ /* * @test - * @summary Basic test for asFileDownload + * @summary Basic test for ofFileDownload * @bug 8196965 * @modules java.base/sun.net.www.http * java.net.http/jdk.internal.net.http.common @@ -37,6 +37,7 @@ * @build jdk.test.lib.Platform * @build jdk.test.lib.util.FileUtils * @run testng/othervm AsFileDownloadTest + * @run testng/othervm/java.security.policy=AsFileDownloadTest.policy AsFileDownloadTest */ import com.sun.net.httpserver.HttpExchange; @@ -61,6 +62,9 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.logging.ConsoleHandler; +import java.util.logging.Level; +import java.util.logging.Logger; import javax.net.ssl.SSLContext; import jdk.testlibrary.SimpleSSLContext; import jdk.test.lib.util.FileUtils; @@ -251,7 +255,9 @@ @BeforeTest public void setup() throws Exception { tempDir = Paths.get("asFileDownloadTest.tmp.dir"); - FileUtils.deleteFileIfExistsWithRetry(tempDir); + if (Files.exists(tempDir)) + throw new AssertionError("Unexpected test work dir existence: " + tempDir.toString()); + Files.createDirectory(tempDir); // Unique dirs per test run, based on the URI path Files.createDirectories(tempDir.resolve("http1/afdt/")); @@ -259,6 +265,13 @@ Files.createDirectories(tempDir.resolve("http2/afdt/")); Files.createDirectories(tempDir.resolve("https2/afdt/")); + // HTTP/1.1 server logging in case of security exceptions ( uncomment if needed ) + //Logger logger = Logger.getLogger("com.sun.net.httpserver"); + //ConsoleHandler ch = new ConsoleHandler(); + //logger.setLevel(Level.ALL); + //ch.setLevel(Level.ALL); + //logger.addHandler(ch); + sslContext = new SimpleSSLContext().get(); if (sslContext == null) throw new AssertionError("Unexpected null sslContext"); @@ -295,6 +308,11 @@ httpsTestServer.stop(0); http2TestServer.stop(); https2TestServer.stop(); + + if (System.getSecurityManager() == null && Files.exists(tempDir)) { + // clean up before next run with security manager + FileUtils.deleteFileTreeWithRetry(tempDir); + } } static String contentDispositionValueFromURI(URI uri) { diff -r 0fe17c3f9b4f -r 82a9340bdda6 test/jdk/java/net/httpclient/RequestBodyTest.java --- a/test/jdk/java/net/httpclient/RequestBodyTest.java Wed Mar 07 13:00:11 2018 +0000 +++ b/test/jdk/java/net/httpclient/RequestBodyTest.java Wed Mar 07 14:06:39 2018 +0000 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, 2017, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2015, 2018, 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 @@ -31,11 +31,12 @@ * @compile ../../../com/sun/net/httpserver/LogFilter.java * @compile ../../../com/sun/net/httpserver/EchoHandler.java * @compile ../../../com/sun/net/httpserver/FileServerHandler.java + * @build jdk.testlibrary.SimpleSSLContext + * @build LightWeightHttpServer * @build jdk.test.lib.Platform * @build jdk.test.lib.util.FileUtils - * @build LightWeightHttpServer - * @build jdk.testlibrary.SimpleSSLContext * @run testng/othervm RequestBodyTest + * @run testng/othervm/java.security.policy=RequestBodyTest.policy RequestBodyTest */ import java.io.*; @@ -51,6 +52,9 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.security.AccessController; +import java.security.PrivilegedActionException; +import java.security.PrivilegedExceptionAction; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -328,9 +332,11 @@ @Override public FileInputStream get() { try { - return new FileInputStream(file.toFile()); - } catch (FileNotFoundException x) { - throw new UncheckedIOException(x); + PrivilegedExceptionAction pa = + () -> new FileInputStream(file.toFile()); + return AccessController.doPrivileged(pa); + } catch (PrivilegedActionException x) { + throw new UncheckedIOException((IOException)x.getCause()); } } }; diff -r 0fe17c3f9b4f -r 82a9340bdda6 test/jdk/java/net/httpclient/security/filePerms/FileProcessorPermissionTest.java --- a/test/jdk/java/net/httpclient/security/filePerms/FileProcessorPermissionTest.java Wed Mar 07 13:00:11 2018 +0000 +++ b/test/jdk/java/net/httpclient/security/filePerms/FileProcessorPermissionTest.java Wed Mar 07 14:06:39 2018 +0000 @@ -24,9 +24,10 @@ /* * @test * @summary Basic checks for SecurityException from body processors APIs - * @run testng/othervm/java.security.policy=httpclient.policy FileProcessorPermissionTest + * @run testng/othervm/java.security.policy=allpermissions.policy FileProcessorPermissionTest */ +import java.io.File; import java.io.FilePermission; import java.nio.file.Path; import java.nio.file.Paths; @@ -39,7 +40,9 @@ import java.security.ProtectionDomain; import java.util.List; import java.net.http.HttpRequest; +import java.net.http.HttpRequest.BodyPublishers; import java.net.http.HttpResponse; +import java.net.http.HttpResponse.BodyHandlers; import org.testng.annotations.Test; import static java.nio.file.StandardOpenOption.*; import static org.testng.Assert.*; @@ -70,16 +73,16 @@ List> list = List.of( () -> HttpRequest.BodyPublishers.ofFile(fromFilePath), - () -> HttpResponse.BodyHandlers.ofFile(asFilePath), - () -> HttpResponse.BodyHandlers.ofFile(asFilePath, CREATE), - () -> HttpResponse.BodyHandlers.ofFile(asFilePath, CREATE, WRITE), + () -> BodyHandlers.ofFile(asFilePath), + () -> BodyHandlers.ofFile(asFilePath, CREATE), + () -> BodyHandlers.ofFile(asFilePath, CREATE, WRITE), - () -> HttpResponse.BodyHandlers.ofFileDownload(CWD), - () -> HttpResponse.BodyHandlers.ofFileDownload(CWD, CREATE), - () -> HttpResponse.BodyHandlers.ofFileDownload(CWD, CREATE, WRITE) + () -> BodyHandlers.ofFileDownload(CWD), + () -> BodyHandlers.ofFileDownload(CWD, CREATE), + () -> BodyHandlers.ofFileDownload(CWD, CREATE, WRITE) ); - // sanity, just run http ( no security manager ) + // TEST 1 - sanity, just run ( no security manager ) System.setSecurityManager(null); try { for (PrivilegedExceptionAction pa : list) { @@ -100,11 +103,27 @@ } } - // Run with limited permissions, i.e. just what is required + // TEST 2 - with all file permissions + AccessControlContext allFilesACC = withPermissions( + new FilePermission("<>" , "read,write") + ); + for (PrivilegedExceptionAction pa : list) { + try { + assert System.getSecurityManager() != null; + AccessController.doPrivileged(pa, allFilesACC); + } catch (PrivilegedActionException pae) { + fail("UNEXPECTED Exception:" + pae); + pae.printStackTrace(); + } + } + + // TEST 3 - with limited permissions, i.e. just what is required AccessControlContext minimalACC = withPermissions( new FilePermission(fromFilePath.toString() , "read"), - new FilePermission(asFilePath.toString(), "read,write,delete"), - new FilePermission(CWD.toString(), "read,write,delete") + new FilePermission(asFilePath.toString(), "write"), + // ofFileDownload requires read and write to the dir + new FilePermission(CWD.toString(), "read,write"), + new FilePermission(CWD.toString() + File.separator + "*", "read,write") ); for (PrivilegedExceptionAction pa : list) { try { @@ -116,7 +135,7 @@ } } - // Run with NO permissions, i.e. expect SecurityException + // TEST 4 - with NO permissions, i.e. expect SecurityException for (PrivilegedExceptionAction pa : list) { try { assert System.getSecurityManager() != null; diff -r 0fe17c3f9b4f -r 82a9340bdda6 test/jdk/java/net/httpclient/security/filePerms/SecurityBeforeFile.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/jdk/java/net/httpclient/security/filePerms/SecurityBeforeFile.java Wed Mar 07 14:06:39 2018 +0000 @@ -0,0 +1,94 @@ +/* + * Copyright (c) 2018, 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 + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test + * @summary Verifies security checks are performed before existence checks + * in pre-defined body processors APIs + * @run testng/othervm SecurityBeforeFile + * @run testng/othervm/java.security.policy=nopermissions.policy SecurityBeforeFile + */ + +import java.io.FileNotFoundException; +import java.nio.file.Files; +import java.nio.file.OpenOption; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.net.http.HttpRequest.BodyPublishers; +import java.net.http.HttpResponse.BodyHandlers; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; +import static java.lang.System.out; +import static java.nio.file.StandardOpenOption.*; +import static org.testng.Assert.*; + +public class SecurityBeforeFile { + + static final boolean hasSecurityManager = System.getSecurityManager() != null; + static final boolean hasNoSecurityManager = !hasSecurityManager; + + @Test + public void BodyPublishersOfFile() { + Path p = Paths.get("doesNotExist.txt"); + if (hasNoSecurityManager && Files.exists(p)) + throw new AssertionError("Unexpected " + p); + + try { + BodyPublishers.ofFile(p); + fail("UNEXPECTED, file " + p.toString() + " exists?"); + } catch (SecurityException se) { + assertTrue(hasSecurityManager); + out.println("caught expected security exception: " + se); + } catch (FileNotFoundException fnfe) { + assertTrue(hasNoSecurityManager); + out.println("caught expected file not found exception: " + fnfe); + } + } + + @DataProvider(name = "handlerOpenOptions") + public Object[][] handlerOpenOptions() { + return new Object[][] { + { new OpenOption[] { } }, + { new OpenOption[] { CREATE } }, + { new OpenOption[] { CREATE, WRITE } }, + }; + } + + @Test(dataProvider = "handlerOpenOptions") + public void BodyHandlersOfFileDownload(OpenOption[] openOptions) { + Path p = Paths.get("doesNotExistDir"); + if (hasNoSecurityManager && Files.exists(p)) + throw new AssertionError("Unexpected " + p); + + try { + BodyHandlers.ofFileDownload(p, openOptions); + fail("UNEXPECTED, file " + p.toString() + " exists?"); + } catch (SecurityException se) { + assertTrue(hasSecurityManager); + out.println("caught expected security exception: " + se); + } catch (IllegalArgumentException iae) { + assertTrue(hasNoSecurityManager); + out.println("caught expected illegal argument exception: " + iae); + } + } +} diff -r 0fe17c3f9b4f -r 82a9340bdda6 test/jdk/java/net/httpclient/security/filePerms/allpermissions.policy --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/jdk/java/net/httpclient/security/filePerms/allpermissions.policy Wed Mar 07 14:06:39 2018 +0000 @@ -0,0 +1,28 @@ +// +// Copyright (c) 2017, 2018, 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 +// under the terms of the GNU General Public License version 2 only, as +// published by the Free Software Foundation. +// +// This code is distributed in the hope that it will be useful, but WITHOUT +// ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +// FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License +// version 2 for more details (a copy is included in the LICENSE file that +// accompanied this code). +// +// You should have received a copy of the GNU General Public License version +// 2 along with this work; if not, write to the Free Software Foundation, +// Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. +// +// Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA +// or visit www.oracle.com if you need additional information or have any +// questions. +// + +// bootstrap to get the test going, it will do its own restrictions +grant codeBase "file:${test.classes}/*" { + permission java.security.AllPermission; +}; + diff -r 0fe17c3f9b4f -r 82a9340bdda6 test/jdk/java/net/httpclient/security/filePerms/httpclient.policy --- a/test/jdk/java/net/httpclient/security/filePerms/httpclient.policy Wed Mar 07 13:00:11 2018 +0000 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,28 +0,0 @@ -// -// Copyright (c) 2017, 2018, 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 -// under the terms of the GNU General Public License version 2 only, as -// published by the Free Software Foundation. -// -// This code is distributed in the hope that it will be useful, but WITHOUT -// ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or -// FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License -// version 2 for more details (a copy is included in the LICENSE file that -// accompanied this code). -// -// You should have received a copy of the GNU General Public License version -// 2 along with this work; if not, write to the Free Software Foundation, -// Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. -// -// Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA -// or visit www.oracle.com if you need additional information or have any -// questions. -// - -// bootstrap to get the test going, it will do its own restrictions -grant codeBase "file:${test.classes}/*" { - permission java.security.AllPermission; -}; - diff -r 0fe17c3f9b4f -r 82a9340bdda6 test/jdk/lib/testlibrary/jdk/testlibrary/SimpleSSLContext.java --- a/test/jdk/lib/testlibrary/jdk/testlibrary/SimpleSSLContext.java Wed Mar 07 13:00:11 2018 +0000 +++ b/test/jdk/lib/testlibrary/jdk/testlibrary/SimpleSSLContext.java Wed Mar 07 14:06:39 2018 +0000 @@ -53,27 +53,44 @@ * source directory */ public SimpleSSLContext() throws IOException { - String paths = System.getProperty("test.src.path"); - StringTokenizer st = new StringTokenizer(paths, File.pathSeparator); - boolean securityExceptions = false; - while (st.hasMoreTokens()) { - String path = st.nextToken(); - try { - File f = new File(path, "jdk/testlibrary/testkeys"); - if (f.exists()) { - try (FileInputStream fis = new FileInputStream(f)) { - init(fis); - return; + try { + AccessController.doPrivileged(new PrivilegedExceptionAction() { + @Override + public Void run() throws Exception { + String paths = System.getProperty("test.src.path"); + StringTokenizer st = new StringTokenizer(paths, File.pathSeparator); + boolean securityExceptions = false; + while (st.hasMoreTokens()) { + String path = st.nextToken(); + try { + File f = new File(path, "jdk/testlibrary/testkeys"); + if (f.exists()) { + try (FileInputStream fis = new FileInputStream(f)) { + init(fis); + return null; + } + } + } catch (SecurityException e) { + // catch and ignore because permission only required + // for one entry on path (at most) + securityExceptions = true; + } } + if (securityExceptions) { + System.err.println("SecurityExceptions thrown on loading testkeys"); + } + return null; } - } catch (SecurityException e) { - // catch and ignore because permission only required - // for one entry on path (at most) - securityExceptions = true; - } - } - if (securityExceptions) { - System.err.println("SecurityExceptions thrown on loading testkeys"); + }); + } catch (PrivilegedActionException pae) { + Throwable t = pae.getCause() != null ? pae.getCause() : pae; + if (t instanceof IOException) + throw (IOException)t; + if (t instanceof RuntimeException) + throw (RuntimeException)t; + if (t instanceof Error) + throw (Error)t; + throw new RuntimeException(t); } }