Skip to content

Use Base64Url instead of Base64 - #1146

Merged
KostyaSha merged 2 commits into
docker-java:masterfrom
PawelJ-PL:master
Jan 9, 2019
Merged

KostyaSha merged 2 commits into
docker-java:masterfrom
PawelJ-PL:master

Conversation

@PawelJ-PL

@PawelJ-PL PawelJ-PL commented Jan 4, 2019 •

Copy link
Copy Markdown
Contributor

Docker is using Base64Url to encode X-Registry-Auth header. It isn't documented anywhere, but we could check it in source code (for example here: https://github.com/docker/docker-ce/blob/10e40bd1548f69354a803a15fde1b672cc024b91/components/cli/cli/command/registry.go#L47).

Currently some passwords doesn't work with this library. For example password a!?testpassword will be encoded to YSE/dGVzdHBhc3N3b3Jk and will be rejected by Docker registry (because it should be encoded to YSE_dGVzdHBhc3N3b3Jk.


This change is Reviewable

@KostyaSha KostyaSha left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@kubukoz

kubukoz commented Jan 4, 2019

Copy link
Copy Markdown

(Closes #1145)

@KostyaSha

KostyaSha commented Jan 4, 2019 •

Copy link
Copy Markdown
Member

ALL tests failed (should fail only 2 variants), need to be investigated

@PawelJ-PL

Copy link
Copy Markdown
Contributor Author

It looks, that Base64 from Apache Commons omits padding. I've used Base64 from Guava instead.

@PawelJ-PL

Copy link
Copy Markdown
Contributor Author

Now almost all tests are green. There are some failed tests on 2 workers with Swarm, but similar errors are in previous pipelines.

@PawelJ-PL

Copy link
Copy Markdown
Contributor Author

Now it should be ok. Could You please merge it manually?

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.

3 participants