Skip to content

Fix initializing client modifying custom HTTPHeaders#2756

Merged
silvin-lubecki merged 1 commit intodocker:masterfrom
thaJeztah:fix_overwrite_headers
Sep 30, 2020
Merged

Fix initializing client modifying custom HTTPHeaders#2756
silvin-lubecki merged 1 commit intodocker:masterfrom
thaJeztah:fix_overwrite_headers

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

When initializing the API client, the User-Agent was added to any custom
HTTPHeaders that were configured. However, because the map was not properly
dereferenced, the original map was modified, causing the User-Agent to also
be saved to config.json after docker login and docker logout:

Before this change;

$ cat ~/.docker/config.json
cat: can't open '/root/.docker/config.json': No such file or directory

$ docker login -u myusername
Password:
...
Login Succeeded

$ cat ~/.docker/config.json
{
    "auths": {
        "https://index.docker.io/v1/": {
            "auth": "<base64 auth>"
        }
    },
    "HttpHeaders": {
        "User-Agent": "Docker-Client/19.03.12 (linux)"
    }
}

$ docker logout
{
    "auths": {},
    "HttpHeaders": {
        "User-Agent": "Docker-Client/19.03.12 (linux)"
    }
}

After this change:

$ cat ~/.docker/config.json
cat: can't open '/root/.docker/config.json': No such file or directory

$ docker login -u myusername
Password:
...
Login Succeeded

$ cat ~/.docker/config.json
{
    "auths": {
        "https://index.docker.io/v1/": {
            "auth": "<base64 auth>"
        }
    }
}

$ docker logout
Removing login credentials for https://index.docker.io/v1/

$ cat ~/.docker/config.json
{
    "auths": {}
}

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

When initializing the API client, the User-Agent was added to any custom
HTTPHeaders that were configured. However, because the map was not properly
dereferenced, the original map was modified, causing the User-Agent to also
be saved to config.json after `docker login` and `docker logout`:

Before this change;

    $ cat ~/.docker/config.json
    cat: can't open '/root/.docker/config.json': No such file or directory

    $ docker login -u myusername
    Password:
    ...
    Login Succeeded

    $ cat ~/.docker/config.json
    {
        "auths": {
            "https://index.docker.io/v1/": {
                "auth": "<base64 auth>"
            }
        },
        "HttpHeaders": {
            "User-Agent": "Docker-Client/19.03.12 (linux)"
        }
    }

    $ docker logout
    {
        "auths": {},
        "HttpHeaders": {
            "User-Agent": "Docker-Client/19.03.12 (linux)"
        }
    }

After this change:

    $ cat ~/.docker/config.json
    cat: can't open '/root/.docker/config.json': No such file or directory

    $ docker login -u myusername
    Password:
    ...
    Login Succeeded

    $ cat ~/.docker/config.json
    {
        "auths": {
            "https://index.docker.io/v1/": {
                "auth": "<base64 auth>"
            }
        }
    }

    $ docker logout
    Removing login credentials for https://index.docker.io/v1/

    $ cat ~/.docker/config.json
    {
        "auths": {}
    }

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah
Copy link
Copy Markdown
Member Author

@silvin-lubecki PTAL

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

Merging #2756 into master will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #2756      +/-   ##
==========================================
- Coverage   57.15%   57.15%   -0.01%     
==========================================
  Files         297      297              
  Lines       18657    18656       -1     
==========================================
- Hits        10663    10662       -1     
  Misses       7132     7132              
  Partials      862      862              

Copy link
Copy Markdown
Contributor

@silvin-lubecki silvin-lubecki left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for fixing, that bugged me for a long time 😄

@silvin-lubecki silvin-lubecki merged commit b4097f7 into docker:master Sep 30, 2020
@thaJeztah thaJeztah deleted the fix_overwrite_headers branch September 30, 2020 13:14
@thaJeztah thaJeztah added this to the 20.10.0 milestone Jan 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants