Skip to content

Backport of Avoid decoding nil pointer in map walker into release/1.15.x#17053

Merged
hc-github-team-consul-core merged 1 commit into
release/1.15.xfrom
backport/NET-3664/decode-mapwalk-fix/weekly-major-aphid
Apr 19, 2023
Merged

Backport of Avoid decoding nil pointer in map walker into release/1.15.x#17053
hc-github-team-consul-core merged 1 commit into
release/1.15.xfrom
backport/NET-3664/decode-mapwalk-fix/weekly-major-aphid

Conversation

@hc-github-team-consul-core
Copy link
Copy Markdown
Collaborator

Backport

This PR is auto-generated from #17048 to be assessed for backporting due to the inclusion of the label backport/1.15.

The below text is copied from the body of the original PR.


Description

This PR fixes a bug with the mapwalk decoding logic used on the client agents for things like config entries and resolved service configs. Currently, if there is a struct stored within a map[string]interface{} with at least one field set and one pointer field unset/nil (such as EnforcingConsecutive5xx in the PassiveHealthCheck struct), the mapwalk logic here would attempt to call reflect.Value.Type() which is invalid for a nil pointer and we'd get reflect: call of reflect.Value.Type on zero Value.

Testing & Reproduction steps

Replication steps:

  1. Set up a server and client agent
  2. Register the following service config:
{
    "Kind": "service-defaults",
    "Name": "frontend",
    "Protocol": "http",
    "UpstreamConfig": {
        "Defaults": {
            "ConnectTimeoutMs": 15000,
            "PassiveHealthCheck": {
                "MaxFailures": 1000000
            },
            "MeshGateway": {}
        }
    }
}
  1. Attempt to register the following service on the client agent:
{
    "id": "frontend",
    "name": "frontend",
    "port": 9090,
    "checks": [
        {
            "Name": "check-9090-is-open",
            "TCP": "127.0.0.1:9090",
            "Interval": "10s"
        }
    ],
    "connect": {
        "sidecar_service": {
            "checks": [
                {
                    "Name": "check-9191-is-open",
                    "TCP": "127.0.0.1:9191",
                    "Interval": "10s"
                }
            ],
            "proxy": {
                "upstreams": [
                    {
                        "destination_name": "backend",
                        "local_bind_port": 9191
                    }
                ]
            }
        }
    }
}
  1. Registration should fail with reflect: call of reflect.Value.Type on zero Value

PR Checklist

  • updated test coverage
  • [ ] external facing docs updated
  • appropriate backport labels added
  • not a security concern

Overview of commits

@hc-github-team-consul-core hc-github-team-consul-core force-pushed the backport/NET-3664/decode-mapwalk-fix/weekly-major-aphid branch from d352a45 to e62111c Compare April 19, 2023 17:24
@hc-github-team-consul-core hc-github-team-consul-core enabled auto-merge (squash) April 19, 2023 17:24
@hc-github-team-consul-core hc-github-team-consul-core force-pushed the backport/NET-3664/decode-mapwalk-fix/weekly-major-aphid branch from d91b84d to 1fdbaa3 Compare April 19, 2023 17:24
@github-actions github-actions Bot added the theme/internals Serf, Raft, SWIM, Lifeguard, Anti-Entropy, locking topics label Apr 19, 2023
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Auto approved Consul Bot automated PR

@hc-github-team-consul-core hc-github-team-consul-core merged commit a9edaf9 into release/1.15.x Apr 19, 2023
@hc-github-team-consul-core hc-github-team-consul-core deleted the backport/NET-3664/decode-mapwalk-fix/weekly-major-aphid branch April 19, 2023 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

theme/internals Serf, Raft, SWIM, Lifeguard, Anti-Entropy, locking topics

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants