diff --git a/src/apps.js b/src/apps.js index 133f67184..96aa0b4b1 100644 --- a/src/apps.js +++ b/src/apps.js @@ -240,13 +240,13 @@ function validatePortBindings(portBindings, manifest) { if (!portBindings) return null; - const tcpPorts = manifest.tcpPorts || { }; - const udpPorts = manifest.udpPorts || { }; + const tcpPorts = manifest.tcpPorts || {}; + const udpPorts = manifest.udpPorts || {}; for (const portName in portBindings) { if (!/^[A-Z0-9_]+$/.test(portName)) return new BoxError(BoxError.BAD_FIELD, `${portName} is not a valid environment variable in portBindings`); - const hostPort = portBindings[portName].hostPort; + const hostPort = portBindings[portName]; if (!Number.isInteger(hostPort)) return new BoxError(BoxError.BAD_FIELD, `${hostPort} is not an integer in ${portName} portBindings`); if (RESERVED_PORTS.indexOf(hostPort) !== -1) return new BoxError(BoxError.BAD_FIELD, `Port ${hostPort} for ${portName} is reserved in portBindings`); if (RESERVED_PORT_RANGES.find(range => (hostPort >= range[0] && hostPort <= range[1]))) return new BoxError(BoxError.BAD_FIELD, `Port ${hostPort} for ${portName} is reserved in portBindings`); @@ -1287,7 +1287,6 @@ async function install(data, auditSource) { const subdomain = data.subdomain.toLowerCase(), domain = data.domain.toLowerCase(), - portBindings = translatePortBindings(data.portBindings || null, data.manifest), accessRestriction = data.accessRestriction || null, memoryLimit = data.memoryLimit || 0, debugMode = data.debugMode || null, @@ -1312,8 +1311,9 @@ async function install(data, auditSource) { error = checkManifestConstraints(manifest); if (error) throw error; - error = validatePortBindings(portBindings, manifest); + error = validatePortBindings(data.portBindings || null, manifest); if (error) throw error; + const portBindings = translatePortBindings(data.portBindings || null, manifest); error = validateAccessRestriction(accessRestriction); if (error) throw error; @@ -1851,10 +1851,9 @@ async function setLocation(app, data, auditSource) { }; if ('portBindings' in data) { - values.portBindings = translatePortBindings(data.portBindings || null, app.manifest); - - error = validatePortBindings(values.portBindings, app.manifest); + error = validatePortBindings(data.portBindings || null, app.manifest); if (error) throw error; + values.portBindings = translatePortBindings(data.portBindings || null, app.manifest); } // rename the auto-created mailbox to match the new location @@ -2258,7 +2257,6 @@ async function clone(app, data, user, auditSource) { const subdomain = data.subdomain.toLowerCase(), domain = data.domain.toLowerCase(), - portBindings = data.portBindings || null, backupId = data.backupId, overwriteDns = 'overwriteDns' in data ? data.overwriteDns : false, skipDnsSetup = 'skipDnsSetup' in data ? data.skipDnsSetup : false; @@ -2276,8 +2274,6 @@ async function clone(app, data, user, auditSource) { const manifest = backupInfo.manifest, appStoreId = app.appStoreId; - const newPortBindings = translatePortBindings(data.portBindings, manifest); - let error = validateSecondaryDomains(data.secondaryDomains || {}, manifest); if (error) throw error; const secondaryDomains = translateSecondaryDomains(data.secondaryDomains || {}); @@ -2292,8 +2288,9 @@ async function clone(app, data, user, auditSource) { error = checkManifestConstraints(manifest); if (error) throw error; - error = validatePortBindings(newPortBindings, manifest); + error = validatePortBindings(data.portBindings || null, manifest); if (error) throw error; + const newPortBindings = translatePortBindings(data.portBindings || null, manifest); // should we copy the original app's mailbox settings instead? const mailboxName = manifest.addons?.sendmail ? mailboxNameForSubdomain(subdomain, manifest) : null; diff --git a/src/test/apps-test.js b/src/test/apps-test.js index 74285a698..2dd06c96d 100644 --- a/src/test/apps-test.js +++ b/src/test/apps-test.js @@ -38,33 +38,33 @@ describe('Apps', function () { describe('validatePortBindings', function () { it('does not allow invalid host port', function () { - expect(apps._validatePortBindings({ port: -1 }, { tcpPorts: { port: 5000 } })).to.be.an(Error); - expect(apps._validatePortBindings({ port: 0 }, { tcpPorts: { port: 5000 } })).to.be.an(Error); - expect(apps._validatePortBindings({ port: 'text' }, { tcpPorts: { port: 5000 } })).to.be.an(Error); - expect(apps._validatePortBindings({ port: 65536 }, { tcpPorts: { port: 5000 } })).to.be.an(Error); - expect(apps._validatePortBindings({ port: 470 }, { tcpPorts: { port: 5000 } })).to.be.an(Error); + expect(apps._validatePortBindings({ PORT: -1 }, { tcpPorts: { PORT: 5000 } })).to.be.an(Error); + expect(apps._validatePortBindings({ PORT: 0 }, { tcpPorts: { PORT: 5000 } })).to.be.an(Error); + expect(apps._validatePortBindings({ PORT: 'text' }, { tcpPorts: { PORT: 5000 } })).to.be.an(Error); + expect(apps._validatePortBindings({ PORT: 65536 }, { tcpPorts: { PORT: 5000 } })).to.be.an(Error); + expect(apps._validatePortBindings({ PORT: 470 }, { tcpPorts: { PORT: 5000 } })).to.be.an(Error); }); it('does not allow ports not as part of manifest', function () { - expect(apps._validatePortBindings({ port: 1567 }, { tcpPorts: { } })).to.be.an(Error); - expect(apps._validatePortBindings({ port: 1567 }, { tcpPorts: { port3: null } })).to.be.an(Error); + expect(apps._validatePortBindings({ PORT: 1567 }, { tcpPorts: { } })).to.be.an(Error); + expect(apps._validatePortBindings({ PORT: 1567 }, { tcpPorts: { port3: null } })).to.be.an(Error); }); it('does not allow reserved ports', function () { - expect(apps._validatePortBindings({ port: 443 }, { tcpPorts: { port: 5000 } })).to.be.an(Error); - expect(apps._validatePortBindings({ port: 50000 }, { tcpPorts: { port: 5000 } })).to.be.an(Error); - expect(apps._validatePortBindings({ port: 51000 }, { tcpPorts: { port: 5000 } })).to.be.an(Error); - expect(apps._validatePortBindings({ port: 50100 }, { tcpPorts: { port: 5000 } })).to.be.an(Error); + expect(apps._validatePortBindings({ PORT: 443 }, { tcpPorts: { PORT: 5000 } })).to.be.an(Error); + expect(apps._validatePortBindings({ PORT: 50000 }, { tcpPorts: { PORT: 5000 } })).to.be.an(Error); + expect(apps._validatePortBindings({ PORT: 51000 }, { tcpPorts: { PORT: 5000 } })).to.be.an(Error); + expect(apps._validatePortBindings({ PORT: 50100 }, { tcpPorts: { PORT: 5000 } })).to.be.an(Error); }); it('allows valid bindings', function () { - expect(apps._validatePortBindings({ port: 1024 }, { tcpPorts: { port: 5000 } })).to.be(null); + expect(apps._validatePortBindings({ PORT: 1024 }, { tcpPorts: { PORT: 5000 } })).to.be(null); expect(apps._validatePortBindings({ - port1: 4033, - port2: 3242, - port3: 1234 - }, { tcpPorts: { port1: {}, port2: {}, port3: {} } })).to.be(null); + PORT1: 4033, + PORT2: 3242, + PORT3: 1234 + }, { tcpPorts: { PORT1: {}, PORT2: {}, PORT3: {} } })).to.be(null); }); });