From cec7aa26d8b3b24ca8eb8dc0bfdec19b3f00d691 Mon Sep 17 00:00:00 2001 From: David Smiley Date: Mon, 26 Aug 2024 13:12:48 -0400 Subject: [PATCH 1/3] Refactor: Use InputStream.transferTo Instead of longer code that accomplishes the same. --- .../java/org/apache/solr/cli/PostTool.java | 15 ++++------ .../org/apache/solr/handler/BlobHandler.java | 16 +++++++++-- .../DefaultSampleDocumentsLoader.java | 10 ++----- .../apache/solr/cloud/TestConfigSetsAPI.java | 28 +++---------------- .../apache/solr/util/TestCborDataFormat.java | 13 ++------- .../solr/common/util/BytesOutputStream.java | 1 + .../org/apache/solr/common/util/Utils.java | 23 ++------------- 7 files changed, 32 insertions(+), 74 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/cli/PostTool.java b/solr/core/src/java/org/apache/solr/cli/PostTool.java index 9c81486906f..32b7d3cba77 100644 --- a/solr/core/src/java/org/apache/solr/cli/PostTool.java +++ b/solr/core/src/java/org/apache/solr/cli/PostTool.java @@ -69,6 +69,7 @@ import org.apache.commons.cli.CommandLine; import org.apache.commons.cli.DeprecatedAttributes; import org.apache.commons.cli.Option; +import org.apache.commons.io.output.NullOutputStream; import org.apache.solr.client.api.util.SolrVersion; import org.apache.solr.client.solrj.SolrClient; import org.apache.solr.client.solrj.SolrServerException; @@ -1054,16 +1055,12 @@ public static InputStream stringToStream(String s) { * source and thrown away. */ private static void pipe(InputStream source, OutputStream dest) throws IOException { - byte[] buf = new byte[1024]; - int read = 0; - while ((read = source.read(buf)) >= 0) { - if (null != dest) { - dest.write(buf, 0, read); - } - } - if (null != dest) { - dest.flush(); + if (dest == null) { + dest = NullOutputStream.INSTANCE; } + // copy source to dest + source.transferTo(dest); + dest.flush(); } public FileFilter getFileFilterFromFileTypes(String fileTypes) { diff --git a/solr/core/src/java/org/apache/solr/handler/BlobHandler.java b/solr/core/src/java/org/apache/solr/handler/BlobHandler.java index ff59b99f624..003a9412741 100644 --- a/solr/core/src/java/org/apache/solr/handler/BlobHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/BlobHandler.java @@ -26,6 +26,7 @@ import java.io.InputStream; import java.io.OutputStream; import java.lang.invoke.MethodHandles; +import java.nio.BufferOverflowException; import java.nio.ByteBuffer; import java.security.MessageDigest; import java.util.ArrayList; @@ -34,6 +35,7 @@ import java.util.List; import java.util.Map; import org.apache.commons.codec.binary.Hex; +import org.apache.commons.io.input.BoundedInputStream; import org.apache.lucene.document.Document; import org.apache.lucene.index.IndexableField; import org.apache.lucene.index.Term; @@ -111,8 +113,8 @@ public void handleRequestBody(final SolrQueryRequest req, SolrQueryResponse rsp) for (ContentStream stream : req.getContentStreams()) { ByteBuffer payload; - try (InputStream is = stream.getStream()) { - payload = Utils.toByteArray(is, maxSize); + try (InputStream is = boundedInputStream(stream.getStream(), maxSize)) { + payload = Utils.toByteArray(is); } MessageDigest m = MessageDigest.getInstance("MD5"); m.update(payload.array(), payload.arrayOffset() + payload.position(), payload.limit()); @@ -261,6 +263,16 @@ public void write(OutputStream os) throws IOException { } } + private static InputStream boundedInputStream(final InputStream is, final long maxLength) + throws IOException { + return new BoundedInputStream(is, maxLength) { + @Override + protected void onMaxLength(long maxLength, long count) { + throw new BufferOverflowException(); + } + }; + } + private void verifyWithRealtimeGet( String blobName, long version, SolrQueryRequest req, Map doc) { for (; ; ) { diff --git a/solr/core/src/java/org/apache/solr/handler/designer/DefaultSampleDocumentsLoader.java b/solr/core/src/java/org/apache/solr/handler/designer/DefaultSampleDocumentsLoader.java index 17909b5771f..456f88bf8af 100644 --- a/solr/core/src/java/org/apache/solr/handler/designer/DefaultSampleDocumentsLoader.java +++ b/solr/core/src/java/org/apache/solr/handler/designer/DefaultSampleDocumentsLoader.java @@ -21,7 +21,6 @@ import static org.apache.solr.handler.loader.CSVLoaderBase.SEPARATOR; import java.io.BufferedReader; -import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; import java.io.Reader; @@ -65,14 +64,11 @@ public class DefaultSampleDocumentsLoader implements SampleDocumentsLoader { private static final int MAX_STREAM_SIZE = (5 * 1024 * 1024); private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); - public static byte[] streamAsBytes(final InputStream in) throws IOException { - ByteArrayOutputStream baos = new ByteArrayOutputStream(); - byte[] buf = new byte[1024]; - int r; + /** Reads input completely into a byte array. Closes the stream. */ + static byte[] streamAsBytes(final InputStream in) throws IOException { try (in) { - while ((r = in.read(buf)) != -1) baos.write(buf, 0, r); + return in.readAllBytes(); } - return baos.toByteArray(); } @Override diff --git a/solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java b/solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java index a45232a3af9..3bd405b8524 100644 --- a/solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java +++ b/solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java @@ -1754,14 +1754,7 @@ private static void zipWithForbiddenEndings(File fileOrDirectory, File zipfile) zout.putNextEntry(new ZipEntry("test." + fileType)); try (InputStream in = new FileInputStream(fileOrDirectory)) { - byte[] buffer = new byte[1024]; - while (true) { - int readCount = in.read(buffer); - if (readCount < 0) { - break; - } - zout.write(buffer, 0, readCount); - } + in.transferTo(zout); } zout.closeEntry(); @@ -1783,8 +1776,7 @@ private static void zip(File directory, File zipfile) throws IOException { Deque queue = new ArrayDeque<>(); queue.push(directory); OutputStream out = new FileOutputStream(zipfile); - ZipOutputStream zout = new ZipOutputStream(out); - try { + try (ZipOutputStream zout = new ZipOutputStream(out)) { while (!queue.isEmpty()) { directory = queue.pop(); for (File kid : directory.listFiles()) { @@ -1796,26 +1788,14 @@ private static void zip(File directory, File zipfile) throws IOException { } else { zout.putNextEntry(new ZipEntry(name)); - InputStream in = new FileInputStream(kid); - try { - byte[] buffer = new byte[1024]; - while (true) { - int readCount = in.read(buffer); - if (readCount < 0) { - break; - } - zout.write(buffer, 0, readCount); - } - } finally { - in.close(); + try (InputStream in = new FileInputStream(kid)) { + in.transferTo(zout); } zout.closeEntry(); } } } - } finally { - zout.close(); } } diff --git a/solr/core/src/test/org/apache/solr/util/TestCborDataFormat.java b/solr/core/src/test/org/apache/solr/util/TestCborDataFormat.java index be423b1a21e..cd4285b19fb 100644 --- a/solr/core/src/test/org/apache/solr/util/TestCborDataFormat.java +++ b/solr/core/src/test/org/apache/solr/util/TestCborDataFormat.java @@ -139,22 +139,13 @@ private byte[] runQuery(String testCollection, CloudSolrClient client, String wt request.setResponseParser(new InputStreamResponseParser(wt)); } result = client.request(request, testCollection); - byte[] b = copyStream((InputStream) result.get("stream")); + InputStream inputStream = (InputStream) result.get("stream"); + byte[] b = inputStream.readAllBytes(); System.out.println(wt + "_time : " + timer.getTime()); System.out.println(wt + "_size : " + b.length); return b; } - private static byte[] copyStream(InputStream inputStream) throws IOException { - ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); - byte[] buffer = new byte[4096]; - int bytesRead; - while ((bytesRead = inputStream.read(buffer)) != -1) { - outputStream.write(buffer, 0, bytesRead); - } - return outputStream.toByteArray(); - } - private void modifySchema(String testCollection, CloudSolrClient client) throws SolrServerException, IOException { GenericSolrRequest req = diff --git a/solr/solrj/src/java/org/apache/solr/common/util/BytesOutputStream.java b/solr/solrj/src/java/org/apache/solr/common/util/BytesOutputStream.java index b1b2bd45644..7a83241c684 100644 --- a/solr/solrj/src/java/org/apache/solr/common/util/BytesOutputStream.java +++ b/solr/solrj/src/java/org/apache/solr/common/util/BytesOutputStream.java @@ -22,6 +22,7 @@ import java.io.UnsupportedEncodingException; import java.util.Arrays; +@Deprecated public class BytesOutputStream extends OutputStream { private static final int MAX_ARRAY_SIZE = Integer.MAX_VALUE - 8; diff --git a/solr/solrj/src/java/org/apache/solr/common/util/Utils.java b/solr/solrj/src/java/org/apache/solr/common/util/Utils.java index 8a9aec1b80f..7102ab75598 100644 --- a/solr/solrj/src/java/org/apache/solr/common/util/Utils.java +++ b/solr/solrj/src/java/org/apache/solr/common/util/Utils.java @@ -1175,29 +1175,10 @@ public byte[] getbuf() { } } + /** Reads an input stream into a byte array. Does not close the input. */ public static ByteBuffer toByteArray(InputStream is) throws IOException { - return toByteArray(is, Integer.MAX_VALUE); - } - - /** - * Reads an input stream into a byte array - * - * @param is the input stream - * @return the byte array - * @throws IOException If there is a low-level I/O error. - */ - public static ByteBuffer toByteArray(InputStream is, long maxSize) throws IOException { try (BAOS bos = new BAOS()) { - long sz = 0; - int next = is.read(); - while (next > -1) { - if (++sz > maxSize) { - throw new BufferOverflowException(); - } - bos.write(next); - next = is.read(); - } - bos.flush(); + is.transferTo(bos); return bos.getByteBuffer(); } } From f7bbbfe52f90a4d2342b4e4ed15b8475eb52d5ec Mon Sep 17 00:00:00 2001 From: David Smiley Date: Tue, 27 Aug 2024 00:36:50 -0400 Subject: [PATCH 2/3] delete unused BytesOutputStream --- .../solr/common/util/BytesOutputStream.java | 136 ------------------ 1 file changed, 136 deletions(-) delete mode 100644 solr/solrj/src/java/org/apache/solr/common/util/BytesOutputStream.java diff --git a/solr/solrj/src/java/org/apache/solr/common/util/BytesOutputStream.java b/solr/solrj/src/java/org/apache/solr/common/util/BytesOutputStream.java deleted file mode 100644 index 7a83241c684..00000000000 --- a/solr/solrj/src/java/org/apache/solr/common/util/BytesOutputStream.java +++ /dev/null @@ -1,136 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.solr.common.util; - -import java.io.ByteArrayInputStream; -import java.io.InputStream; -import java.io.OutputStream; -import java.io.UnsupportedEncodingException; -import java.util.Arrays; - -@Deprecated -public class BytesOutputStream extends OutputStream { - private static final int MAX_ARRAY_SIZE = Integer.MAX_VALUE - 8; - - protected byte[] buf; - - protected int sz; - - public BytesOutputStream() { - this(64); - } - - public BytesOutputStream(int size) { - if (size < 0) { - throw new IllegalArgumentException("Size must be > 0: " + size); - } - buf = new byte[size]; - } - - public byte[] toBytes() { - return Arrays.copyOf(buf, sz); - } - - public Bytes bytes() { - return new Bytes(buf, 0, sz); - } - - public InputStream inputStream() { - return new ByteArrayInputStream(buf); - } - - private void ensureCapacity(int minCapacity) { - if (minCapacity - buf.length > 0) expandBuf(minCapacity); - } - - /** * Write a byte to the stream. */ - @Override - public void write(int b) { - - try { - buf[sz] = (byte) b; - sz += 1; - } catch (IndexOutOfBoundsException e) { - ensureCapacity(sz + 1); - buf[sz] = (byte) b; - sz += 1; - } - } - - @Override - public void write(byte[] b, int off, int len) { - try { - System.arraycopy(b, off, buf, sz, len); - sz += len; - } catch (IndexOutOfBoundsException e) { - ensureCapacity(sz + len); - System.arraycopy(b, off, buf, sz, len); - sz += len; - } - } - - public void writeBytes(byte[] b) { - write(b, 0, b.length); - } - - public void reset() { - sz = 0; - } - - public int size() { - return sz; - } - - public String toString(String charset) { - try { - return new String(buf, 0, sz, charset); - } catch (UnsupportedEncodingException e) { - throw new IllegalArgumentException(e); - } - } - - private void expandBuf(int minCapacity) { - int oldCapacity = buf.length; - int newCapacity = oldCapacity << 1; - if (newCapacity - minCapacity < 0) newCapacity = minCapacity; - if (newCapacity - MAX_ARRAY_SIZE > 0) { - if (minCapacity < 0) - // overflow - throw new OutOfMemoryError(); - newCapacity = (minCapacity > MAX_ARRAY_SIZE) ? Integer.MAX_VALUE : MAX_ARRAY_SIZE; - } - buf = Arrays.copyOf(buf, newCapacity); - } - - @Override - public void close() { - // noop - } - - public static class Bytes { - - public final byte[] bytes; - public final int offset; - public final int length; - - public Bytes(byte[] bytes, int offset, int length) { - this.bytes = bytes; - this.offset = offset; - this.length = length; - } - } -} From 12b1eb8892cbbca466ca5b2897f72322224cb1e8 Mon Sep 17 00:00:00 2001 From: David Smiley Date: Tue, 27 Aug 2024 01:22:07 -0400 Subject: [PATCH 3/3] Avoid awkwardness in closing the arg --- .../DefaultSampleDocumentsLoader.java | 20 +++++++------------ .../handler/designer/SchemaDesignerAPI.java | 7 ++++--- .../SchemaDesignerConfigSetHelper.java | 17 ++++++++++------ .../TestSchemaDesignerConfigSetHelper.java | 2 +- 4 files changed, 23 insertions(+), 23 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/handler/designer/DefaultSampleDocumentsLoader.java b/solr/core/src/java/org/apache/solr/handler/designer/DefaultSampleDocumentsLoader.java index 456f88bf8af..d35a76a75c6 100644 --- a/solr/core/src/java/org/apache/solr/handler/designer/DefaultSampleDocumentsLoader.java +++ b/solr/core/src/java/org/apache/solr/handler/designer/DefaultSampleDocumentsLoader.java @@ -22,7 +22,6 @@ import java.io.BufferedReader; import java.io.IOException; -import java.io.InputStream; import java.io.Reader; import java.io.StringReader; import java.lang.invoke.MethodHandles; @@ -64,10 +63,10 @@ public class DefaultSampleDocumentsLoader implements SampleDocumentsLoader { private static final int MAX_STREAM_SIZE = (5 * 1024 * 1024); private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); - /** Reads input completely into a byte array. Closes the stream. */ - static byte[] streamAsBytes(final InputStream in) throws IOException { - try (in) { - return in.readAllBytes(); + /** Reads all bytes from the stream. */ + private static byte[] readAllBytes(ContentStream cs) throws IOException { + try (var is = cs.getStream()) { + return is.readAllBytes(); } } @@ -97,7 +96,7 @@ public SampleDocuments parseDocsFromStream( fileSource = stream.getSourceInfo() != null ? stream.getSourceInfo() : "file"; } - byte[] uploadedBytes = streamAsBytes(stream.getStream()); + byte[] uploadedBytes = readAllBytes(stream); // recheck the upload size in case the stream returned null for getSize if (uploadedBytes.length > MAX_STREAM_SIZE) { throw new SolrException( @@ -204,8 +203,7 @@ protected List loadJsonDocs( String charset = ContentStreamBase.getCharsetFromContentType(stream.getContentType()); String jsonStr = new String( - streamAsBytes(stream.getStream()), - charset != null ? charset : ContentStreamBase.DEFAULT_CHARSET); + readAllBytes(stream), charset != null ? charset : ContentStreamBase.DEFAULT_CHARSET); String[] lines = jsonStr.split("\n"); if (lines.length > 1) { for (String line : lines) { @@ -235,7 +233,7 @@ protected List loadJsonDocs( protected List loadXmlDocs( SolrParams params, ContentStreamBase.ByteArrayStream stream, final int maxDocsToLoad) throws IOException { - String xmlString = readInputAsString(stream.getStream()).trim(); + String xmlString = new String(readAllBytes(stream), StandardCharsets.UTF_8).trim(); List docs; if (xmlString.contains("") && xmlString.contains("")) { XMLInputFactory inputFactory = XMLInputFactory.newInstance(); @@ -316,10 +314,6 @@ protected List> loadJsonLines(String[] lines) throws IOExcep return docs; } - protected String readInputAsString(InputStream in) throws IOException { - return new String(streamAsBytes(in), StandardCharsets.UTF_8); - } - protected char detectTSV(String csvStr) { char sep = ','; int endOfFirstLine = csvStr.indexOf('\n'); diff --git a/solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerAPI.java b/solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerAPI.java index a8a4b5091d4..f34793aa744 100644 --- a/solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerAPI.java +++ b/solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerAPI.java @@ -241,9 +241,10 @@ public void updateFileContents(SolrQueryRequest req, SolrQueryResponse rsp) "File '" + file + "' not found in configSet: " + configSet); } - byte[] data = - DefaultSampleDocumentsLoader.streamAsBytes( - extractSingleContentStream(req, true).getStream()); + byte[] data; + try (InputStream in = extractSingleContentStream(req, true).getStream()) { + data = in.readAllBytes(); + } Exception updateFileError = null; boolean requestIsTrusted = ConfigSetAPIBase.isTrusted(req.getUserPrincipal(), coreContainer.getAuthenticationPlugin()); diff --git a/solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerConfigSetHelper.java b/solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerConfigSetHelper.java index db35a805c51..a7618db74a9 100644 --- a/solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerConfigSetHelper.java +++ b/solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerConfigSetHelper.java @@ -65,6 +65,7 @@ import org.apache.http.client.utils.URLEncodedUtils; import org.apache.http.entity.ByteArrayEntity; import org.apache.http.util.EntityUtils; +import org.apache.lucene.util.IOSupplier; import org.apache.solr.client.solrj.SolrResponse; import org.apache.solr.client.solrj.SolrServerException; import org.apache.solr.client.solrj.impl.CloudLegacySolrClient; @@ -542,12 +543,12 @@ List getStoredSampleDocs(final String configSet) throws IOExc ((CloudLegacySolrClient) cloudClient()).getHttpClient().execute(httpGet); int statusCode = entity.getStatusLine().getStatusCode(); if (statusCode == HttpStatus.SC_OK) { - byte[] bytes = DefaultSampleDocumentsLoader.streamAsBytes(entity.getEntity().getContent()); + byte[] bytes = readAllBytes(() -> entity.getEntity().getContent()); if (bytes.length > 0) { docs = (List) Utils.fromJavabin(bytes); } } else if (statusCode != HttpStatus.SC_NOT_FOUND) { - byte[] bytes = DefaultSampleDocumentsLoader.streamAsBytes(entity.getEntity().getContent()); + byte[] bytes = readAllBytes(() -> entity.getEntity().getContent()); throw new IOException( "Failed to lookup stored docs for " + configSet @@ -562,10 +563,14 @@ List getStoredSampleDocs(final String configSet) throws IOExc void storeSampleDocs(final String configSet, List docs) throws IOException { docs.forEach(d -> d.removeField(VERSION_FIELD)); // remove _version_ field before storing ... - postDataToBlobStore( - cloudClient(), - configSet + "_sample", - DefaultSampleDocumentsLoader.streamAsBytes(toJavabin(docs))); + postDataToBlobStore(cloudClient(), configSet + "_sample", readAllBytes(() -> toJavabin(docs))); + } + + /** Gets the stream, reads all the bytes, closes the stream. */ + static byte[] readAllBytes(IOSupplier hasStream) throws IOException { + try (InputStream in = hasStream.get()) { + return in.readAllBytes(); + } } protected void postDataToBlobStore(CloudSolrClient cloudClient, String blobName, byte[] bytes) diff --git a/solr/core/src/test/org/apache/solr/handler/designer/TestSchemaDesignerConfigSetHelper.java b/solr/core/src/test/org/apache/solr/handler/designer/TestSchemaDesignerConfigSetHelper.java index 979ecd9ef0b..af6412da947 100644 --- a/solr/core/src/test/org/apache/solr/handler/designer/TestSchemaDesignerConfigSetHelper.java +++ b/solr/core/src/test/org/apache/solr/handler/designer/TestSchemaDesignerConfigSetHelper.java @@ -256,7 +256,7 @@ public void testPersistSampleDocs() throws Exception { helper.postDataToBlobStore( cluster.getSolrClient(), configSet + "_sample", - DefaultSampleDocumentsLoader.streamAsBytes(toJavabin(Collections.singletonList(doc)))); + SchemaDesignerConfigSetHelper.readAllBytes(() -> toJavabin(List.of(doc)))); List docs = helper.getStoredSampleDocs(configSet); assertTrue(docs != null && docs.size() == 1);