Skip to content

Commit

Permalink
fix: fix the 5.2.x build to be compatible with newer jetty/jackson ve…
Browse files Browse the repository at this point in the history
…rsions (#7575)

* chore: Remove use of deprecated Jackson API in 5.4.x (#5725)
  • Loading branch information
agavra authored May 24, 2021
1 parent 260b979 commit 103eeec
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import static com.fasterxml.jackson.core.JsonToken.VALUE_TRUE;

import com.fasterxml.jackson.core.JsonFactory;
import com.fasterxml.jackson.core.JsonFactoryBuilder;
import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.JsonToken;
import io.confluent.ksql.function.KsqlFunctionException;
Expand All @@ -38,8 +39,9 @@
import java.util.Map;

public class ArrayContainsKudf implements Kudf {
private static final JsonFactory JSON_FACTORY =
new JsonFactory().disable(CANONICALIZE_FIELD_NAMES);
private static final JsonFactory JSON_FACTORY = new JsonFactoryBuilder()
.disable(CANONICALIZE_FIELD_NAMES)
.build();

interface Matcher {
boolean matches(JsonParser parser, Object searchValue) throws IOException;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,10 @@
import io.confluent.rest.Application;
import io.confluent.rest.validation.JacksonMessageBodyProvider;
import java.io.Console;
import java.io.IOException;
import java.io.OutputStreamWriter;
import java.io.PrintWriter;
import java.io.Writer;
import java.net.URL;
import java.nio.charset.StandardCharsets;
import java.time.Duration;
Expand All @@ -91,12 +93,17 @@
import java.util.function.Predicate;
import java.util.function.Supplier;
import java.util.stream.Collectors;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import javax.websocket.DeploymentException;
import javax.websocket.server.ServerEndpoint;
import javax.websocket.server.ServerEndpointConfig;
import javax.websocket.server.ServerEndpointConfig.Configurator;
import javax.ws.rs.core.Configurable;
import org.eclipse.jetty.http.MimeTypes;
import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.server.ServerConnector;
import org.eclipse.jetty.server.handler.ErrorHandler;
import org.eclipse.jetty.websocket.jsr356.server.ServerContainer;
import org.glassfish.jersey.server.ServerProperties;
import org.slf4j.Logger;
Expand Down Expand Up @@ -184,6 +191,58 @@ public void setupResources(final Configurable<?> config, final KsqlRestConfig ap
@Override
public void start() throws Exception {
super.start();

// WARN: SUPER HACKY CODE BELOW
//
// as of jetty 9.4.21.v20190926 the behavior of the error handler
// changed in a "backwards incompatible" way.
//
// it used to have the behavior below where the response type
// is TEXT_HTML regardless of the input type and there was no
// special content (it was just handled as an error page) returned
// so the contents of the message were just an empty string
//
// in 9.4.21+ it is a bit more intelligent about responding in
// the same format as the input. If the request content type
// accepts JSON, it will respond with more detail error message
// including the url, status code and message that caused the error
//
// while this is in theory a wonderful improvement, it proves to
// be backwards incompatible with our client :( Specifically, we
// expect there to be no error contents UNLESS it's properly formatted
// JSON representation of KsqlErrorMessage.
//
// this code below forces an error handler that has the old behavior to
// maintain backwards compatibility while still addressing the CVEs fixed
// in more recent jetty versions
server.setErrorHandler(new ErrorHandler() {
@SuppressWarnings("deprecation")
@Override
protected void generateAcceptableResponse(
final Request baseRequest,
final HttpServletRequest request,
final HttpServletResponse response,
final int code,
final String message,
final String contentType
) throws IOException {
switch (contentType) {
case "text/html":
case "text/*":
case "*/*":
baseRequest.setHandled(true);
final Writer writer = getAcceptableWriter(baseRequest, request, response);
if (writer != null) {
response.setContentType(MimeTypes.Type.TEXT_HTML.asString());
handleErrorPage(request, writer, code, message);
}
break;
default:
break;
}
}
});

startKsql();
commandRunnerThread.start();
final Properties metricsProperties = new Properties();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import static org.junit.Assert.assertThrows;

import com.fasterxml.jackson.databind.exc.InvalidDefinitionException;
import com.fasterxml.jackson.databind.exc.ValueInstantiationException;
import com.google.common.testing.EqualsTester;
import io.confluent.ksql.rest.server.computation.ConfigTopicKey.StringKey;
import io.confluent.ksql.rest.util.InternalTopicJsonSerdeUtil;
Expand Down Expand Up @@ -79,7 +80,7 @@ private static class IllegalArgumentMatcher extends TypeSafeMatcher<Exception> {
@Override
public boolean matchesSafely(final Exception e) {
return e instanceof SerializationException
&& e.getCause() instanceof InvalidDefinitionException
&& e.getCause() instanceof InvalidDefinitionException || e.getCause() instanceof ValueInstantiationException
&& exceptionClass.isInstance(e.getCause().getCause())
&& e.getCause().getCause().getMessage().contains(msg);
}
Expand Down

0 comments on commit 103eeec

Please sign in to comment.