* Don't allow derivations with fixed and non-fixed outputs.

1 files changed, 38 insertions, 43 deletions

diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc
index c01bd6e54..11c67e138 100644
--- a/src/libexpr/primops.cc
+++ b/src/libexpr/primops.cc
@@ -394,53 +394,48 @@ static void prim_derivationStrict(EvalState & state, Value * * args, Value & v)
         throw EvalError(format("derivation names are not allowed to end in `%1%'")
             % drvExtension);
 
-    /* Construct the "masked" store derivation, which is the final one
-       except that in the list of outputs, the output paths are empty,
-       and the corresponding environment variables have an empty
-       value.  This ensures that changes in the set of output names do
-       get reflected in the hash.
-
-       However, for fixed-output derivations, we can compute the
-       output path directly, so we don't need this. */
-    bool fixedOnly = true;
-    foreach (StringSet::iterator, i, outputs) {
-        if (*i != "out" || outputHash == "") {
+    if (outputHash != "") {
+        /* Handle fixed-output derivations. */
+        if (outputs.size() != 1 || *(outputs.begin()) != "out")
+            throw Error("multiple outputs are not supported in fixed-output derivations");
+        
+        HashType ht = parseHashType(outputHashAlgo);
+        if (ht == htUnknown)
+            throw EvalError(format("unknown hash algorithm `%1%'") % outputHashAlgo);
+        Hash h(ht);
+        if (outputHash.size() == h.hashSize * 2)
+            /* hexadecimal representation */
+            h = parseHash(ht, outputHash);
+        else if (outputHash.size() == hashLength32(h))
+            /* base-32 representation */
+            h = parseHash32(ht, outputHash);
+        else
+            throw Error(format("hash `%1%' has wrong length for hash type `%2%'")
+                % outputHash % outputHashAlgo);
+        string s = outputHash;
+        outputHash = printHash(h);
+        if (outputHashRecursive) outputHashAlgo = "r:" + outputHashAlgo;
+        
+        Path outPath = makeFixedOutputPath(outputHashRecursive, ht, h, drvName);
+        drv.env["out"] = outPath;
+        drv.outputs["out"] = DerivationOutput(outPath, outputHashAlgo, outputHash);
+    }
+
+    else {
+        /* Construct the "masked" store derivation, which is the final
+           one except that in the list of outputs, the output paths
+           are empty, and the corresponding environment variables have
+           an empty value.  This ensures that changes in the set of
+           output names do get reflected in the hash. */
+        foreach (StringSet::iterator, i, outputs) {
             drv.env[*i] = "";
             drv.outputs[*i] = DerivationOutput("", "", "");
-            fixedOnly = false;
-        } else {
-            /* If an output hash was given, check it, and compute the
-               output path. */
-            HashType ht = parseHashType(outputHashAlgo);
-            if (ht == htUnknown)
-                throw EvalError(format("unknown hash algorithm `%1%'") % outputHashAlgo);
-            Hash h(ht);
-            if (outputHash.size() == h.hashSize * 2)
-                /* hexadecimal representation */
-                h = parseHash(ht, outputHash);
-            else if (outputHash.size() == hashLength32(h))
-                /* base-32 representation */
-                h = parseHash32(ht, outputHash);
-            else
-                throw Error(format("hash `%1%' has wrong length for hash type `%2%'")
-                    % outputHash % outputHashAlgo);
-            string s = outputHash;
-            outputHash = printHash(h);
-            if (outputHashRecursive) outputHashAlgo = "r:" + outputHashAlgo;
-            Path outPath = makeFixedOutputPath(outputHashRecursive, ht, h, drvName);
-            drv.env[*i] = outPath;
-            drv.outputs[*i] = DerivationOutput(outPath, outputHashAlgo, outputHash);
         }
-    }
 
-    /* Use the masked derivation expression to compute the output
-       path.  !!! Isn't it a potential security problem that the name
-       of each output path (including the suffix) isn't taken into
-       account?  For instance, changing the suffix for one path
-       (‘i->first == "out" ...’) doesn't affect the hash of the
-       others.  Is that exploitable? */
-    if (!fixedOnly) {
+        /* Use the masked derivation expression to compute the output
+           path. */
         Hash h = hashDerivationModulo(drv);
+        
         foreach (DerivationOutputs::iterator, i, drv.outputs)
             if (i->second.path == "") {
                 Path outPath = makeOutputPath(i->first, h, drvName);
@@ -448,7 +443,7 @@ static void prim_derivationStrict(EvalState & state, Value * * args, Value & v)
                 i->second.path = outPath;
             }
     }
-    
+
     /* Write the resulting term into the Nix store directory. */
     Path drvPath = writeDerivation(drv, drvName);