Fix File stat-before-open crash risk, JSON string-quoting bug, and MString buffer overflow

- File: stat() only after a successful open() (previously ran first with
  its result ignored, feeding a missing file's garbage st_size into a
  realloc - could crash the whole APIServer on a request for a
  nonexistent static asset)
- JString::setString(): always quote+escape the value instead of going
  through set()'s literal-value guess, which misclassified any string
  starting with digits (e.g. a UUID) as a bare numeric literal and wrote
  it unquoted, corrupting later edits to the same JSON document
- MString::remove() (both overloads): fixed an off-by-one that read one
  byte past the buffer on a completely full string (found via ASan under
  concurrent load)
- CGIFormData: added fields() so callers can iterate every parsed field
  without knowing names in advance (used to merge POST body into
  jet::Global::variables the same way query-string params already are)

CGIFormData.h

@@ -34,6 +34,14 @@ namespace coreutils {
 
       MString operator[](ZString name);
 
+      // Every parsed field, keyed as "name:N" (see storeValue's own doc
+      // comment below) - lets a caller (ApiSession::handleComplete, to
+      // merge POST body fields into jet::Global::variables the same way
+      // path params/query string already are - see its own doc comment)
+      // iterate every field without knowing its name in advance, which
+      // operator[] alone doesn't support.
+      const std::map<MString, MString> &fields() const { return data; }
+
     protected:
 
       Status parse() override;

File.cpp

@@ -17,19 +17,34 @@ namespace coreutils {
 
       this->fileName = fileName;
 
-      struct stat status;
-      stat(fileName.c_str(), &status);
-
       buffer = NULL;
 
-      setBufferSize(status.st_size);
-
       fd = ::open(fileName.c_str(), mode, authority);
       if(fd < 0) {
          std::stringstream temp;
          temp << "Error opening file " << fileName << ".";
          throw Exception(temp.str(), __FILE__, __LINE__);
       }
+
+      // stat() only after a successful open(), and only used to size the
+      // read buffer - previously this ran BEFORE the open()/fd check with
+      // its return value ignored, so a missing file left the stat struct fully
+      // uninitialized and fed that garbage st_size straight into
+      // setBufferSize()'s realloc(), before the open-failure path ever
+      // got a chance to throw its clean Exception. Found while adding a
+      // static-asset route (GET /assets/{filename}) that legitimately
+      // hits this path on every request for a nonexistent file - a
+      // garbage-sized realloc there could abort/crash the whole
+      // long-running APIServer process (shared with every other live
+      // route) instead of cleanly 500ing just that one request.
+      struct stat status;
+      if (fstat(fd, &status) != 0) {
+         std::stringstream temp;
+         temp << "Error statting file " << fileName << ".";
+         throw Exception(temp.str(), __FILE__, __LINE__);
+      }
+
+      setBufferSize(status.st_size);
    }
 
    File::~File() {

IMFMessage.cpp

@@ -7,5 +7,6 @@
 namespace coreutils {
 
    IMFMessage::IMFMessage(int fd) {}
+   IMFMessage::~IMFMessage() {}
 
 }

JString.cpp

@@ -474,7 +474,11 @@ namespace coreutils {
 	formatted << value;
       else
 	formatted << "\"" << jsonEscape(value).c_str() << "\"";
+      setFormatted(path, formatted);
+   }
 
+   void JString::setFormatted(ZString path, MString formatted) {
+      reset();
       if (locate(path)) {
 	 int startOffset = offset();
 	 skipValue();
@@ -491,8 +495,28 @@ namespace coreutils {
    }
 
    void JString::setString(ZString path, std::string value) {
+      // Deliberately NOT routed through set() (which calls isLiteralValue()
+      // to guess whether the raw text should be quoted) - setString() is
+      // explicitly, unambiguously a string setter, so the value must
+      // ALWAYS be quoted+escaped regardless of what it looks like.
+      // isLiteralValue() classifies anything ZString::startsWithNumber()
+      // accepts as a bare numeric literal - true for "42", but ALSO true
+      // for a string that merely STARTS with digits, like a UUID
+      // ("00146df8-e42c-11e7-9c78-525400b869dc"). Going through set()
+      // wrote that UUID completely unquoted; skipValue() then only
+      // recognizes the JSON-number PREFIX ("00146") as the value's real
+      // span when locating it for a later update, so the next setString()
+      // call spliced its own new key/value pair in right after "00146",
+      // leaving "df8-e42c-...-869dc" as orphaned text outside the object
+      // entirely - malformed JSON, confirmed via a real corrupted session
+      // file (jet-2.0's own native $[@name] session variables, backed by
+      // this class - see Global.h). Every other setString() caller
+      // sitewide was silently exposed to the same risk for any string
+      // value that happens to start with a digit.
       MString v(value);
-      set(path, v);
+      MString formatted;
+      formatted << "\"" << jsonEscape(v).c_str() << "\"";
+      setFormatted(path, formatted);
    }
 
    void JString::setNumber(ZString path, double value) {

JString.h

@@ -56,6 +56,18 @@ namespace coreutils {
 
       void set(ZString &path, ZString &value);
 
+      ///
+      /// Inserts/replaces path with already-formatted, valid JSON text
+      /// (a quoted+escaped string, or a bare number/bool/null literal) -
+      /// the locate()/skipValue()/insert() mechanics set() and setString()
+      /// both share, factored out so setString() can force string quoting
+      /// unconditionally instead of going through set()'s own
+      /// isLiteralValue() guess (see setString()'s own comment in the
+      /// .cpp for why that guess is wrong for a string setter).
+      ///
+
+      void setFormatted(ZString path, MString formatted);
+
       ///
       /// Use the find method to look through an object for member specified by
       /// the key in parameter 1. The cursor must be pointing to the { character

MString.cpp

@@ -266,15 +266,26 @@ namespace coreutils {
    }
    
    void MString::remove(int length) {
+      // Shifts the tail (everything after the removed range) down to close
+      // the gap. The loop must stop once ix + length reaches the end of the
+      // valid data - reading cursor[ix + length] past that point is a
+      // heap-buffer-overflow (confirmed via AddressSanitizer under
+      // concurrent APIServer load: a completely full MString hit exactly
+      // this on the final iteration, reading 1 byte past the allocation).
+      // The old condition `ix < len` let the loop run `length` iterations
+      // too far.
       int len = (data + this->length - cursor);
-      for (int ix = 0; ix < len; ix++)
+      for (int ix = 0; ix < len - length; ix++)
 	cursor[ix] = cursor[ix + length];
       setSize(this->length - length);
       onChange();
    }
 
    void MString::remove(int offset, int length) {
-      for (int ix = offset; ix < this->length; ix++)
+      // Same off-by-one as the single-argument overload above - the loop
+      // must stop once ix + length reaches this->length, not run all the
+      // way to this->length itself.
+      for (int ix = offset; ix < this->length - length; ix++)
 	getData()[ix] = getData()[ix + length];
       setSize(this->length - length);
       onChange();