Merge pull request #5696 from obsidiansystems/fix-5299

Fix #5299 and remove uncesssary unbounded buffer

src/libstore/daemon.cc

@@ -431,25 +431,30 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
                 hashAlgo = parseHashType(hashAlgoRaw);
             }
 
-            StringSink saved;
+            auto dumpSource = sinkToSource([&](Sink & saved) {
-            TeeSource savedNARSource(from, saved);
-            RetrieveRegularNARSink savedRegular { saved };
-
                 if (method == FileIngestionMethod::Recursive) {
-                /* Get the entire NAR dump from the client and save it to
+                    /* We parse the NAR dump through into `saved` unmodified,
-                  a string so that we can pass it to
+                       so why all this extra work? We still parse the NAR so
-                  addToStoreFromDump(). */
+                       that we aren't sending arbitrary data to `saved`
+                       unwittingly`, and we know when the NAR ends so we don't
+                       consume the rest of `from` and can't parse another
+                       command. (We don't trust `addToStoreFromDump` to not
+                       eagerly consume the entire stream it's given, past the
+                       length of the Nar. */
+                    TeeSource savedNARSource(from, saved);
                     ParseSink sink; /* null sink; just parse the NAR */
                     parseDump(sink, savedNARSource);
-            } else
+                } else {
+                    /* Incrementally parse the NAR file, stripping the
+                       metadata, and streaming the sole file we expect into
+                       `saved`. */
+                    RetrieveRegularNARSink savedRegular { saved };
                     parseDump(savedRegular, from);
-
-            logger->startWork();
                     if (!savedRegular.regular) throw Error("regular file expected");
-
+                }
-            // FIXME: try to stream directly from `from`.
+            });
-            StringSource dumpSource { *saved.s };
+            logger->startWork();
-            auto path = store->addToStoreFromDump(dumpSource, baseName, method, hashAlgo);
+            auto path = store->addToStoreFromDump(*dumpSource, baseName, method, hashAlgo);
             logger->stopWork();
 
             to << store->printStorePath(path);

src/libstore/local-store.cc

@@ -8,6 +8,7 @@
 #include "references.hh"
 #include "callback.hh"
 #include "topo-sort.hh"
+#include "finally.hh"
 
 #include <iostream>
 #include <algorithm>
@@ -1333,13 +1334,15 @@ StorePath LocalStore::addToStoreFromDump(Source & source0, const string & name,
         auto want = std::min(chunkSize, settings.narBufferSize - oldSize);
         dump.resize(oldSize + want);
         auto got = 0;
+        Finally cleanup([&]() {
+            dump.resize(oldSize + got);
+        });
         try {
             got = source.read(dump.data() + oldSize, want);
         } catch (EndOfFile &) {
             inMemory = true;
             break;
         }
-        dump.resize(oldSize + got);
     }
 
     std::unique_ptr<AutoDelete> delTempDir;