diff --git a/changelog/unreleased/PR#4606-charset-api-modernization.yml b/changelog/unreleased/PR#4606-charset-api-modernization.yml new file mode 100644 index 000000000000..79b724b022df --- /dev/null +++ b/changelog/unreleased/PR#4606-charset-api-modernization.yml @@ -0,0 +1,8 @@ +# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc +title: Modernize String-based charset APIs to java.nio.charset.Charset; invalid client-supplied charset names are still handled as I/O errors +type: other +authors: + - name: Jan Høydahl +links: +- name: PR#4606 + url: https://github.com/apache/solr/pull/4606 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 785be168ca93..fa796fc0709c 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 @@ -43,6 +43,7 @@ import org.apache.solr.common.params.SolrParams; import org.apache.solr.common.util.ContentStream; import org.apache.solr.common.util.ContentStreamBase; +import org.apache.solr.common.util.IOUtils; import org.apache.solr.handler.loader.CSVLoaderBase; import org.apache.solr.handler.loader.JsonLoader; import org.apache.solr.handler.loader.XMLLoader; @@ -139,7 +140,7 @@ protected List loadCsvDocs( throws IOException { ContentStream stream; if (params.get(SEPARATOR) == null) { - String csvStr = new String(streamBytes, charset); + String csvStr = new String(streamBytes, IOUtils.charsetForName(charset)); char sep = detectTSV(csvStr); ModifiableSolrParams modifiableSolrParams = new ModifiableSolrParams(params); modifiableSolrParams.set(SEPARATOR, String.valueOf(sep)); @@ -198,7 +199,8 @@ protected List loadJsonDocs( String charset = ContentStreamBase.getCharsetFromContentType(stream.getContentType()); String jsonStr = new String( - readAllBytes(stream), charset != null ? charset : ContentStreamBase.DEFAULT_CHARSET); + readAllBytes(stream), + charset != null ? IOUtils.charsetForName(charset) : StandardCharsets.UTF_8); String[] lines = jsonStr.split("\n"); if (lines.length > 1) { for (String line : lines) { diff --git a/solr/core/src/java/org/apache/solr/handler/loader/XMLLoader.java b/solr/core/src/java/org/apache/solr/handler/loader/XMLLoader.java index 72d93f87b78d..a6490299d5ac 100644 --- a/solr/core/src/java/org/apache/solr/handler/loader/XMLLoader.java +++ b/solr/core/src/java/org/apache/solr/handler/loader/XMLLoader.java @@ -23,6 +23,7 @@ import java.io.IOException; import java.io.InputStream; import java.lang.invoke.MethodHandles; +import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; @@ -45,6 +46,7 @@ import org.apache.solr.common.util.CollectionUtil; import org.apache.solr.common.util.ContentStream; import org.apache.solr.common.util.ContentStreamBase; +import org.apache.solr.common.util.IOUtils; import org.apache.solr.common.util.StrUtils; import org.apache.solr.common.util.XMLErrorLogger; import org.apache.solr.handler.RequestHandlerUtils; @@ -134,7 +136,9 @@ private InputStream getStream(ContentStream cs, String charset) throws IOExcepti if (log.isTraceEnabled()) { log.trace( "body: {}", - new String(body, (charset == null) ? ContentStreamBase.DEFAULT_CHARSET : charset)); + new String( + body, + (charset == null) ? StandardCharsets.UTF_8 : IOUtils.charsetForName(charset))); } return new ByteArrayInputStream(body); } diff --git a/solr/core/src/java/org/apache/solr/response/TextQueryResponseWriter.java b/solr/core/src/java/org/apache/solr/response/TextQueryResponseWriter.java index f4df38f830d9..34b464864a27 100644 --- a/solr/core/src/java/org/apache/solr/response/TextQueryResponseWriter.java +++ b/solr/core/src/java/org/apache/solr/response/TextQueryResponseWriter.java @@ -26,6 +26,7 @@ import java.nio.charset.StandardCharsets; import org.apache.solr.common.util.ContentStreamBase; import org.apache.solr.common.util.FastWriter; +import org.apache.solr.common.util.IOUtils; import org.apache.solr.request.SolrQueryRequest; /** A writer supporting character streams ({@link Writer} based). */ @@ -70,7 +71,7 @@ private static Writer buildWriter(OutputStream outputStream, String charset) Writer writer = (charset == null) ? new OutputStreamWriter(outputStream, StandardCharsets.UTF_8) - : new OutputStreamWriter(outputStream, charset); + : new OutputStreamWriter(outputStream, IOUtils.charsetForName(charset)); return new FastWriter(writer); // note: buffered; therefore we need to call flush() } diff --git a/solr/core/src/java/org/apache/solr/spelling/FileBasedSpellChecker.java b/solr/core/src/java/org/apache/solr/spelling/FileBasedSpellChecker.java index 2300f0a8dca4..e3bf30127da1 100644 --- a/solr/core/src/java/org/apache/solr/spelling/FileBasedSpellChecker.java +++ b/solr/core/src/java/org/apache/solr/spelling/FileBasedSpellChecker.java @@ -33,6 +33,7 @@ import org.apache.lucene.search.spell.PlainTextDictionary; import org.apache.lucene.store.ByteBuffersDirectory; import org.apache.lucene.store.Directory; +import org.apache.solr.common.util.IOUtils; import org.apache.solr.common.util.NamedList; import org.apache.solr.core.SolrCore; import org.apache.solr.schema.FieldType; @@ -122,7 +123,8 @@ private void loadExternalFileDictionary(SolrCore core, SolrIndexSearcher searche dictionary = new PlainTextDictionary( new InputStreamReader( - core.getResourceLoader().openResource(sourceLocation), characterEncoding)); + core.getResourceLoader().openResource(sourceLocation), + IOUtils.charsetForName(characterEncoding))); } } diff --git a/solr/core/src/test/org/apache/solr/rest/schema/analysis/TestManagedSynonymGraphFilterFactory.java b/solr/core/src/test/org/apache/solr/rest/schema/analysis/TestManagedSynonymGraphFilterFactory.java index 9bb95fc52a5a..dc58da73485f 100644 --- a/solr/core/src/test/org/apache/solr/rest/schema/analysis/TestManagedSynonymGraphFilterFactory.java +++ b/solr/core/src/test/org/apache/solr/rest/schema/analysis/TestManagedSynonymGraphFilterFactory.java @@ -20,6 +20,7 @@ import static org.apache.solr.common.util.Utils.toJSONString; import java.net.URLEncoder; +import java.nio.charset.StandardCharsets; import java.nio.file.Path; import java.util.ArrayList; import java.util.Arrays; @@ -150,7 +151,10 @@ public void testManagedSynonyms() throws Exception { "/response/result[@name='response'][@numFound='1']", "/response/result[@name='response']/doc/str[@name='id'][.='5150']"); assertQ( - "/select?q=" + newFieldName + ":" + URLEncoder.encode(multiTermOrigin, "UTF-8"), + "/select?q=" + + newFieldName + + ":" + + URLEncoder.encode(multiTermOrigin, StandardCharsets.UTF_8), "/response/lst[@name='responseHeader']/int[@name='status'] = '0'", "/response/result[@name='response'][@numFound='1']", "/response/result[@name='response']/doc/str[@name='id'][.='040']"); @@ -168,7 +172,7 @@ public void testManagedSynonyms() throws Exception { assertJPut(endpoint, toJSONString(syns), "/responseHeader/status==0"); assertJQ( - endpoint + "/" + URLEncoder.encode(multiTermSynonym, "UTF-8"), + endpoint + "/" + URLEncoder.encode(multiTermSynonym, StandardCharsets.UTF_8), "/" + multiTermSynonym + "==['" + multiTermOrigin + "']"); // should not match as the synonym mapping between mad and angry does not @@ -184,7 +188,7 @@ public void testManagedSynonyms() throws Exception { "/select?q=" + newFieldName + ":(" - + URLEncoder.encode(multiTermSynonym, "UTF-8") + + URLEncoder.encode(multiTermSynonym, StandardCharsets.UTF_8) + ")&sow=false", "/response/lst[@name='responseHeader']/int[@name='status'] = '0'", "/response/result[@name='response'][@numFound='0']"); @@ -203,7 +207,7 @@ public void testManagedSynonyms() throws Exception { "/select?q=" + newFieldName + ":(" - + URLEncoder.encode(multiTermSynonym, "UTF-8") + + URLEncoder.encode(multiTermSynonym, StandardCharsets.UTF_8) + ")&sow=false", "/response/lst[@name='responseHeader']/int[@name='status'] = '0'", "/response/result[@name='response'][@numFound='1']", diff --git a/solr/modules/cross-dc/src/test/org/apache/solr/crossdc/common/ConfUtilTest.java b/solr/modules/cross-dc/src/test/org/apache/solr/crossdc/common/ConfUtilTest.java index 49de459b288a..28a98d6d7a1e 100644 --- a/solr/modules/cross-dc/src/test/org/apache/solr/crossdc/common/ConfUtilTest.java +++ b/solr/modules/cross-dc/src/test/org/apache/solr/crossdc/common/ConfUtilTest.java @@ -22,6 +22,7 @@ import java.io.ByteArrayOutputStream; import java.io.OutputStreamWriter; +import java.nio.charset.StandardCharsets; import java.util.HashMap; import java.util.Map; import java.util.Properties; @@ -129,7 +130,7 @@ public void testFillProperties_FromZooKeeper() throws Exception { zkProps.setProperty("custom.zk.property", "zk-value"); ByteArrayOutputStream baos = new ByteArrayOutputStream(); - OutputStreamWriter writer = new OutputStreamWriter(baos, "UTF-8"); + OutputStreamWriter writer = new OutputStreamWriter(baos, StandardCharsets.UTF_8); zkProps.store(writer, null); writer.close(); byte[] zkData = baos.toByteArray(); @@ -155,7 +156,7 @@ public void testFillProperties_PriorityOrder() throws Exception { zkProps.setProperty(KafkaCrossDcConf.TOPIC_NAME, "zk-topic"); ByteArrayOutputStream baos = new ByteArrayOutputStream(); - OutputStreamWriter writer = new OutputStreamWriter(baos, "UTF-8"); + OutputStreamWriter writer = new OutputStreamWriter(baos, StandardCharsets.UTF_8); zkProps.store(writer, null); writer.close(); byte[] zkData = baos.toByteArray(); @@ -318,7 +319,7 @@ public void testFillProperties_ComplexScenario() throws Exception { zkProps.setProperty("zk.only.property", "zk-value"); ByteArrayOutputStream baos = new ByteArrayOutputStream(); - OutputStreamWriter writer = new OutputStreamWriter(baos, "UTF-8"); + OutputStreamWriter writer = new OutputStreamWriter(baos, StandardCharsets.UTF_8); zkProps.store(writer, null); writer.close(); byte[] zkData = baos.toByteArray(); diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java index bcf5eb6b3c17..fccc4357fcde 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java @@ -290,9 +290,11 @@ private void checkContentType( try { ByteArrayOutputStream body = new ByteArrayOutputStream(); is.transferTo(body); + Charset charset = encoding != null ? Charset.forName(encoding) : FALLBACK_CHARSET; throw new RemoteSolrException( - urlExceptionMessage, httpStatus, prefix + body.toString(exceptionEncoding), null); - } catch (IOException e) { + urlExceptionMessage, httpStatus, prefix + body.toString(charset), null); + // IllegalArgumentException covers an unsupported/invalid charset name from the response + } catch (IOException | IllegalArgumentException e) { throw new RemoteSolrException( urlExceptionMessage, httpStatus, diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/response/json/JacksonDataBindResponseParser.java b/solr/solrj/src/java/org/apache/solr/client/solrj/response/json/JacksonDataBindResponseParser.java index a09ac3d2155a..9e91af8416b6 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/response/json/JacksonDataBindResponseParser.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/response/json/JacksonDataBindResponseParser.java @@ -25,6 +25,7 @@ import org.apache.solr.client.api.model.SolrJerseyResponse; import org.apache.solr.client.solrj.request.json.JacksonContentWriter; import org.apache.solr.client.solrj.response.ResponseParser; +import org.apache.solr.common.util.IOUtils; import org.apache.solr.common.util.NamedList; import org.apache.solr.common.util.SimpleOrderedMap; @@ -68,7 +69,9 @@ public NamedList processResponse(InputStream stream, String encoding) th if (encoding == null) { parsedVal = mapper.readValue(stream, typeParam); } else { - parsedVal = mapper.readValue(new InputStreamReader(stream, encoding), typeParam); + parsedVal = + mapper.readValue( + new InputStreamReader(stream, IOUtils.charsetForName(encoding)), typeParam); } final var result = new SimpleOrderedMap(); diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/response/json/JsonMapResponseParser.java b/solr/solrj/src/java/org/apache/solr/client/solrj/response/json/JsonMapResponseParser.java index e1848ba44ff0..2eb2d376e7ef 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/response/json/JsonMapResponseParser.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/response/json/JsonMapResponseParser.java @@ -20,10 +20,12 @@ import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; +import java.nio.charset.StandardCharsets; import java.util.Map; import java.util.Set; import org.apache.solr.client.solrj.response.ResponseParser; import org.apache.solr.common.SolrException; +import org.apache.solr.common.util.IOUtils; import org.apache.solr.common.util.NamedList; import org.noggit.JSONParser; import org.noggit.ObjectBuilder; @@ -43,7 +45,8 @@ public NamedList processResponse(InputStream body, String encoding) thro @SuppressWarnings({"rawtypes"}) Map map = null; try (InputStreamReader reader = - new InputStreamReader(body, encoding == null ? "UTF-8" : encoding)) { + new InputStreamReader( + body, encoding == null ? StandardCharsets.UTF_8 : IOUtils.charsetForName(encoding))) { ObjectBuilder builder = new ObjectBuilder(new JSONParser(reader)); map = (Map) builder.getObject(); } catch (JSONParser.ParseException e) { diff --git a/solr/solrj/src/java/org/apache/solr/common/params/SolrParams.java b/solr/solrj/src/java/org/apache/solr/common/params/SolrParams.java index ec414d763134..e06cc4cd7cb3 100644 --- a/solr/solrj/src/java/org/apache/solr/common/params/SolrParams.java +++ b/solr/solrj/src/java/org/apache/solr/common/params/SolrParams.java @@ -18,8 +18,8 @@ import java.io.IOException; import java.io.Serializable; -import java.io.UnsupportedEncodingException; import java.net.URLEncoder; +import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.Iterator; @@ -458,25 +458,20 @@ public NamedList toNamedList() { * empty. */ public String toQueryString() { - try { - final String charset = StandardCharsets.UTF_8.name(); - final StringBuilder sb = new StringBuilder(128); - boolean first = true; - for (final Iterator it = getParameterNamesIterator(); it.hasNext(); ) { - final String name = it.next(), nameEnc = URLEncoder.encode(name, charset); - for (String val : getParams(name)) { - sb.append(first ? '?' : '&') - .append(nameEnc) - .append('=') - .append(URLEncoder.encode(val, charset)); - first = false; - } + final Charset charset = StandardCharsets.UTF_8; + final StringBuilder sb = new StringBuilder(128); + boolean first = true; + for (final Iterator it = getParameterNamesIterator(); it.hasNext(); ) { + final String name = it.next(), nameEnc = URLEncoder.encode(name, charset); + for (String val : getParams(name)) { + sb.append(first ? '?' : '&') + .append(nameEnc) + .append('=') + .append(URLEncoder.encode(val, charset)); + first = false; } - return sb.toString(); - } catch (UnsupportedEncodingException e) { - // impossible! - throw new AssertionError(e); } + return sb.toString(); } /** diff --git a/solr/solrj/src/java/org/apache/solr/common/util/ContentStreamBase.java b/solr/solrj/src/java/org/apache/solr/common/util/ContentStreamBase.java index ee46e6ddded5..bd87e049b8a7 100644 --- a/solr/solrj/src/java/org/apache/solr/common/util/ContentStreamBase.java +++ b/solr/solrj/src/java/org/apache/solr/common/util/ContentStreamBase.java @@ -24,7 +24,6 @@ import java.io.InputStreamReader; import java.io.Reader; import java.io.StringReader; -import java.io.UnsupportedEncodingException; import java.net.URL; import java.net.URLConnection; import java.nio.charset.StandardCharsets; @@ -214,12 +213,7 @@ public StringStream(String str, String contentType) { this.str = str; this.contentType = contentType; name = null; - try { - size = (long) str.getBytes(DEFAULT_CHARSET).length; - } catch (UnsupportedEncodingException e) { - // won't happen - throw new RuntimeException(e); - } + size = (long) str.getBytes(StandardCharsets.UTF_8).length; sourceInfo = "string"; } @@ -249,14 +243,16 @@ public static String detect(String str) { @Override public InputStream getStream() throws IOException { - return new ByteArrayInputStream(str.getBytes(DEFAULT_CHARSET)); + return new ByteArrayInputStream(str.getBytes(StandardCharsets.UTF_8)); } /** If a charset is defined (by the contentType) use that, otherwise use a StringReader */ @Override public Reader getReader() throws IOException { String charset = getCharsetFromContentType(contentType); - return charset == null ? new StringReader(str) : new InputStreamReader(getStream(), charset); + return charset == null + ? new StringReader(str) + : new InputStreamReader(getStream(), IOUtils.charsetForName(charset)); } } @@ -268,8 +264,8 @@ public Reader getReader() throws IOException { public Reader getReader() throws IOException { String charset = getCharsetFromContentType(getContentType()); return charset == null - ? new InputStreamReader(getStream(), DEFAULT_CHARSET) - : new InputStreamReader(getStream(), charset); + ? new InputStreamReader(getStream(), StandardCharsets.UTF_8) + : new InputStreamReader(getStream(), IOUtils.charsetForName(charset)); } // ------------------------------------------------------------------ diff --git a/solr/solrj/src/java/org/apache/solr/common/util/IOUtils.java b/solr/solrj/src/java/org/apache/solr/common/util/IOUtils.java index 93a2c5eb22ef..c1d1a58f736e 100644 --- a/solr/solrj/src/java/org/apache/solr/common/util/IOUtils.java +++ b/solr/solrj/src/java/org/apache/solr/common/util/IOUtils.java @@ -16,7 +16,9 @@ */ package org.apache.solr.common.util; +import java.io.UnsupportedEncodingException; import java.lang.invoke.MethodHandles; +import java.nio.charset.Charset; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -37,4 +39,21 @@ public static void closeQuietly(AutoCloseable closeable) { log.error("Error while closing", e); } } + + /** + * Resolves a charset name (typically from a Content-Type header) to a {@link Charset}. Unlike + * {@link Charset#forName(String)}, an illegal or unsupported name results in a checked {@link + * UnsupportedEncodingException}, matching the behavior of the legacy {@code String}-based JDK + * charset APIs, so callers can treat a bad charset as an I/O error. + */ + public static Charset charsetForName(String charsetName) throws UnsupportedEncodingException { + try { + return Charset.forName(charsetName); + } catch (IllegalArgumentException e) { + UnsupportedEncodingException uee = + new UnsupportedEncodingException("Unsupported charset: " + charsetName); + uee.initCause(e); + throw uee; + } + } }