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

Fix #1512: Plugin doesn't abort building an image in case Podman is u… #1585

Merged
merged 2 commits into from
Jul 31, 2022
Merged
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
@@ -1,11 +1,14 @@
package io.fabric8.maven.docker.access.hc;

import com.google.gson.JsonIOException;
import com.google.gson.JsonParser;
import com.google.gson.JsonSyntaxException;
import com.google.gson.stream.JsonReader;
import io.fabric8.maven.docker.access.chunked.EntityStreamReaderUtil;
import org.apache.http.HttpResponse;
import org.apache.http.client.ResponseHandler;

import java.io.IOException;
import java.io.InputStream;
import java.io.*;
import java.util.stream.Stream;

public class HcChunkedResponseHandlerWrapper implements ResponseHandler<Object> {
Expand All @@ -18,15 +21,64 @@ public class HcChunkedResponseHandlerWrapper implements ResponseHandler<Object>
@Override
public Object handleResponse(HttpResponse response) throws IOException {
try (InputStream stream = response.getEntity().getContent()) {
// Parse text as json
if (isJson(response)) {
EntityStreamReaderUtil.processJsonStream(handler, stream);
ByteArrayOutputStream baos = getMultipleReadbleOutputStream(stream);
InputStream is = new ByteArrayInputStream(baos.toByteArray());
// In the previous version of this file the following if() was as follows. The methode isJson() checks
// by header (not by body):
// if(isJson(response)) {
// EntityStreamReaderUtil.processJsonStream(handler, stream);
// }
//
// In case the Podman daemon is used, the POST /build response is JSON and the HTTP status code is 200
// as expected, but there is no header "Content-Type = application/json" nor "Content-Type" at all. Seen in
// Podman 3.4.2. But the docker-maven-plugin relies on that JSON-body. In BuildJsonResponseHandler.process():
// if (json.has("error")) {
// ...
// throw new DockerAccessException();
// ...
//
// If no error is detected, the Maven-build goes on despite there was a problem building the
// image!
// The following if() first checks for the application/json Content-Type. If no Content-Type is set,
// it tries to detect if the body is JSON. If so, the handler is called.
if(isJsonCheckedByHeader(response) || (isMissingContentType(response) && isJsonCheckedByBody(is))){
is = new ByteArrayInputStream(baos.toByteArray());
EntityStreamReaderUtil.processJsonStream(handler, is);
}
}
return null;
}

private static boolean isJson(HttpResponse response) {
private ByteArrayOutputStream getMultipleReadbleOutputStream(InputStream is) throws IOException {
ByteArrayOutputStream baos = new ByteArrayOutputStream();
byte[] buffer = new byte[1024];
int len;
while ((len = is.read(buffer)) > -1 ) {
baos.write(buffer, 0, len);
}
baos.flush();
return baos;
}

private static boolean isJsonCheckedByBody(InputStream is){
try {
JsonReader json = new JsonReader(new InputStreamReader(is));
JsonParser parser = new JsonParser();
parser.parse(json);
// No exception until here: Content is JSON.
} catch (JsonIOException | JsonSyntaxException e){
// No JSON.
return false;
}
return true;
}

private static boolean isMissingContentType(HttpResponse response){
return Stream.of(response.getAllHeaders())
.noneMatch(h -> h.getName().equalsIgnoreCase("Content-Type"));
}

private static boolean isJsonCheckedByHeader(HttpResponse response) {
return Stream.of(response.getAllHeaders())
.filter(h -> h.getName().equalsIgnoreCase("Content-Type"))
.anyMatch(h -> h.getValue().toLowerCase().startsWith("application/json"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.io.InputStream;

@SuppressWarnings("unused")
@ExtendWith(MockitoExtension.class)
Expand Down Expand Up @@ -56,7 +57,9 @@ void handleResponseWithTextPlainResponse() throws IOException {
void handleResponseWithNoContentType() throws IOException {
givenResponseHeaders();
hcChunkedResponseHandlerWrapper.handleResponse(response);
verifyProcessJsonStream(0);
// timesCalled is 1 here because without "Content-Type" handleResponse() tries to parse the body to
// detect if it is JSON or not. See HcChunkedResponseHandlerWrapper.handleResponse() for more details.
verifyProcessJsonStream(1);
}

private void givenResponseHeaders(Header... headers) throws IOException {
Expand Down