Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jetty 12 mimetype cleanup #8919

Merged
merged 21 commits into from
Nov 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
481 changes: 204 additions & 277 deletions jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MimeTypes.java

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,6 @@ svgz=image/svg+xml
swf=application/x-shockwave-flash
t=application/x-troff
tar=application/x-tar
tar.gz=application/x-gtar
tcl=application/x-tcl
tex=application/x-tex
texi=application/x-texinfo
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ public static Stream<Arguments> mimeTypesByExtensionCases()
{
return Stream.of(
Arguments.of("test.gz", "application/gzip"),
Arguments.of("test.tar.gz", "application/gzip"),
Arguments.of("test.tgz", "application/x-gtar"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is deliberate, yes? You removed type application/x-gtar from mime.properties ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct. Aligning with common extension handling practice of just looking at the final dot segment. This is more efficient, same as most other projects and there is only one extension we had with a dot in it.

Arguments.of("foo.webp", "image/webp"),
Arguments.of("zed.avif", "image/avif"),
// make sure that filename case isn't an issue
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import java.util.List;
import java.util.concurrent.Executor;

import org.eclipse.jetty.http.MimeTypes;
import org.eclipse.jetty.util.Attributes;
import org.eclipse.jetty.util.Decorator;
import org.eclipse.jetty.util.resource.Resource;
Expand Down Expand Up @@ -47,6 +48,8 @@ public interface Context extends Attributes, Decorator, Executor

List<String> getVirtualHosts();

MimeTypes getMimeTypes();

@Override
/** execute runnable in container thread scoped to context */
void execute(Runnable runnable);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.eclipse.jetty.http.HttpField;
import org.eclipse.jetty.http.HttpGenerator;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.MimeTypes;
import org.eclipse.jetty.http.PreEncodedHttpField;
import org.eclipse.jetty.io.Connection;
import org.eclipse.jetty.server.handler.ContextHandler;
Expand Down Expand Up @@ -725,6 +726,12 @@ public String getContextPath()
return null;
}

@Override
public MimeTypes getMimeTypes()
{
return MimeTypes.DEFAULTS;
}

@Override
public ClassLoader getClassLoader()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public class BufferedResponseHandler extends Handler.Wrapper
public BufferedResponseHandler()
{
_methods.include(HttpMethod.GET.asString());
for (String type : MimeTypes.getKnownMimeTypes())
for (String type : MimeTypes.DEFAULTS.getMimeMap().values())
{
if (type.startsWith("image/") ||
type.startsWith("audio/") ||
Expand Down Expand Up @@ -151,7 +151,7 @@ public Request.Processor handle(Request request) throws Exception
}

// If the mime type is known from the path then apply mime type filtering.
String mimeType = MimeTypes.getDefaultMimeByExtension(path); // TODO context specicif mimetypes : context.getMimeType(path);
String mimeType = request.getContext().getMimeTypes().getMimeByExtension(path);
if (mimeType != null)
{
mimeType = MimeTypes.getContentTypeWithoutCharset(mimeType);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.eclipse.jetty.http.HttpField;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.http.MimeTypes;
import org.eclipse.jetty.server.AliasCheck;
import org.eclipse.jetty.server.Connector;
import org.eclipse.jetty.server.Handler;
Expand Down Expand Up @@ -87,9 +88,15 @@ public static String getServerInfo()
return "jetty/" + Server.getVersion();
}

// TODO should persistent attributes be an Attributes.Layer over server attributes?
private final Attributes _persistentAttributes = new Mapped();
/*
* The context (specifically it's attributes and mimeTypes) are not implemented as a layer over
* the server context, as this handler's context replaces the context in the request, it does not
* wrap it. This is so that any cross context dispatch does not inherit attributes and types from
* the dispatching context.
*/
private final Context _context;
private final Attributes _persistentAttributes = new Mapped();
private final MimeTypes.Mutable _mimeTypes = new MimeTypes.Mutable();
private final List<ContextScopeListener> _contextListeners = new CopyOnWriteArrayList<>();
private final List<VHost> _vhosts = new ArrayList<>();

Expand Down Expand Up @@ -158,6 +165,11 @@ protected Context newContext()
return new Context();
}

public MimeTypes.Mutable getMimeTypes()
{
return _mimeTypes;
}

@Override
public void dump(Appendable out, String indent) throws IOException
{
Expand Down Expand Up @@ -1002,6 +1014,12 @@ public String getContextPath()
return _contextPath;
}

@Override
public MimeTypes getMimeTypes()
{
return _mimeTypes;
}

@Override
public String toString()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,7 @@ public void doStart() throws Exception
_resourceBase = context.getBaseResource();
}

// TODO: _mimeTypes = _context == null ? new MimeTypes() : _context.getMimeTypes();
if (_mimeTypes == null)
_mimeTypes = new MimeTypes();
_mimeTypes = context == null ? MimeTypes.DEFAULTS : context.getMimeTypes();

_byteBufferPool = getByteBufferPool(context);
_resourceService.setHttpContentFactory(newHttpContentFactory());
Expand Down Expand Up @@ -320,11 +318,6 @@ public int getEncodingCacheSize()
return _resourceService.getEncodingCacheSize();
}

public void setMimeTypes(MimeTypes mimeTypes)
{
_mimeTypes = mimeTypes;
}

/**
* @param redirectWelcome If true, welcome files are redirected rather than forwarded to.
* redirection is always used if the ResourceHandler is not scoped by
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public GzipHandler()
{
_methods.include(HttpMethod.GET.asString());
_methods.include(HttpMethod.POST.asString());
for (String type : MimeTypes.getKnownMimeTypes())
for (String type : MimeTypes.DEFAULTS.getMimeMap().values())
{
if ("image/svg+xml".equals(type))
_paths.exclude("*.svgz");
Expand Down Expand Up @@ -624,9 +624,8 @@ else if (COMMA_GZIP.matcher(field.getValue()).matches())
return wrappedRequest.wrapProcessor(super.handle(wrappedRequest));
}

// TODO: get mimetype from context.
// Exclude non compressible mime-types known from URI extension. - no Vary because no matter what client, this URI is always excluded
String mimeType = MimeTypes.getDefaultMimeByExtension(path);
String mimeType = request.getContext().getMimeTypes().getMimeByExtension(path);
if (mimeType != null)
{
mimeType = HttpField.valueParameters(mimeType, null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.http.HttpTester;
import org.eclipse.jetty.http.HttpVersion;
import org.eclipse.jetty.http.MimeTypes;
import org.eclipse.jetty.io.Content;
import org.eclipse.jetty.server.Context;
import org.eclipse.jetty.server.FormFields;
Expand Down Expand Up @@ -118,8 +117,6 @@ public class GzipHandlerTest
private static final String CONTENT_ETAG = String.format("W/\"%x\"", CONTENT.hashCode());
private static final String CONTENT_ETAG_GZIP = String.format("W/\"%x" + CompressedContentFormat.GZIP.getEtagSuffix() + "\"", CONTENT.hashCode());

private static final MimeTypes MIME_TYPES = new MimeTypes();

public WorkDir _workDir;
private Server _server;
private LocalConnector _connector;
Expand Down Expand Up @@ -1667,9 +1664,8 @@ private String getContentTypeFromRequest(String filename, Request request)
if (parameters.get("type") != null)
defaultContentType = parameters.get("type").getValue();

// TODO get mime type from context.
Context context = request.getContext();
String contentType = MIME_TYPES.getMimeByExtension(filename);
String contentType = context.getMimeTypes().getMimeByExtension(filename);
if (contentType != null)
return contentType;
return defaultContentType;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,9 @@ public static String getExtension(Path path)
* for the extension (if any), including the dot, lower-cased.
*
* @param filename The string path
* @return The last segment extension, or null if not a file, or null if there is no extension present
* @return The last segment extension excluding the leading dot;
* or null if not a file;
* or null if there is no extension present
*/
public static String getExtension(String filename)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import java.util.stream.Collectors;

import org.apache.maven.artifact.Artifact;
import org.apache.maven.plugin.AbstractMojoExecutionException;
import org.apache.maven.plugin.MojoExecutionException;
import org.apache.maven.plugin.MojoFailureException;
import org.apache.maven.plugins.annotations.Parameter;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,10 @@
import java.nio.file.Path;
import java.nio.file.Paths;

import org.apache.maven.plugin.AbstractMojoExecutionException;
import org.apache.maven.plugin.MojoExecutionException;
import org.apache.maven.plugin.MojoFailureException;
import org.apache.maven.plugins.annotations.Mojo;
import org.apache.maven.plugins.annotations.Parameter;
import org.apache.maven.plugins.annotations.ResolutionScope;
import org.eclipse.jetty.util.StringUtil;

/**
* Generate the effective web.xml for a pre-built webapp. This goal will NOT
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import java.util.Date;
import java.util.Set;

import org.apache.maven.plugin.AbstractMojoExecutionException;
import org.apache.maven.plugin.MojoExecutionException;
import org.apache.maven.plugins.annotations.Execute;
import org.apache.maven.plugins.annotations.LifecyclePhase;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import java.io.File;
import java.nio.file.Path;

import org.apache.maven.plugin.AbstractMojoExecutionException;
import org.apache.maven.plugin.MojoExecutionException;
import org.apache.maven.plugins.annotations.Mojo;
import org.apache.maven.plugins.annotations.Parameter;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@
import org.eclipse.jetty.ee10.servlet.security.ConstraintSecurityHandler;
import org.eclipse.jetty.ee10.servlet.security.SecurityHandler;
import org.eclipse.jetty.http.HttpURI;
import org.eclipse.jetty.http.MimeTypes;
import org.eclipse.jetty.http.pathmap.MatchedResource;
import org.eclipse.jetty.server.Handler;
import org.eclipse.jetty.server.Request;
Expand Down Expand Up @@ -192,7 +191,6 @@ public interface ServletContainerInitializerCaller extends LifeCycle {}
private String _defaultRequestCharacterEncoding;
private String _defaultResponseCharacterEncoding;
private String _contextPathEncoded = "/";
protected MimeTypes _mimeTypes; // TODO move to core context?
private Map<String, String> _localeEncodingMap;
private String[] _welcomeFiles;
private Logger _logger;
Expand Down Expand Up @@ -649,24 +647,6 @@ else if (contextPath.length() > 1 && contextPath.endsWith("/"))
}
}

/**
* @return Returns the mimeTypes.
*/
public MimeTypes getMimeTypes()
{
if (_mimeTypes == null)
_mimeTypes = new MimeTypes();
return _mimeTypes;
}

/**
* @param mimeTypes The mimeTypes to set.
*/
public void setMimeTypes(MimeTypes mimeTypes)
{
_mimeTypes = mimeTypes;
}

public void setWelcomeFiles(String[] files)
{
_welcomeFiles = files;
Expand Down Expand Up @@ -1079,9 +1059,6 @@ protected void doStart() throws Exception
if (getServer() != null)
_servletContext.setAttribute("org.eclipse.jetty.server.Executor", getServer().getThreadPool());

if (_mimeTypes == null)
_mimeTypes = new MimeTypes();

_durableListeners.addAll(getEventListeners());

getContext().call(() ->
Expand Down Expand Up @@ -2811,9 +2788,7 @@ public ServletContext getContext(String uripath)
@Override
public String getMimeType(String file)
{
if (_mimeTypes == null)
return null;
return _mimeTypes.getMimeByExtension(file);
return getContext().getMimeTypes().getMimeByExtension(file);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -349,14 +349,15 @@ public String getCharacterEncoding(boolean setContentType)
// Try charset from mime type.
if (_mimeType != null && _mimeType.isCharsetAssumed())
return _mimeType.getCharsetString();

// Try charset assumed from content type (assumed charsets are not added to content type header).
encoding = MimeTypes.getCharsetAssumedFromContentType(_contentType);
MimeTypes mimeTypes = _request.getContext().getMimeTypes();
encoding = mimeTypes.getCharsetAssumedFromContentType(_contentType);
if (encoding != null)
return encoding;

// Try char set inferred from content type.
encoding = MimeTypes.getCharsetInferredFromContentType(_contentType);
encoding = mimeTypes.getCharsetInferredFromContentType(_contentType);
if (encoding != null)
{
if (setContentType)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@

package org.eclipse.jetty.ee10.servlets;

import java.util.Objects;

import jakarta.servlet.Filter;
import jakarta.servlet.FilterConfig;
import jakarta.servlet.ServletContext;
Expand Down Expand Up @@ -109,7 +111,7 @@ protected String guessMimeType(HttpServletRequest httpRequest, HttpServletRespon
String contentType = httpResponse.getContentType();
LOG.debug("Content Type is: {}", contentType);

String mimeType = "";
String mimeType;
if (contentType != null)
{
mimeType = MimeTypes.getContentTypeWithoutCharset(contentType);
Expand All @@ -118,13 +120,7 @@ protected String guessMimeType(HttpServletRequest httpRequest, HttpServletRespon
else
{
String requestUrl = httpRequest.getPathInfo();
mimeType = MimeTypes.getDefaultMimeByExtension(requestUrl);

if (mimeType == null)
{
mimeType = "";
}

mimeType = Objects.requireNonNullElse(httpRequest.getServletContext().getMimeType(requestUrl), "");
LOG.debug("Guessed mime type is {}", mimeType);
}

Expand Down Expand Up @@ -169,11 +165,9 @@ public void destroy()
@Override
public String toString()
{
StringBuilder sb = new StringBuilder();
sb.append("filter configuration:\n");
sb.append("paths:\n").append(_paths).append("\n");
sb.append("mime types:\n").append(_mimeTypes).append("\n");
sb.append("http methods:\n").append(_httpMethods);
return sb.toString();
return "filter configuration:\n" +
"paths:\n" + _paths + "\n" +
"mime types:\n" + _mimeTypes + "\n" +
"http methods:\n" + _httpMethods;
}
}
Loading