Skip to content

Conversation

@VahidRamezaniDB
Copy link
Contributor

The secure field is not being copied in the ChannelConfig.copy() method, which can lead to incorrect channel configuration when a channel config is copied after modifying the secure flag from its default value. For example while setting the spring.grpc.client.default-channel.secure to false and then call the getChannel() with an unknown name the secure field would still be set to true in the returned instance.

Changes

  • Added the field assignment to the copy method
  • Modified the unit test to verify it

Note

I also thought about how to enforce future extensions of the ChannelConfig to modify the copy method but I could think of nothing but using the java.land.reflect api which I currently think is not a preferred option in this project. But I will accept any other recomendations.

The copy method implemented in GrpcClientProperties.ChannelConfig was missing a class field. The <secure> field was not copied into the destination instance inside the copy method. This adds the missing field and also updates the unit tests to ensure the field is properly copied into the destination instance

Signed-off-by: Vahid Ramezani <vahid.ramezani.2014@gmail.com>
@onobc onobc added the bug Something isn't working label Dec 17, 2025
@onobc onobc added this to the 1.0.1 milestone Dec 17, 2025
@onobc onobc closed this in 8e7bf80 Dec 17, 2025
onobc added a commit that referenced this pull request Dec 17, 2025
Adds author tags to changes files in previous commit.

Original pull request #327

Signed-off-by: onobc <chris.bono@gmail.com>
@onobc
Copy link
Contributor

onobc commented Dec 17, 2025

👋🏻 @VahidRamezaniDB

Great 🐞 catch! Thanks for another great contribution.

Your original commit closed this PR. I also added a minor polishing commit.

I also thought about how to enforce future extensions of the ChannelConfig to modify the copy method but I could think of nothing but using the java.land.reflect api which I currently think is not a preferred option in this project. But I will accept any other recomendations.

Yes, I know that having the specific fields becomes a brittle maintenance point (this PR is the case and point of that). In general, Spring projects tend to avoid reflection for these cases and instead make sure we have proper test coverage. Another thing to note that we are likely to push back on any changes that are in the auto-configuration modules as they will move into Spring Boot 4.1 (#234).

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

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants