jojochuang commented on code in PR #11367: URL: https://github.com/apache/ozone/pull/11367#discussion_r4141418856
########## hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/S3ContentTypeFilter.java: ########## @@ -0,0 +1,126 @@ +/* + * 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.hadoop.ozone.s3; + +import java.io.IOException; +import java.util.Locale; +import javax.servlet.Filter; +import javax.servlet.FilterChain; +import javax.servlet.FilterConfig; +import javax.servlet.ServletException; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; +import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.HttpServletResponseWrapper; + +/** + * Preserve the S3 Content-Type verbatim, without an auto-appended charset. + * + * <p>AWS S3 (and Ozone before the Jetty 12 upgrade) returns the Content-Type + * exactly as the gateway set it: the stored object type for GET/HEAD and a bare + * {@code application/xml} for the XML API/error responses, in both cases without + * a charset parameter unless one was explicitly stored. + * + * <p>On Jetty 12 the servlet response tracks a response character encoding + * ({@code _encodingFrom}). HttpServer2's global {@code QuotingInputFilter} runs + * first on every request and, for a request URI with no known extension (most + * object keys), defaults the response to {@code text/plain; charset=utf-8}, + * promoting that encoding away from {@code NOT_SET}. Jetty then appends the + * tracked charset to any later <em>bare</em> Content-Type, so a stored + * {@code binary/octet-stream} or a bare {@code application/xml} goes out as + * {@code ...;charset=utf-8}. Stripping the charset from the header value does + * not help, because Jetty re-appends it from the tracked encoding after the + * wrapper runs. + * + * <p>This wrapper resets the tracked encoding back to {@code NOT_SET} (via + * {@code setCharacterEncoding(null)}) right before it writes a Content-Type + * that carries no {@code charset} parameter, so Jetty keeps that value bare. A + * Content-Type that already carries an explicit {@code charset} is written + * unchanged. In all cases the value itself is passed through verbatim, so the + * media type and any explicit charset are preserved. + */ +public class S3ContentTypeFilter implements Filter { + + private static final String CONTENT_TYPE = "Content-Type"; + + @Override + public void init(FilterConfig filterConfig) throws ServletException { + } + + @Override + public void doFilter( + ServletRequest request, ServletResponse response, FilterChain chain + ) throws IOException, ServletException { + if (response instanceof HttpServletResponse) { + chain.doFilter(request, new VerbatimContentTypeResponse((HttpServletResponse) response)); + } else { + chain.doFilter(request, response); + } + } + + @Override + public void destroy() { + } + + private static boolean hasCharset(String contentType) { + return contentType != null + && contentType.toLowerCase(Locale.ROOT).contains("charset="); Review Comment: [P3] Check for an actual charset parameter `contains("charset=")` can match text inside another parameter's quoted value. For example, this valid Content-Type has a `name` parameter but no charset parameter: ```text application/octet-stream; name="charset=example" ``` The filter incorrectly treats it as having an explicit charset and skips the encoding reset. Jetty then appends the UTF-8 encoding established by `QuotingInputFilter`: ```text application/octet-stream; name="charset=example";charset=utf-8 ``` I reproduced this with real servlet responses: Jetty 9 preserves the original header, while Jetty 12 with this filter returns the modified value. This changes the Content-Type returned for a stored S3 object. The reproduction used a servlet harness matching the encoding setup and this filter, rather than a full Ozone round-trip test. Could we parse the media-type parameters to detect an actual `charset` parameter, while forwarding the original header unchanged, and add a test for this quoted-value case? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
