Skip to content

fix: Serialize ConsoleSize as JSON array to match Docker API format#67

Open
j4587698 wants to merge 2 commits intotestcontainers:mainfrom
j4587698:fix/console-size-serialization
Open

fix: Serialize ConsoleSize as JSON array to match Docker API format#67
j4587698 wants to merge 2 commits intotestcontainers:mainfrom
j4587698:fix/console-size-serialization

Conversation

@j4587698
Copy link

Summary

The ConsoleSize model was serialized as a JSON object {"Height":24,"Width":80}, but the Docker Engine API expects the Go [2]uint format — a JSON array [height, width]. This caused 400 Bad Request responses when passing ConsoleSize in exec create/start requests.

Changes

  • Added ConsoleSizeConverter (JsonConverter<ConsoleSize>) that serializes as [height, width] and deserializes from the same format. Follows the pattern of existing converters (Base64Converter, TimeSpanSecondsConverter).
  • Applied [JsonConverter(typeof(ConsoleSizeConverter))] to the ConsoleSize class.
  • Added 9 unit tests covering serialization, deserialization, round-trip, nested model serialization (ContainerExecCreateParameters, ContainerExecStartParameters), null handling, and edge cases (zero values, large values).

Copy link
Collaborator

@HofmeisterAn HofmeisterAn left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. We just need to adjust the implementation slightly.

The classes in src\Docker.DotNet\Models are auto-generated by specgen.

Currently, the properties receive the JsonConverter attribute during generation. specgen adds this attribute automatically.

We need to implement the same behavior for the ConsoleSizeConverter. Any manual changes made to the auto-generated classes will be lost.

Basically, you need to configure the ConsoleSize property for those two types (1, 2) in specgen, similar to the existing override for other properties. This will override the auto-generated configuration for that specific property. After that, run the specgen tool again to regenerate the model classes.

I'm not sure if specgen already supports adding attributes to the class instead of the property, but something like this should work:

typeToKey(reflect.TypeOf(ContainerExecStartParameters{})): {
    Properties: []CSProperty{
        {
            Name:       "ConsoleSize",
            Type:       CSType{"", "ConsoleSize", true},
            Attributes: []CSAttribute{{Type: CSType{"System.Text.Json.Serialization", "JsonConverter", false}, Arguments: []CSArgument{{Value: "typeof(ConsoleSizeConverter)"}}}},
        },
    },
},
.\tools\specgen\update-generated-code.ps1 -ReleaseTag docker-v29.1.5 

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants