Merge pull request #14682 from duglin/Issue14621

Remove panic in nat package on invalid hostport
This commit is contained in:
David Calavera 2015-07-21 15:48:51 -07:00
commit 36106a20ca
11 changed files with 128 additions and 37 deletions

View file

@ -48,7 +48,11 @@ func (cli *DockerCli) CmdPort(args ...string) error {
proto = parts[1]
}
natPort := port + "/" + proto
if frontends, exists := c.NetworkSettings.Ports[nat.Port(port+"/"+proto)]; exists && frontends != nil {
newP, err := nat.NewPort(proto, port)
if err != nil {
return err
}
if frontends, exists := c.NetworkSettings.Ports[newP]; exists && frontends != nil {
for _, frontend := range frontends {
fmt.Fprintf(cli.out, "%s:%s\n", frontend.HostIp, frontend.HostPort)
}

View file

@ -521,7 +521,10 @@ func (container *Container) buildPortMapInfo(n libnetwork.Network, ep libnetwork
if expData, ok := driverInfo[netlabel.ExposedPorts]; ok {
if exposedPorts, ok := expData.([]types.TransportPort); ok {
for _, tp := range exposedPorts {
natPort := nat.NewPort(tp.Proto.String(), strconv.Itoa(int(tp.Port)))
natPort, err := nat.NewPort(tp.Proto.String(), strconv.Itoa(int(tp.Port)))
if err != nil {
return nil, fmt.Errorf("Error parsing Port value(%s):%v", tp.Port, err)
}
networkSettings.Ports[natPort] = nil
}
}
@ -534,7 +537,10 @@ func (container *Container) buildPortMapInfo(n libnetwork.Network, ep libnetwork
if portMapping, ok := mapData.([]types.PortBinding); ok {
for _, pp := range portMapping {
natPort := nat.NewPort(pp.Proto.String(), strconv.Itoa(int(pp.Port)))
natPort, err := nat.NewPort(pp.Proto.String(), strconv.Itoa(int(pp.Port)))
if err != nil {
return nil, err
}
natBndg := nat.PortBinding{HostIp: pp.HostIP.String(), HostPort: strconv.Itoa(int(pp.HostPort))}
networkSettings.Ports[natPort] = append(networkSettings.Ports[natPort], natBndg)
}
@ -710,7 +716,11 @@ func (container *Container) buildCreateEndpointOptions() ([]libnetwork.EndpointO
binding := bindings[port]
for i := 0; i < len(binding); i++ {
pbCopy := pb.GetCopy()
pbCopy.HostPort = uint16(nat.Port(binding[i].HostPort).Int())
newP, err := nat.NewPort(nat.SplitProtoPort(binding[i].HostPort))
if err != nil {
return nil, fmt.Errorf("Error parsing HostPort value(%s):%v", binding[i].HostPort, err)
}
pbCopy.HostPort = uint16(newP.Int())
pbCopy.HostIP = net.ParseIP(binding[i].HostIp)
pbList = append(pbList, pbCopy)
}

View file

@ -132,7 +132,13 @@ func (daemon *Daemon) verifyContainerSettings(hostConfig *runconfig.HostConfig,
for port := range hostConfig.PortBindings {
_, portStr := nat.SplitProtoPort(string(port))
if _, err := nat.ParsePort(portStr); err != nil {
return warnings, fmt.Errorf("Invalid port specification: %s", portStr)
return warnings, fmt.Errorf("Invalid port specification: %q", portStr)
}
for _, pb := range hostConfig.PortBindings[port] {
_, err := nat.NewPort(nat.SplitProtoPort(pb.HostPort))
if err != nil {
return warnings, fmt.Errorf("Invalid port specification: %q", pb.HostPort)
}
}
}
if hostConfig.LxcConf.Len() > 0 && !strings.Contains(daemon.ExecutionDriver().Name(), "lxc") {

View file

@ -158,7 +158,10 @@ func (daemon *Daemon) Containers(config *ContainersConfig) ([]*types.Container,
newC.Ports = []types.Port{}
for port, bindings := range container.NetworkSettings.Ports {
p, _ := nat.ParsePort(port.Port())
p, err := nat.ParsePort(port.Port())
if err != nil {
return err
}
if len(bindings) == 0 {
newC.Ports = append(newC.Ports, types.Port{
PrivatePort: p,
@ -167,7 +170,10 @@ func (daemon *Daemon) Containers(config *ContainersConfig) ([]*types.Container,
continue
}
for _, binding := range bindings {
h, _ := nat.ParsePort(binding.HostPort)
h, err := nat.ParsePort(binding.HostPort)
if err != nil {
return err
}
newC.Ports = append(newC.Ports, types.Port{
PrivatePort: p,
PublicPort: h,

View file

@ -872,6 +872,32 @@ func (s *DockerSuite) TestContainerApiCommitWithLabelInConfig(c *check.C) {
dockerCmd(c, "run", img.Id, "ls", "/test")
}
func (s *DockerSuite) TestContainerApiBadPort(c *check.C) {
config := map[string]interface{}{
"Image": "busybox",
"Cmd": []string{"/bin/sh", "-c", "echo test"},
"PortBindings": map[string]interface{}{
"8080/tcp": []map[string]interface{}{
{
"HostIp": "",
"HostPort": "aa80",
},
},
},
}
jsonData := bytes.NewBuffer(nil)
json.NewEncoder(jsonData).Encode(config)
status, b, err := sockRequest("POST", "/containers/create", config)
c.Assert(err, check.IsNil)
c.Assert(status, check.Equals, http.StatusInternalServerError)
if strings.TrimSpace(string(b)) != `Invalid port specification: "aa80"` {
c.Fatalf("Incorrect error msg: %s", string(b))
}
}
func (s *DockerSuite) TestContainerApiCreate(c *check.C) {
config := map[string]interface{}{
"Image": "busybox",

View file

@ -8,9 +8,15 @@ import (
"github.com/docker/docker/pkg/nat"
)
// Just to make life easier
func newPortNoError(proto, port string) nat.Port {
p, _ := nat.NewPort(proto, port)
return p
}
func TestLinkNaming(t *testing.T) {
ports := make(nat.PortSet)
ports[nat.Port("6379/tcp")] = struct{}{}
ports[newPortNoError("tcp", "6379")] = struct{}{}
link, err := NewLink("172.0.17.3", "172.0.17.2", "/db/docker-1", nil, ports)
if err != nil {
@ -40,7 +46,7 @@ func TestLinkNaming(t *testing.T) {
func TestLinkNew(t *testing.T) {
ports := make(nat.PortSet)
ports[nat.Port("6379/tcp")] = struct{}{}
ports[newPortNoError("tcp", "6379")] = struct{}{}
link, err := NewLink("172.0.17.3", "172.0.17.2", "/db/docker", nil, ports)
if err != nil {
@ -63,7 +69,7 @@ func TestLinkNew(t *testing.T) {
t.Fail()
}
for _, p := range link.Ports {
if p != nat.Port("6379/tcp") {
if p != newPortNoError("tcp", "6379") {
t.Fail()
}
}
@ -71,7 +77,7 @@ func TestLinkNew(t *testing.T) {
func TestLinkEnv(t *testing.T) {
ports := make(nat.PortSet)
ports[nat.Port("6379/tcp")] = struct{}{}
ports[newPortNoError("tcp", "6379")] = struct{}{}
link, err := NewLink("172.0.17.3", "172.0.17.2", "/db/docker", []string{"PASSWORD=gordon"}, ports)
if err != nil {
@ -112,9 +118,9 @@ func TestLinkEnv(t *testing.T) {
func TestLinkMultipleEnv(t *testing.T) {
ports := make(nat.PortSet)
ports[nat.Port("6379/tcp")] = struct{}{}
ports[nat.Port("6380/tcp")] = struct{}{}
ports[nat.Port("6381/tcp")] = struct{}{}
ports[newPortNoError("tcp", "6379")] = struct{}{}
ports[newPortNoError("tcp", "6380")] = struct{}{}
ports[newPortNoError("tcp", "6381")] = struct{}{}
link, err := NewLink("172.0.17.3", "172.0.17.2", "/db/docker", []string{"PASSWORD=gordon"}, ports)
if err != nil {
@ -161,9 +167,9 @@ func TestLinkMultipleEnv(t *testing.T) {
func TestLinkPortRangeEnv(t *testing.T) {
ports := make(nat.PortSet)
ports[nat.Port("6379/tcp")] = struct{}{}
ports[nat.Port("6380/tcp")] = struct{}{}
ports[nat.Port("6381/tcp")] = struct{}{}
ports[newPortNoError("tcp", "6379")] = struct{}{}
ports[newPortNoError("tcp", "6380")] = struct{}{}
ports[newPortNoError("tcp", "6381")] = struct{}{}
link, err := NewLink("172.0.17.3", "172.0.17.2", "/db/docker", []string{"PASSWORD=gordon"}, ports)
if err != nil {

View file

@ -29,8 +29,16 @@ type PortSet map[Port]struct{}
// 80/tcp
type Port string
func NewPort(proto, port string) Port {
return Port(fmt.Sprintf("%s/%s", port, proto))
func NewPort(proto, port string) (Port, error) {
// Check for parsing issues on "port" now so we can avoid having
// to check it later on.
portInt, err := ParsePort(port)
if err != nil {
return "", err
}
return Port(fmt.Sprintf("%d/%s", portInt, proto)), nil
}
func ParsePort(rawPort string) (int, error) {
@ -55,11 +63,15 @@ func (p Port) Port() string {
}
func (p Port) Int() int {
port, err := ParsePort(p.Port())
if err != nil {
panic(err)
portStr := p.Port()
if len(portStr) == 0 {
return 0
}
return port
// We don't need to check for an error because we're going to
// assume that any error would have been found, and reported, in NewPort()
port, _ := strconv.ParseUint(portStr, 10, 16)
return int(port)
}
// Splits a port in the format of proto/port
@ -152,7 +164,10 @@ func ParsePortSpecs(ports []string) (map[Port]struct{}, map[Port][]PortBinding,
if len(hostPort) > 0 {
hostPort = strconv.FormatUint(startHostPort+i, 10)
}
port := NewPort(strings.ToLower(proto), containerPort)
port, err := NewPort(strings.ToLower(proto), containerPort)
if err != nil {
return nil, nil, err
}
if _, exists := exposedPorts[port]; !exists {
exposedPorts[port] = struct{}{}
}

View file

@ -42,7 +42,11 @@ func TestParsePort(t *testing.T) {
}
func TestPort(t *testing.T) {
p := NewPort("tcp", "1234")
p, err := NewPort("tcp", "1234")
if err != nil {
t.Fatalf("tcp, 1234 had a parsing issue: %v", err)
}
if string(p) != "1234/tcp" {
t.Fatal("tcp, 1234 did not result in the string 1234/tcp")
@ -59,6 +63,11 @@ func TestPort(t *testing.T) {
if p.Int() != 1234 {
t.Fatal("port int value was not 1234")
}
p, err = NewPort("tcp", "asd1234")
if err == nil {
t.Fatal("tcp, asd1234 was supposed to fail")
}
}
func TestSplitProtoPort(t *testing.T) {

View file

@ -6,17 +6,23 @@ import (
"github.com/docker/docker/pkg/nat"
)
// Just to make life easier
func newPortNoError(proto, port string) nat.Port {
p, _ := nat.NewPort(proto, port)
return p
}
func TestCompare(t *testing.T) {
ports1 := make(nat.PortSet)
ports1[nat.Port("1111/tcp")] = struct{}{}
ports1[nat.Port("2222/tcp")] = struct{}{}
ports1[newPortNoError("tcp", "1111")] = struct{}{}
ports1[newPortNoError("tcp", "2222")] = struct{}{}
ports2 := make(nat.PortSet)
ports2[nat.Port("3333/tcp")] = struct{}{}
ports2[nat.Port("4444/tcp")] = struct{}{}
ports2[newPortNoError("tcp", "3333")] = struct{}{}
ports2[newPortNoError("tcp", "4444")] = struct{}{}
ports3 := make(nat.PortSet)
ports3[nat.Port("1111/tcp")] = struct{}{}
ports3[nat.Port("2222/tcp")] = struct{}{}
ports3[nat.Port("5555/tcp")] = struct{}{}
ports3[newPortNoError("tcp", "1111")] = struct{}{}
ports3[newPortNoError("tcp", "2222")] = struct{}{}
ports3[newPortNoError("tcp", "5555")] = struct{}{}
volumes1 := make(map[string]struct{})
volumes1["/test1"] = struct{}{}
volumes2 := make(map[string]struct{})

View file

@ -11,8 +11,8 @@ func TestMerge(t *testing.T) {
volumesImage["/test1"] = struct{}{}
volumesImage["/test2"] = struct{}{}
portsImage := make(nat.PortSet)
portsImage[nat.Port("1111/tcp")] = struct{}{}
portsImage[nat.Port("2222/tcp")] = struct{}{}
portsImage[newPortNoError("tcp", "1111")] = struct{}{}
portsImage[newPortNoError("tcp", "2222")] = struct{}{}
configImage := &Config{
ExposedPorts: portsImage,
Env: []string{"VAR1=1", "VAR2=2"},
@ -20,8 +20,8 @@ func TestMerge(t *testing.T) {
}
portsUser := make(nat.PortSet)
portsUser[nat.Port("2222/tcp")] = struct{}{}
portsUser[nat.Port("3333/tcp")] = struct{}{}
portsUser[newPortNoError("tcp", "2222")] = struct{}{}
portsUser[newPortNoError("tcp", "3333")] = struct{}{}
volumesUser := make(map[string]struct{})
volumesUser["/test3"] = struct{}{}
configUser := &Config{

View file

@ -261,7 +261,10 @@ func Parse(cmd *flag.FlagSet, args []string) (*Config, *HostConfig, *flag.FlagSe
return nil, nil, cmd, fmt.Errorf("Invalid range format for --expose: %s, error: %s", e, err)
}
for i := start; i <= end; i++ {
p := nat.NewPort(proto, strconv.FormatUint(i, 10))
p, err := nat.NewPort(proto, strconv.FormatUint(i, 10))
if err != nil {
return nil, nil, cmd, err
}
if _, exists := ports[p]; !exists {
ports[p] = struct{}{}
}