fabric8io / kubernetes-client

Java client for Kubernetes & OpenShift
http://fabric8.io
Apache License 2.0
3.35k stars 1.44k forks source link

DefaultMockServer alters binary data/byte arrays/octet stream #6061

Open Steve973 opened 2 weeks ago

Steve973 commented 2 weeks ago

Describe the bug

I am creating a basic, non-TLS DefaultMockServer for some testing that I am doing. I set up a byte array for an interaction/expect to return as a response and retrieve it with an HttpURLConnection. I examine the data that I am sending back from the web server, and I examine the data that I actually received, and they do not match! I was looking for documentation that describes this behavior and, either it does not exist, or this is not the intended behavior. I think that this should absolutely not be the intended behavior.

Fabric8 Kubernetes Client version

6.13.0

Steps to reproduce

I have created code that demonstrates this (mis)behavior simply, consistently and repeatably:

    void testBytesRoundTrip() throws Exception {
        BiConsumer<String, byte[]> printBytes = (desc, bytes) -> {
            StringBuilder sb = new StringBuilder();
            for (Byte b : bytes) {
                sb.append(String.format("%02x", b));
            }
            System.err.println("########## " + desc + " (bytes as hex): " + sb);
        };
        DefaultMockServer mockServer = new DefaultMockServer();
        mockServer.start();
        String mockServerUrl = mockServer.url("");
        byte[] dataBytes = new byte[]{0x01, 0x02, 0x03, 0x04};
        printBytes.accept("Sending data", dataBytes);
        HashMap<String, String> headers = new HashMap<>();
        headers.put("Content-Type", "application/octet-stream");
        mockServer.expect()
                .post()
                .withPath("/")
                .andReply(ResponseProviders.of(200, dataBytes, headers))
                .once();

        HttpURLConnection con;
        URL url = new URL(mockServerUrl);
        con = (HttpURLConnection) url.openConnection();
        con.setDoOutput(true);

        try (OutputStream out = con.getOutputStream();
             DataOutputStream dataOut = new DataOutputStream(new BufferedOutputStream(out))) {
            dataOut.write(dataBytes);
            dataOut.flush();
        }

        int responseCode = con.getResponseCode();
        if (responseCode >= 200 && responseCode < 300) {
            Map<String, List<String>> rxHeaders = con.getHeaderFields();
            for (Map.Entry<String, List<String>> entry : rxHeaders.entrySet()) {
                System.err.println("Received header: " + entry.getKey() + " -> " + entry.getValue());
            }
            int contentLength = con.getContentLength();
            try (InputStream in = con.getInputStream();
                 ByteArrayOutputStream out = new ByteArrayOutputStream(contentLength)) {
                int readByte;
                while ((readByte = in.read()) != -1) {
                    out.write(readByte);
                }
                out.flush();
                printBytes.accept("Received data", out.toByteArray());
            }
        }
        mockServer.shutdown();
    }

here is the output:

########## Sending data (bytes as hex): 01020304
Jun 18, 2024 11:28:10 AM okhttp3.mockwebserver.MockWebServer$2 execute
INFO: MockWebServer[57777] starting to accept connections
Jun 18, 2024 11:28:10 AM okhttp3.mockwebserver.MockWebServer$3 processOneRequest
INFO: MockWebServer[57777] received request: POST / HTTP/1.1 and responded: HTTP/1.1 200 OK
Received header: null -> [HTTP/1.1 200 OK]
Received header: Content-Length -> [10]
Received header: Content-Type -> [application/octet-stream]
########## Received data (bytes as hex): 224151494442413d3d22
Jun 18, 2024 11:28:10 AM okhttp3.mockwebserver.MockWebServer$2 acceptConnections
INFO: MockWebServer[57777] done accepting connections: Socket closed

Expected behavior

The mock server should return the exact data that was set up in the expectation, regardless of how the underlying implementation handles the data internally.

Runtime

other (please specify in additional context)

Kubernetes API Server version

other (please specify in additional context)

Environment

Linux

Fabric8 Kubernetes Client Logs

No response

Additional context

This is just the fabric8 DefaultMockServer without Kubernetes, although that is probably entirely apparent from the problem description, but it asked me to specify details in this field.

manusa commented 2 weeks ago

I don't think that the current MockServer implementations supports responses that can't be converted to Strings.

In version 7.0.0 we're going to switch the core library supporting the Kubernetes Mock Server implementation from OkHttp to Vert.x.

There is a PoC for the new implementation at https://github.com/fabric8io/kubernetes-client/pull/5632. You might want to check it out and see if it covers your use case. If you do, please provide feedback.

Steve973 commented 2 weeks ago

I did diagnose exactly what is going on, but I'm disappointed that I have not seen this in the documentation. I wonder if the newer versions of okhttp3 still have this problem, or if this was introduced in the fabric8 contribution. This is getting base64 encoded and wrapped in a string, and those are the bytes that are being sent, even including the quotes. What was the reason for doing it this way? That seems entirely un-intuitive. I can perfectly understand doing whatever conversion for internal representation and handling, but why not convert it back to the exact representation that was specified? Especially since there are many different media types, there is nothing that suggests that a response should be stringified.

manusa commented 2 weeks ago

What was the reason for doing it this way?

I'm not sure why this was implemented this way, so I can't reply to that.

Note that the main purpose of this module (at least when it was incepted) is to provide JSON responses emulating the Kubernetes API component. Also note that the Kube API has evolved a lot since the early days and has added support for newer protocols.

As I said before, it would be good if you could try the proposal in #5632 to see if there's something missing so that we can address it before releasing v7.0.0. In this case, this is just a port of what we already had based on OkHttp with some improvements. Si it's possible that your use case is still uncovered.

Steve973 commented 2 weeks ago

Ok. I just made a node in the comments to please make sure that the data arrives unmolested (though in less pedestrian wording).

Steve973 commented 2 weeks ago

I cannot even find where (in either fabric8 code, or in okhttp3 code) that this encoding is happening to the body data. I was going to try to see if a workaround is possible, but I won't be able to determine if it's possible unless I know where this occurs. Do you know what I am missing (besides a functioning brain)?

manusa commented 2 weeks ago

I cannot even find where (in either fabric8 code, or in okhttp3 code

This is probably part of the OkHttp implementation when the body of the mock response is set as a string (either in the mockwebserver or okio modules).

I just made a node in the comments

Have you actually tried the reproducer with the compiled changes in the PoC?

Steve973 commented 2 weeks ago

This is probably part of the OkHttp implementation when the body of the mock response is set as a string (either in the mockwebserver or okio modules).

I'm not setting anything as a String, so if that's the likely cause, then something else must be going on.

Have you actually tried the reproducer with the compiled changes in the PoC?

Not yet. Do these builds get deployed to any repo? Or is this something that I will need to build for myself?

manusa commented 2 weeks ago

I'm not setting anything as a String, so if that's the likely cause, then something else must be going on.

I know, but the original Kubernetes Mock Server implementation most likely is.

Not yet. Do these builds get deployed to any repo? Or is this something that I will need to build for myself?

Not for the pull requests. For this one you'd need to check out the branch, and install to your local Maven repo (mvn -DskipTests clean install). Then, update the dependency in your project to 7.0-SNAPSHOT)

Steve973 commented 2 weeks ago

I can use a Buffer with the okhttp3 version (that fabric8 is using) and it doesn't change anything about the body, so this is something that fabric8 is introducing at least by the usage, if not through specific code. This comment is just a bit of an aside, or FYI, and not related to building and using the snapshot.