Add FileSystemUtil method to read 'file:/' URLs (#23020)

As part of #22116 we are going to forbid usage of api
java.net.URL#openStream(). However in a number of places across the
we use this method to read files from the local filesystem. This commit
introduces a helper method openFileURLStream(URL url) to read files
from URLs. It does specific validation to only ensure that file:/
urls are read.

Additionlly, this commit removes unneeded method
FileSystemUtil.newBufferedReader(URL, Charset). This method used the
openStream () method which will soon be forbidden. Instead we use the
Files.newBufferedReader(Path, Charset).
This commit is contained in:
Tim Brooks 2017-02-07 10:24:22 -06:00 committed by GitHub
parent c898e8ab83
commit 27b7d9bd8d
10 changed files with 80 additions and 33 deletions

View File

@ -19,6 +19,7 @@
package org.elasticsearch;
import org.elasticsearch.common.io.FileSystemUtils;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
@ -44,7 +45,7 @@ public class Build {
final URL url = getElasticsearchCodebase();
if (url.toString().endsWith(".jar")) {
try (JarInputStream jar = new JarInputStream(url.openStream())) {
try (JarInputStream jar = new JarInputStream(FileSystemUtils.openFileURLStream(url))) {
Manifest manifest = jar.getManifest();
shortHash = manifest.getMainAttributes().getValue("Change");
date = manifest.getMainAttributes().getValue("Build-Date");

View File

@ -21,14 +21,11 @@ package org.elasticsearch.common.io;
import org.apache.logging.log4j.Logger;
import org.apache.lucene.util.IOUtils;
import org.elasticsearch.common.Strings;
import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.io.Reader;
import java.io.InputStream;
import java.net.URL;
import java.nio.charset.Charset;
import java.nio.charset.CharsetDecoder;
import java.nio.file.DirectoryStream;
import java.nio.file.Files;
import java.nio.file.Path;
@ -120,14 +117,20 @@ public final class FileSystemUtils {
}
/**
* Opens the given url for reading returning a {@code BufferedReader} that may be
* used to read text from the URL in an efficient manner. Bytes from the
* file are decoded into characters using the specified charset.
* Returns an InputStream the given url if the url has a protocol of 'file', no host, and no port.
*/
public static BufferedReader newBufferedReader(URL url, Charset cs) throws IOException {
CharsetDecoder decoder = cs.newDecoder();
Reader reader = new InputStreamReader(url.openStream(), decoder);
return new BufferedReader(reader);
public static InputStream openFileURLStream(URL url) throws IOException {
String protocol = url.getProtocol();
if ("file".equals(protocol) == false) {
throw new IllegalArgumentException("Invalid protocol [" + protocol + "], must be [file]");
}
if (Strings.isEmpty(url.getHost()) == false) {
throw new IllegalArgumentException("URL cannot have host. Found: [" + url.getHost() + ']');
}
if (url.getPort() != -1) {
throw new IllegalArgumentException("URL cannot have port. Found: [" + url.getPort() + ']');
}
return url.openStream();
}
/**

View File

@ -67,6 +67,7 @@ import java.io.IOException;
import java.io.Reader;
import java.nio.charset.CharacterCodingException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Arrays;
@ -245,7 +246,7 @@ public class Analysis {
final Path path = env.configFile().resolve(wordListPath);
try (BufferedReader reader = FileSystemUtils.newBufferedReader(path.toUri().toURL(), StandardCharsets.UTF_8)) {
try (BufferedReader reader = Files.newBufferedReader(path, StandardCharsets.UTF_8)) {
return loadWordList(reader, "#");
} catch (CharacterCodingException ex) {
String message = String.format(Locale.ROOT,
@ -296,7 +297,7 @@ public class Analysis {
}
final Path path = env.configFile().resolve(filePath);
try {
return FileSystemUtils.newBufferedReader(path.toUri().toURL(), StandardCharsets.UTF_8);
return Files.newBufferedReader(path, StandardCharsets.UTF_8);
} catch (CharacterCodingException ex) {
String message = String.format(Locale.ROOT,
"Unsupported character encoding detected while reading %s_path: %s files must be UTF-8 encoded",

View File

@ -19,6 +19,7 @@
package org.elasticsearch;
import org.elasticsearch.common.io.FileSystemUtils;
import org.elasticsearch.test.ESTestCase;
import java.io.IOException;
@ -31,7 +32,7 @@ public class BuildTests extends ESTestCase {
public void testJarMetadata() throws IOException {
URL url = Build.getElasticsearchCodebase();
// throws exception if does not exist, or we cannot access it
try (InputStream ignored = url.openStream()) {}
try (InputStream ignored = FileSystemUtils.openFileURLStream(url)) {}
// these should never be null
assertNotNull(Build.CURRENT.date());
assertNotNull(Build.CURRENT.shortHash());

View File

@ -24,15 +24,16 @@ import org.elasticsearch.test.ESTestCase;
import org.junit.Before;
import java.io.IOException;
import java.io.InputStream;
import java.net.URISyntaxException;
import java.nio.charset.StandardCharsets;
import java.net.URL;
import java.nio.ByteBuffer;
import java.nio.channels.ByteChannel;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.StandardOpenOption;
import java.util.Arrays;
import static org.elasticsearch.common.io.FileTestUtils.assertFileContent;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertFileNotExists;
/**
* Unit tests for {@link org.elasticsearch.common.io.FileSystemUtils}.
*/
@ -41,6 +42,8 @@ public class FileSystemUtilsTests extends ESTestCase {
private Path src;
private Path dst;
private Path txtFile;
private byte[] expectedBytes;
@Before
public void copySourceFilesToTarget() throws IOException, URISyntaxException {
@ -48,6 +51,15 @@ public class FileSystemUtilsTests extends ESTestCase {
dst = createTempDir();
Files.createDirectories(src);
Files.createDirectories(dst);
txtFile = src.resolve("text-file.txt");
try (ByteChannel byteChannel = Files.newByteChannel(txtFile, StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE)) {
expectedBytes = new byte[3];
expectedBytes[0] = randomByte();
expectedBytes[1] = randomByte();
expectedBytes[2] = randomByte();
byteChannel.write(ByteBuffer.wrap(expectedBytes));
}
}
public void testAppend() {
@ -95,4 +107,34 @@ public class FileSystemUtilsTests extends ESTestCase {
assertTrue(FileSystemUtils.isHidden(path));
}
}
public void testOpenFileURLStream() throws IOException {
URL urlWithWrongProtocol = new URL("http://www.google.com");
try (InputStream is = FileSystemUtils.openFileURLStream(urlWithWrongProtocol)) {
fail("Should throw IllegalArgumentException due to invalid protocol");
} catch (IllegalArgumentException e) {
assertEquals("Invalid protocol [http], must be [file]", e.getMessage());
}
URL urlWithHost = new URL("file", "localhost", txtFile.toString());
try (InputStream is = FileSystemUtils.openFileURLStream(urlWithHost)) {
fail("Should throw IllegalArgumentException due to host");
} catch (IllegalArgumentException e) {
assertEquals("URL cannot have host. Found: [localhost]", e.getMessage());
}
URL urlWithPort = new URL("file", "", 80, txtFile.toString());
try (InputStream is = FileSystemUtils.openFileURLStream(urlWithPort)) {
fail("Should throw IllegalArgumentException due to port");
} catch (IllegalArgumentException e) {
assertEquals("URL cannot have port. Found: [80]", e.getMessage());
}
URL validUrl = txtFile.toUri().toURL();
try (InputStream is = FileSystemUtils.openFileURLStream(validUrl)) {
byte[] actualBytes = new byte[3];
is.read(actualBytes);
assertArrayEquals(expectedBytes, actualBytes);
}
}
}

View File

@ -46,6 +46,7 @@ import org.elasticsearch.client.HeapBufferedAsyncResponseConsumer;
import org.elasticsearch.client.RestClient;
import org.elasticsearch.common.ParsingException;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.io.FileSystemUtils;
import org.elasticsearch.common.io.Streams;
import org.elasticsearch.common.unit.ByteSizeUnit;
import org.elasticsearch.common.unit.ByteSizeValue;
@ -522,7 +523,7 @@ public class RemoteScrollableHitSourceTests extends ESTestCase {
} else {
StatusLine statusLine = new BasicStatusLine(protocolVersion, 200, "");
HttpResponse httpResponse = new BasicHttpResponse(statusLine);
httpResponse.setEntity(new InputStreamEntity(resource.openStream(), contentType));
httpResponse.setEntity(new InputStreamEntity(FileSystemUtils.openFileURLStream(resource), contentType));
futureCallback.completed(httpResponse);
}
return null;

View File

@ -28,6 +28,7 @@ import com.google.api.client.testing.http.MockLowLevelHttpRequest;
import com.google.api.client.testing.http.MockLowLevelHttpResponse;
import org.apache.logging.log4j.Logger;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.io.FileSystemUtils;
import org.elasticsearch.common.io.Streams;
import org.elasticsearch.common.logging.Loggers;
import org.elasticsearch.common.util.Callback;
@ -82,16 +83,10 @@ public class GceMockUtils {
if (resource == null) {
throw new IOException("can't read [" + url + "] in src/test/resources/org/elasticsearch/discovery/gce");
}
try (InputStream is = resource.openStream()) {
try (InputStream is = FileSystemUtils.openFileURLStream(resource)) {
final StringBuilder sb = new StringBuilder();
Streams.readAllLines(is, new Callback<String>() {
@Override
public void handle(String s) {
sb.append(s);
}
});
String response = sb.toString();
return response;
Streams.readAllLines(is, sb::append);
return sb.toString();
}
}
}

View File

@ -29,6 +29,7 @@ import org.elasticsearch.cli.Terminal;
import org.elasticsearch.cli.UserException;
import org.elasticsearch.common.SuppressForbidden;
import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.common.io.FileSystemUtils;
import org.elasticsearch.common.io.PathUtils;
import org.elasticsearch.common.io.PathUtilsForTesting;
import org.elasticsearch.common.settings.Settings;
@ -324,7 +325,7 @@ public class InstallPluginCommandTests extends ESTestCase {
Path pluginDir = createPluginDir(temp);
String pluginZip = createPlugin("fake", pluginDir);
Path pluginZipWithSpaces = createTempFile("foo bar", ".zip");
try (InputStream in = new URL(pluginZip).openStream()) {
try (InputStream in = FileSystemUtils.openFileURLStream(new URL(pluginZip))) {
Files.copy(in, pluginZipWithSpaces, StandardCopyOption.REPLACE_EXISTING);
}
installPlugin(pluginZipWithSpaces.toUri().toURL().toString(), env.v1());

View File

@ -24,6 +24,7 @@ import org.apache.lucene.util.LuceneTestCase;
import org.elasticsearch.SecureSM;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.SuppressForbidden;
import org.elasticsearch.common.io.FileSystemUtils;
import org.elasticsearch.common.io.PathUtils;
import org.elasticsearch.common.network.IfConfig;
import org.elasticsearch.plugins.PluginInfo;
@ -155,7 +156,7 @@ public class BootstrapForTesting {
// this just makes unit testing more realistic
for (URL url : Collections.list(BootstrapForTesting.class.getClassLoader().getResources(PluginInfo.ES_PLUGIN_PROPERTIES))) {
Properties properties = new Properties();
try (InputStream stream = url.openStream()) {
try (InputStream stream = FileSystemUtils.openFileURLStream(url)) {
properties.load(stream);
}
String clazz = properties.getProperty("classname");

View File

@ -30,6 +30,7 @@ import org.elasticsearch.client.RestClient;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.SuppressForbidden;
import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.common.io.FileSystemUtils;
import org.elasticsearch.common.xcontent.XContentHelper;
import org.elasticsearch.test.rest.ESRestTestCase;
import org.elasticsearch.test.rest.yaml.restspec.ClientYamlSuiteRestApi;
@ -250,7 +251,7 @@ public abstract class ESClientYamlSuiteTestCase extends ESRestTestCase {
// its checkWritable was incorrect and it won't work without write permissions.
// if we add the permission, it will open jars r/w, which is too scary! so copy to a safe r-w location.
Path tmp = Files.createTempFile(null, ".jar");
try (InputStream in = codeLocation.openStream()) {
try (InputStream in = FileSystemUtils.openFileURLStream(codeLocation)) {
Files.copy(in, tmp, StandardCopyOption.REPLACE_EXISTING);
}
return FileSystems.newFileSystem(new URI("jar:" + tmp.toUri()), Collections.emptyMap());