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

X-Registry-Auth header sent to Docker Engine API contains field "authHeader" #42910

Closed
wants to merge 3 commits into from

Conversation

wickdynex
Copy link
Contributor

@wickdynex wickdynex commented Oct 28, 2024

Overview of Changes

  1. Add annotation @JsonIgnore on variable authHeader to avoid auto-serialization.
  2. Import package com.fasterxml.jackson.annotation.JsonIgnore to make annotation valid.
  3. run command ./gradlew checkstyleMain and ./gradlew format to fix style error.

Related Issues

Fixed #42905

Type of Change

  • Fix bug

Impact

Now when class DockerRegistryUserAuthentication and DockerRegistryTokenAuthentication use method createAuthHeader() would't add "authHeader" : null field.

Test

I've already test the class DockerRegistryUserAuthentication and DockerRegistryTokenAuthentication method getAuthHeader(), but only write rough test code not create a new branch to test. Do I need create a new branch?

Effect of Test

image Use decode tools to display the original string.

Expected Result:

{
  "username" : "Ben",
  "password" : "123455",
  "serveraddress" : "docker@example.com",
  "email" : "https://docker.example.com"
}

it works as expected.

Note

If anything I did worked not as expected, plz contact with me. Finally, looking forward to your reply.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Oct 28, 2024
@snicoll snicoll changed the title Fixed authHeader bug X-Registry-Auth header sent to Docker Engine API contains field "authHeader" Oct 29, 2024
@snicoll snicoll added the type: bug A general bug label Oct 29, 2024
@snicoll snicoll self-assigned this Oct 29, 2024
@snicoll snicoll removed the status: waiting-for-triage An issue we've not yet triaged label Oct 29, 2024
@snicoll snicoll added this to the 3.2.12 milestone Oct 29, 2024
@snicoll snicoll closed this in 7a185a0 Oct 29, 2024
@snicoll
Copy link
Member

snicoll commented Oct 29, 2024

@1328032567 thank you for making your first contribution to Spring Boot. To validate the auth header would not be included, I've changed the assertion of the "full" sample to be strict. See d4010d3.

@wickdynex
Copy link
Contributor Author

@1328032567 thank you for making your first contribution to Spring Boot. To validate the auth header would not be included, I've changed the assertion of the "full" sample to be strict. See d4010d3.

Alright, though you did't merge my branch(what a pity) but also used my modification. And finally I understand the true meaning of DockerRegistryUserAuthenticationTests and DockerRegistryTokenAuthenticationTests 🤣. And you change the test assert more strict, is that necessary? 🤔and why need to do this, could you plz explain it for me? Appreciate so much🥺.

@snicoll
Copy link
Member

snicoll commented Oct 29, 2024

though you did't merge my branch(what a pity) but also used my modification.

I did merge it, see 351018e. If you're worried about this not being recognized as a contribution of yours, See the tags next to your name on this page.

the test assert more strict, is that necessary

Nothing was validating that the auth header is gone. By using a strict assertion (true instead of false) the test will fail if there is an extra attribute present.

@wickdynex
Copy link
Contributor Author

though you did't merge my branch(what a pity) but also used my modification.

I did merge it, see 351018e. If you're worried about this not being recognized as a contribution of yours, See the tags next to your name on this page.

the test assert more strict, is that necessary

Nothing was validating that the auth header is gone. By using a strict assertion (true instead of false) the test will fail if there is an extra attribute present.

oh, I try to look through the doc for JSONAssert() api, and figure out the null attribution would also be valid through flag false, so we need to modify the flag into true. Thanks for your explanation🤩, Im very grateful.
Finally, about contribution, thanks for merging my PR #42910, Im overjoyed🥳.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

X-Registry-Auth header sent to Docker Engine API contains field "authHeader"
3 participants