Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ final class DefaultServerConfig implements ServerConfig {
private final boolean enableDateHeader;
private final ServerErrorHandler errorHandler;
private final Http1HeaderNaming http1HeaderNaming;
private final List<String> additionalAllowedHttpMethods;
private final DependencyInjector dependencyInjector;
private final Function<String, String> absoluteUriTransformer;
private final long unloggedExceptionsReportIntervalMillis;
Expand Down Expand Up @@ -148,6 +149,7 @@ final class DefaultServerConfig implements ServerConfig {
ServerErrorHandler errorHandler,
@Nullable Mapping<String, SslContext> sslContexts,
Http1HeaderNaming http1HeaderNaming,
List<String> additionalAllowedHttpMethods,
DependencyInjector dependencyInjector,
Function<? super String, String> absoluteUriTransformer,
long unloggedExceptionsReportIntervalMillis,
Expand Down Expand Up @@ -259,6 +261,8 @@ final class DefaultServerConfig implements ServerConfig {
this.errorHandler = requireNonNull(errorHandler, "errorHandler");
this.sslContexts = sslContexts;
this.http1HeaderNaming = requireNonNull(http1HeaderNaming, "http1HeaderNaming");
this.additionalAllowedHttpMethods = ImmutableList.copyOf(
requireNonNull(additionalAllowedHttpMethods, "additionalAllowedHttpMethods"));
this.dependencyInjector = requireNonNull(dependencyInjector, "dependencyInjector");
@SuppressWarnings("unchecked")
final Function<String, String> castAbsoluteUriTransformer =
Expand Down Expand Up @@ -654,6 +658,11 @@ public Http1HeaderNaming http1HeaderNaming() {
return http1HeaderNaming;
}

@Override
public List<String> additionalAllowedHttpMethods() {
return additionalAllowedHttpMethods;
}

@Override
public DependencyInjector dependencyInjector() {
return dependencyInjector;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,11 +197,11 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception
cfg, scheme.toString(), reqTarget);
// Do not accept unsupported methods.
final HttpMethod method = headers.method();
switch (method) {
case CONNECT:
case UNKNOWN:
fail(id, headers, HttpStatus.METHOD_NOT_ALLOWED, "Unsupported method", null);
return;
if (method == HttpMethod.CONNECT
|| (method == HttpMethod.UNKNOWN
&& !cfg.additionalAllowedHttpMethods().contains(headers.get(com.linecorp.armeria.common.HttpHeaderNames.METHOD)))) {
fail(id, headers, HttpStatus.METHOD_NOT_ALLOWED, "Unsupported method", null);
return;
}

// Do not accept the request path '*' for a non-OPTIONS request.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,11 @@ public void onHeadersRead(ChannelHandlerContext ctx, int streamId, Http2Headers
}
break;
case UNKNOWN:
writeUnsupportedMethodResponse(streamId, headers);
return;
if (!cfg.additionalAllowedHttpMethods().contains(headers.get(com.linecorp.armeria.common.HttpHeaderNames.METHOD))) {
writeUnsupportedMethodResponse(streamId, headers);
return;
}
break;
}

// Do not accept the request path '*' for a non-OPTIONS request.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import java.security.cert.X509Certificate;
import java.time.Duration;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
Expand Down Expand Up @@ -73,6 +74,7 @@
import com.linecorp.armeria.common.Flags;
import com.linecorp.armeria.common.Http1HeaderNaming;
import com.linecorp.armeria.common.HttpHeaderNames;
import com.linecorp.armeria.common.HttpMethod;
import com.linecorp.armeria.common.HttpRequest;
import com.linecorp.armeria.common.HttpResponse;
import com.linecorp.armeria.common.Request;
Expand Down Expand Up @@ -229,6 +231,7 @@ public final class ServerBuilder implements TlsSetters, ServiceConfigsBuilder<Se
private boolean enableServerHeader = true;
private boolean enableDateHeader = true;
private Http1HeaderNaming http1HeaderNaming = Http1HeaderNaming.ofDefault();
private final List<String> additionalAllowedHttpMethods = new ArrayList<>();
@Nullable
private DependencyInjector dependencyInjector;
private Function<? super String, String> absoluteUriTransformer = Function.identity();
Expand Down Expand Up @@ -2220,6 +2223,17 @@ public ServerBuilder http1HeaderNaming(Http1HeaderNaming http1HeaderNaming) {
return this;
}

/**
* Adds additional allowed HTTP methods to the default set of allowed {@link HttpMethod}s.
*
* @param additionalAllowedHttpMethods the additional allowed HTTP methods
*/
public ServerBuilder additionalAllowedHttpMethods(String... additionalAllowedHttpMethods) {
Copy link
Contributor

Choose a reason for hiding this comment

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

optional) for consistency with other APIs, it may be worth adding a additionalAllowedHttpMethods(Iterable<String>) variant

requireNonNull(additionalAllowedHttpMethods, "additionalAllowedHttpMethods");
this.additionalAllowedHttpMethods.addAll(Arrays.asList(additionalAllowedHttpMethods));
return this;
}

/**
* Sets the interval between reporting exceptions which is not logged
* by any decorators or services such as {@link LoggingService}.
Expand Down Expand Up @@ -2427,7 +2441,7 @@ ports, setSslContextIfAbsent(defaultVirtualHost, defaultSslContext),
childChannelPipelineCustomizer,
clientAddressSources, clientAddressTrustedProxyFilter, clientAddressFilter, clientAddressMapper,
enableServerHeader, enableDateHeader, errorHandler, sslContexts,
http1HeaderNaming, dependencyInjector, absoluteUriTransformer,
http1HeaderNaming, additionalAllowedHttpMethods, dependencyInjector, absoluteUriTransformer,
unloggedExceptionsReportIntervalMillis, ImmutableList.copyOf(shutdownSupports));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,11 @@ default boolean shutdownBlockingTaskExecutorOnStop() {
*/
Http1HeaderNaming http1HeaderNaming();

/**
* Returns the list of HTTP methods that are allowed to be used in addition to the standard HTTP methods.
*/
List<String> additionalAllowedHttpMethods();

/**
* Returns the {@link DependencyInjector} that injects dependencies in annotations.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,11 @@ public Http1HeaderNaming http1HeaderNaming() {
return delegate.http1HeaderNaming();
}

@Override
public List<String> additionalAllowedHttpMethods() {
return delegate.additionalAllowedHttpMethods();
}

@Override
public DependencyInjector dependencyInjector() {
return delegate.dependencyInjector();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -532,8 +532,7 @@ private Request convertRequest(ServiceRequestContext ctx, String mappedPath, Agg
coyoteReq.protocol().setString(ctx.sessionProtocol().isMultiplex() ? "HTTP/2.0" : "HTTP/1.1");

// Set the method.
final HttpMethod method = req.method();
coyoteReq.method().setString(method.name());
coyoteReq.method().setString(req.headers().get(HttpHeaderNames.METHOD));
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you also add a integration test which validates the server can successfully process the custom http method?

Copy link
Contributor

Choose a reason for hiding this comment

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

note: I think it's fine to support just tomcat for now, and add support for jetty later if needed/reported


// Set the request URI.
final byte[] uriBytes = mappedPath.getBytes(StandardCharsets.US_ASCII);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@
import java.net.Socket;
import java.nio.charset.StandardCharsets;

import org.apache.catalina.Context;
import org.apache.catalina.Wrapper;
import org.apache.catalina.connector.Connector;
import org.apache.catalina.servlets.WebdavServlet;
import org.apache.catalina.startup.Tomcat;
import org.apache.hc.client5.http.classic.methods.HttpGet;
import org.apache.hc.client5.http.impl.classic.CloseableHttpClient;
Expand All @@ -46,6 +49,7 @@ class UnmanagedTomcatServiceTest {

private static Tomcat tomcatWithWebApp;
private static Tomcat tomcatWithoutWebApp;
private static Tomcat tomcatWithPropfind;

@RegisterExtension
static final ServerExtension server = new ServerExtension() {
Expand All @@ -66,16 +70,32 @@ protected void configure(ServerBuilder sb) throws Exception {
"tomcat-" + UnmanagedTomcatServiceTest.class.getSimpleName() + "-2");
assertThat(TomcatUtil.engine(tomcatWithoutWebApp.getService(), "bar")).isNotNull();

tomcatWithPropfind = new Tomcat();
tomcatWithPropfind.setPort(0);

Context ctx = tomcatWithPropfind.addContext("", "build" + File.separatorChar +
"tomcat-" + UnmanagedTomcatServiceTest.class.getSimpleName() + "-3");

Wrapper webdavServlet = Tomcat.addServlet(ctx, "webdav", new WebdavServlet());
webdavServlet.addInitParameter("readonly", "false");
webdavServlet.addInitParameter("listings", "true");

ctx.addServletMappingDecoded("/*", "webdav");
assertThat(TomcatUtil.engine(tomcatWithPropfind.getService(), "foobar")).isNotNull();

// Start the Tomcats.
tomcatWithWebApp.start();
tomcatWithoutWebApp.start();
tomcatWithPropfind.start();

// Bind them to the Server.
sb.serviceUnder("/empty/", TomcatService.of(new Connector(), "someHost"))
.serviceUnder("/some-webapp-nohostname/",
TomcatService.of(tomcatWithWebApp.getConnector()))
.serviceUnder("/no-webapp/", TomcatService.of(tomcatWithoutWebApp))
.serviceUnder("/some-webapp/", TomcatService.of(tomcatWithWebApp));
.serviceUnder("/some-webapp/", TomcatService.of(tomcatWithWebApp))
.serviceUnder("/propfind/", TomcatService.of(tomcatWithPropfind))
.additionalAllowedHttpMethods("PROPFIND");
}
};

Expand All @@ -89,6 +109,10 @@ static void destroyTomcat() throws Exception {
tomcatWithoutWebApp.stop();
tomcatWithoutWebApp.destroy();
}
if (tomcatWithPropfind != null) {
tomcatWithPropfind.stop();
tomcatWithPropfind.destroy();
}
}

@Test
Expand Down Expand Up @@ -147,4 +171,20 @@ void okWithoutAuthorityHeader() throws Exception {
}
}
}

@Test
void okWithCustomMethod() throws Exception {
final int port = server.httpPort();
try (Socket s = new Socket(NetUtil.LOCALHOST, port)) {
final InputStream in = s.getInputStream();
final OutputStream out = s.getOutputStream();
out.write(("PROPFIND /propfind/ HTTP/1.1\r\n" +
"Content-Length: 0\r\n" +
"Connection: close\r\n\r\n").getBytes(StandardCharsets.US_ASCII));

try (BufferedReader br = new BufferedReader(new InputStreamReader(in))) {
assertThat(br.readLine()).isEqualTo("HTTP/1.1 200 OK");
}
}
}
}
Loading