[log] Consistently escape log entries

Implicitly escape all output that is passed to Log::MessageBuilder.
We escape non-printable characters and the log field separator ','
using the \x00 and \u0000 escape sequences.

Example:
Before: event-foo,"space: ","comma: ,","double quotes: """
After:  event-foo,space: ,comma: \x2C,double quotes: "

This might slightly impact human readability of the log files in
extreme cases. However, most strings do not contain any escaped
characters.

Bug: 
Change-Id: Ic78f6d9932367d02f9f3c3f70b41b5c283bdf880
Reviewed-on: https://chromium-review.googlesource.com/728332
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48805}

src/log-utils.cc

@@ -51,19 +51,17 @@ Log::Log(Logger* logger, const char* file_name)
   // --prof implies --log-code.
   if (FLAG_prof) FLAG_log_code = true;
 
-  if (output_handle_ != nullptr) {
-    Log::MessageBuilder msg(this);
-    if (strlen(Version::GetEmbedder()) == 0) {
-      msg.Append("v8-version,%d,%d,%d,%d,%d", Version::GetMajor(),
-                 Version::GetMinor(), Version::GetBuild(), Version::GetPatch(),
-                 Version::IsCandidate());
-    } else {
-      msg.Append("v8-version,%d,%d,%d,%d,%s,%d", Version::GetMajor(),
-                 Version::GetMinor(), Version::GetBuild(), Version::GetPatch(),
-                 Version::GetEmbedder(), Version::IsCandidate());
-    }
-    msg.WriteToLogFile();
+  if (output_handle_ == nullptr) return;
+  Log::MessageBuilder msg(this);
+  LogSeparator kNext = LogSeparator::kSeparator;
+  msg << "v8-version" << kNext << Version::GetMajor() << kNext
+      << Version::GetMinor() << kNext << Version::GetBuild() << kNext
+      << Version::GetPatch();
+  if (strlen(Version::GetEmbedder()) != 0) {
+    msg << kNext << Version::GetEmbedder();
   }
+  msg << kNext << Version::IsCandidate();
+  msg.WriteToLogFile();
 }
 
 FILE* Log::Close() {
@@ -102,38 +100,16 @@ void Log::MessageBuilder::AppendVA(const char* format, va_list args) {
   Vector<char> buf(log_->format_buffer_, Log::kMessageBufferSize);
   int length = v8::internal::VSNPrintF(buf, format, args);
   // {length} is -1 if output was truncated.
-  if (length == -1) {
-    length = Log::kMessageBufferSize;
-  }
+  if (length == -1) length = Log::kMessageBufferSize;
   DCHECK_LE(length, Log::kMessageBufferSize);
   AppendStringPart(log_->format_buffer_, length);
 }
 
-
-void Log::MessageBuilder::AppendDoubleQuotedString(const char* string) {
-  OFStream& os = log_->os_;
-  // TODO(cbruni): unify escaping.
-  os << '"';
-  for (const char* p = string; *p != '\0'; p++) {
-    if (*p == '"') os << '\\';
-    os << *p;
-  }
-  os << '"';
-}
-
-void Log::MessageBuilder::AppendDoubleQuotedString(String* string) {
-  OFStream& os = log_->os_;
-  os << '"';
-  // TODO(cbruni): unify escaping.
-  AppendEscapedString(string);
-  os << '"';
-}
-
 void Log::MessageBuilder::Append(String* string) {
   DisallowHeapAllocation no_gc;  // Ensure string stay valid.
   std::unique_ptr<char[]> characters =
       string->ToCString(DISALLOW_NULLS, ROBUST_STRING_TRAVERSAL);
-  log_->os_ << characters.get();
+  AppendString(characters.get());
 }
 
 void Log::MessageBuilder::AppendAddress(Address addr) {
@@ -152,59 +128,115 @@ void Log::MessageBuilder::AppendSymbolName(Symbol* symbol) {
   os << "hash " << std::hex << symbol->Hash() << std::dec << ")";
 }
 
-
 void Log::MessageBuilder::AppendDetailed(String* str, bool show_impl_info) {
   if (str == nullptr) return;
   DisallowHeapAllocation no_gc;  // Ensure string stay valid.
   OFStream& os = log_->os_;
-  int len = str->length();
-  if (len > 0x1000) len = 0x1000;
+  int limit = str->length();
+  if (limit > 0x1000) limit = 0x1000;
   if (show_impl_info) {
     os << (str->IsOneByteRepresentation() ? 'a' : '2');
     if (StringShape(str).IsExternal()) os << 'e';
     if (StringShape(str).IsInternalized()) os << '#';
     os << ':' << str->length() << ':';
   }
-  AppendEscapedString(str, len);
+  AppendStringPart(str, limit);
 }
 
-void Log::MessageBuilder::AppendEscapedString(String* str) {
+void Log::MessageBuilder::AppendString(String* str) {
   if (str == nullptr) return;
   int len = str->length();
-  AppendEscapedString(str, len);
+  AppendStringPart(str, len);
+}
+
+void Log::MessageBuilder::AppendString(const char* string) {
+  for (const char* p = string; *p != '\0'; p++) {
+    this->AppendCharacter(*p);
+  }
 }
 
-void Log::MessageBuilder::AppendEscapedString(String* str, int len) {
+void Log::MessageBuilder::AppendStringPart(String* str, int len) {
   DCHECK_LE(len, str->length());
   DisallowHeapAllocation no_gc;  // Ensure string stay valid.
-  OFStream& os = log_->os_;
   // TODO(cbruni): unify escaping.
   for (int i = 0; i < len; i++) {
     uc32 c = str->Get(i);
-    if (c >= 32 && c <= 126) {
-      if (c == '\"') {
-        os << "\"\"";
-      } else if (c == '\\') {
-        os << "\\\\";
-      } else if (c == ',') {
-        os << "\\,";
-      } else {
-        os << static_cast<char>(c);
-      }
-    } else if (c > 0xff) {
-      Append("\\u%04x", c);
+    if (c <= 0xff) {
+      AppendCharacter(static_cast<char>(c));
     } else {
-      DCHECK(c < 32 || (c > 126 && c <= 0xff));
-      Append("\\x%02x", c);
+      // Escape any non-ascii range characters.
+      Append("\\u%04x", c);
     }
   }
 }
 
 void Log::MessageBuilder::AppendStringPart(const char* str, int len) {
-  log_->os_.write(str, len);
+  for (int i = 0; i < len; i++) {
+    DCHECK_NE(str[i], '\0');
+    this->AppendCharacter(str[i]);
+  }
+}
+
+void Log::MessageBuilder::AppendCharacter(char c) {
+  OFStream& os = log_->os_;
+  // A log entry (separate by commas) cannot contain commas or line-brakes.
+  if (c >= 32 && c <= 126) {
+    if (c == ',') {
+      // Escape commas directly.
+      os << "\x2c";
+    } else {
+      // Directly append any printable ascii character.
+      os << c;
+    }
+  } else {
+    // Escape any non-printable haracters.
+    Append("\\x%02x", c);
+  }
 }
 
 void Log::MessageBuilder::WriteToLogFile() { log_->os_ << std::endl; }
 
+template <>
+Log::MessageBuilder& Log::MessageBuilder::operator<<<const char*>(
+    const char* string) {
+  this->AppendString(string);
+  return *this;
+}
+
+template <>
+Log::MessageBuilder& Log::MessageBuilder::operator<<<char>(char c) {
+  this->AppendCharacter(c);
+  return *this;
+}
+
+template <>
+Log::MessageBuilder& Log::MessageBuilder::operator<<<String*>(String* string) {
+  this->AppendString(string);
+  return *this;
+}
+
+template <>
+Log::MessageBuilder& Log::MessageBuilder::operator<<<Symbol*>(Symbol* symbol) {
+  this->AppendSymbolName(symbol);
+  return *this;
+}
+
+template <>
+Log::MessageBuilder& Log::MessageBuilder::operator<<<Name*>(Name* name) {
+  if (name->IsString()) {
+    this->AppendString(String::cast(name));
+  } else {
+    this->AppendSymbolName(Symbol::cast(name));
+  }
+  return *this;
+}
+
+template <>
+Log::MessageBuilder& Log::MessageBuilder::operator<<<LogSeparator>(
+    LogSeparator separator) {
+  log_->os_ << ',';
+  return *this;
+}
+
 }  // namespace internal
 }  // namespace v8

src/log-utils.h

@@ -20,11 +20,12 @@ namespace internal {
 
 class Logger;
 
+enum class LogSeparator { kSeparator };
+
 // Functions and data for performing output of log messages.
 class Log {
  public:
   Log(Logger* log, const char* log_file_name);
-
   // Disables logging, but preserves acquired resources.
   void stop() { is_stopped_ = true; }
 
@@ -66,10 +67,6 @@ class Log {
     // Append string data to the log message.
     void PRINTF_FORMAT(2, 0) AppendVA(const char* format, va_list args);
 
-    // Append double quoted string to the log message.
-    void AppendDoubleQuotedString(const char* string);
-    void AppendDoubleQuotedString(String* string);
-
     // Append a heap string.
     void Append(String* str);
 
@@ -80,16 +77,18 @@ class Log {
 
     void AppendDetailed(String* str, bool show_impl_info);
 
-    // Append a portion of a string.
+    // Append and escape a full string.
+    void AppendString(String* source);
+    void AppendString(const char* string);
+
+    // Append and escpae a portion of a string.
+    void AppendStringPart(String* source, int len);
     void AppendStringPart(const char* str, int len);
 
-    // Helpers for appending char, C-string and heap string without
-    // buffering. This is useful for entries that can exceed the 2kB
-    // limit.
-    void AppendEscapedString(String* source);
-    void AppendEscapedString(String* source, int len);
+    void AppendCharacter(const char character);
 
     // Delegate insertion to the underlying {log_}.
+    // All appened srings are escaped to maintain one-line log entries.
     template <typename T>
     MessageBuilder& operator<<(T value) {
       log_->os_ << value;
@@ -136,6 +135,20 @@ class Log {
   friend class Logger;
 };
 
+template <>
+Log::MessageBuilder& Log::MessageBuilder::operator<<<LogSeparator>(
+    LogSeparator separator);
+template <>
+Log::MessageBuilder& Log::MessageBuilder::operator<<<const char*>(
+    const char* string);
+template <>
+Log::MessageBuilder& Log::MessageBuilder::operator<<<char>(char c);
+template <>
+Log::MessageBuilder& Log::MessageBuilder::operator<<<String*>(String* string);
+template <>
+Log::MessageBuilder& Log::MessageBuilder::operator<<<Symbol*>(Symbol* symbol);
+template <>
+Log::MessageBuilder& Log::MessageBuilder::operator<<<Name*>(Name* name);
 
 }  // namespace internal
 }  // namespace v8

src/log.h

@@ -14,6 +14,7 @@
 #include "src/base/platform/platform.h"
 #include "src/code-events.h"
 #include "src/isolate.h"
+#include "src/log-utils.h"
 #include "src/objects.h"
 
 namespace v8 {
@@ -95,6 +96,9 @@ class Logger : public CodeEventListener {
  public:
   enum StartEnd { START = 0, END = 1, STAMP = 2 };
 
+  // The separator is used to write an unescaped "," into the log.
+  static const LogSeparator kNext = LogSeparator::kSeparator;
+
   // Acquires resources for logging if the right flags are set.
   bool SetUp(Isolate* isolate);
 
@@ -277,8 +281,6 @@ class Logger : public CodeEventListener {
   void TickEvent(TickSample* sample, bool overflow);
   void RuntimeCallTimerEvent();
 
-  PRINTF_FORMAT(2, 3) void ApiEvent(const char* format, ...);
-
   // Logs a StringEvent regardless of whether FLAG_log is true.
   void UncheckedStringEvent(const char* name, const char* value);
 

test/cctest/test-log.cc

@@ -440,7 +440,7 @@ TEST(LogCallbacks) {
     ObjMethod1_entry = *FUNCTION_ENTRYPOINT_ADDRESS(ObjMethod1_entry);
 #endif
     i::EmbeddedVector<char, 100> ref_data;
-    i::SNPrintF(ref_data, ",0x%" V8PRIxPTR ",1,\"method1\"",
+    i::SNPrintF(ref_data, ",0x%" V8PRIxPTR ",1,method1",
                 reinterpret_cast<intptr_t>(ObjMethod1_entry));
     CHECK(logger.FindLine("code-creation,Callback,-2,", ref_data.start()));
   }
@@ -486,7 +486,7 @@ TEST(LogAccessorCallbacks) {
     Prop1Getter_entry = *FUNCTION_ENTRYPOINT_ADDRESS(Prop1Getter_entry);
 #endif
     EmbeddedVector<char, 100> prop1_getter_record;
-    i::SNPrintF(prop1_getter_record, ",0x%" V8PRIxPTR ",1,\"get prop1\"",
+    i::SNPrintF(prop1_getter_record, ",0x%" V8PRIxPTR ",1,get prop1",
                 reinterpret_cast<intptr_t>(Prop1Getter_entry));
     CHECK(logger.FindLine("code-creation,Callback,-2,",
                           prop1_getter_record.start()));
@@ -496,7 +496,7 @@ TEST(LogAccessorCallbacks) {
     Prop1Setter_entry = *FUNCTION_ENTRYPOINT_ADDRESS(Prop1Setter_entry);
 #endif
     EmbeddedVector<char, 100> prop1_setter_record;
-    i::SNPrintF(prop1_setter_record, ",0x%" V8PRIxPTR ",1,\"set prop1\"",
+    i::SNPrintF(prop1_setter_record, ",0x%" V8PRIxPTR ",1,set prop1",
                 reinterpret_cast<intptr_t>(Prop1Setter_entry));
     CHECK(logger.FindLine("code-creation,Callback,-2,",
                           prop1_setter_record.start()));
@@ -506,7 +506,7 @@ TEST(LogAccessorCallbacks) {
     Prop2Getter_entry = *FUNCTION_ENTRYPOINT_ADDRESS(Prop2Getter_entry);
 #endif
     EmbeddedVector<char, 100> prop2_getter_record;
-    i::SNPrintF(prop2_getter_record, ",0x%" V8PRIxPTR ",1,\"get prop2\"",
+    i::SNPrintF(prop2_getter_record, ",0x%" V8PRIxPTR ",1,get prop2",
                 reinterpret_cast<intptr_t>(Prop2Getter_entry));
     CHECK(logger.FindLine("code-creation,Callback,-2,",
                           prop2_getter_record.start()));

test/mjsunit/tools/csvparser.js

@@ -43,41 +43,44 @@ assertEquals(
     parser.parseLine('1997,Ford,E350'));
 
 assertEquals(
-    ['1997','Ford','E350'],
-    parser.parseLine('"1997","Ford","E350"'));
+    ['"', '\'', ',', '\n'],
+    parser.parseLine('",\',\\x2c,\\x0a'));
 
 assertEquals(
-    ['1997','Ford','E350','Super, luxurious truck'],
-    parser.parseLine('1997,Ford,E350,"Super, luxurious truck"'));
+    ['"1997"','Ford','E350'],
+    parser.parseLine('"1997",Ford,E350'));
+assertEquals(
+    ['1997', 'Ford', 'E350', 'Super', ' luxurious truck'],
+    parser.parseLine('1997,Ford,E350,Super, luxurious truck'));
 
 assertEquals(
     ['1997','Ford','E350','Super "luxurious" truck'],
-    parser.parseLine('1997,Ford,E350,"Super ""luxurious"" truck"'));
+    parser.parseLine('1997,Ford,E350,Super "luxurious" truck'));
 
 assertEquals(
     ['1997','Ford','E350','Super "luxurious" "truck"'],
-    parser.parseLine('1997,Ford,E350,"Super ""luxurious"" ""truck"""'));
+    parser.parseLine('1997,Ford,E350,Super "luxurious" "truck"'));
 
 assertEquals(
     ['1997','Ford','E350','Super "luxurious""truck"'],
-    parser.parseLine('1997,Ford,E350,"Super ""luxurious""""truck"""'));
+    parser.parseLine('1997,Ford,E350,Super "luxurious""truck"'));
 
 assertEquals(
     ['shared-library','/lib/ld-2.3.6.so','0x489a2000','0x489b7000'],
-    parser.parseLine('shared-library,"/lib/ld-2.3.6.so",0x489a2000,0x489b7000'));
+    parser.parseLine('shared-library,/lib/ld-2.3.6.so,0x489a2000,0x489b7000'));
 
 assertEquals(
     ['code-creation','LazyCompile','0xf6fe2d20','1201','APPLY_PREPARE native runtime.js:165'],
-    parser.parseLine('code-creation,LazyCompile,0xf6fe2d20,1201,"APPLY_PREPARE native runtime.js:165"'));
+    parser.parseLine('code-creation,LazyCompile,0xf6fe2d20,1201,APPLY_PREPARE native runtime.js:165'));
 
 assertEquals(
     ['code-creation','LazyCompile','0xf6fe4bc0','282',' native v8natives.js:69'],
-    parser.parseLine('code-creation,LazyCompile,0xf6fe4bc0,282," native v8natives.js:69"'));
+    parser.parseLine('code-creation,LazyCompile,0xf6fe4bc0,282, native v8natives.js:69'));
 
 assertEquals(
     ['code-creation','RegExp','0xf6c21c00','826','NccyrJroXvg\\/([^,]*)'],
-    parser.parseLine('code-creation,RegExp,0xf6c21c00,826,"NccyrJroXvg\\/([^,]*)"'));
+    parser.parseLine('code-creation,RegExp,0xf6c21c00,826,NccyrJroXvg\\x5C/([^\\x2C]*)'));
 
 assertEquals(
-    ['code-creation','Function','0x42f0a0','163',''],
+    ['code-creation','Function','0x42f0a0','163','""'],
     parser.parseLine('code-creation,Function,0x42f0a0,163,""'));

test/mjsunit/tools/tickprocessor-test-func-info.log

-shared-library,"shell",0x08048000,0x081ee000,0
-shared-library,"/lib32/libm-2.7.so",0xf7db6000,0xf7dd9000,0
-shared-library,"ffffe000-fffff000",0xffffe000,0xfffff000,0
-profiler,"begin",1
-code-creation,Stub,0,100,0x424260,348,"CompareStub_GE"
-code-creation,LazyCompile,0,101,0x2a8100,18535,"DrawQube 3d-cube.js:188",0xf43abcac,
-code-creation,LazyCompile,0,102,0x480100,3908,"DrawLine 3d-cube.js:17",0xf43abc50,
+shared-library,shell,0x08048000,0x081ee000,0
+shared-library,/lib32/libm-2.7.so,0xf7db6000,0xf7dd9000,0
+shared-library,ffffe000-fffff000,0xffffe000,0xfffff000,0
+profiler,begin,1
+code-creation,Stub,0,100,0x424260,348,CompareStub_GE
+code-creation,LazyCompile,0,101,0x2a8100,18535,DrawQube 3d-cube.js:188,0xf43abcac,
+code-creation,LazyCompile,0,102,0x480100,3908,DrawLine 3d-cube.js:17,0xf43abc50,
 tick,0x424284,0,0,0x480600,0,0x2aaaa5
 tick,0x42429f,0,0,0x480600,0,0x2aacb4
 tick,0x48063d,0,0,0x2d0f7c,0,0x2aaec6
-profiler,"end"
+profiler,end

test/mjsunit/tools/tickprocessor-test.log

-shared-library,"shell",0x08048000,0x081ee000,0
-shared-library,"/lib32/libm-2.7.so",0xf7db6000,0xf7dd9000,0
-shared-library,"ffffe000-fffff000",0xffffe000,0xfffff000,0
-profiler,"begin",1
-code-creation,Stub,0,100,0xf540a100,474,"CEntryStub"
-code-creation,Script,0,101,0xf541cd80,736,"exp.js"
-code-creation,Stub,0,102,0xf541d0e0,47,"RuntimeStub_Math_exp"
-code-creation,LazyCompile,0,103,0xf541d120,145,"exp native math.js:41"
+shared-library,shell,0x08048000,0x081ee000,0
+shared-library,/lib32/libm-2.7.so,0xf7db6000,0xf7dd9000,0
+shared-library,ffffe000-fffff000,0xffffe000,0xfffff000,0
+profiler,begin,1
+code-creation,Stub,0,100,0xf540a100,474,CEntryStub
+code-creation,Script,0,101,0xf541cd80,736,exp.js
+code-creation,Stub,0,102,0xf541d0e0,47,RuntimeStub_Math_exp
+code-creation,LazyCompile,0,103,0xf541d120,145,exp native math.js:41
 function-creation,0xf441d280,0xf541d120
-code-creation,LoadIC,0,104,0xf541d280,117,"j"
-code-creation,LoadIC,0,105,0xf541d360,63,"i"
+code-creation,LoadIC,0,104,0xf541d280,117,j
+code-creation,LoadIC,0,105,0xf541d360,63,i
 tick,0x80f82d1,0,0,0,0,0xf541ce5c
 tick,0x80f89a1,0,0,0,0,0xf541ce5c
 tick,0x8123b5c,0,0,0,0,0xf541d1a1,0xf541ceea
@@ -22,4 +22,4 @@ tick,0xf7dbc508,0,0,0,0,0xf541d1a1,0xf541ceea
 tick,0xf7dbff21,0,0,0,0,0xf541d1a1,0xf541ceea
 tick,0xf7edec90,0,0,0,0,0xf541d1a1,0xf541ceea
 tick,0xffffe402,0,0,0,0
-profiler,"end"
+profiler,end

tools/csvparser.js

@@ -29,50 +29,69 @@
 /**
  * Creates a CSV lines parser.
  */
-function CsvParser() {
-};
+class CsvParser {
+  /**
+   * Converts \x00 and \u0000 escape sequences in the given string.
+   *
+   * @param {string} input field.
+   **/
+  escapeField(string) {
+    let nextPos = string.indexOf("\\");
+    if (nextPos === -1) return string;
 
+    let result = string.substring(0, nextPos);
+    // Escape sequences of the form \x00 and \u0000;
+    let endPos = string.length;
+    let pos = 0;
+    while (nextPos !== -1) {
+      let escapeIdentifier = string.charAt(nextPos + 1);
+      pos = nextPos + 2;
+      if (escapeIdentifier == 'x') {
+        // \x00 ascii range escapes consume 2 chars.
+        nextPos = pos + 2;
+      } else {
+        // \u0000 unicode range escapes consume 4 chars.
+        nextPos = pos + 4;
+      }
+      // Convert the selected escape sequence to a single character.
+      let escapeChars = string.substring(pos, nextPos);
+      result += String.fromCharCode(parseInt(escapeChars, 16));
 
-/**
- * A regex for matching a CSV field.
- * @private
- */
-CsvParser.CSV_FIELD_RE_ = /^"((?:[^"]|"")*)"|([^,]*)/;
-
-
-/**
- * A regex for matching a double quote.
- * @private
- */
-CsvParser.DOUBLE_QUOTE_RE_ = /""/g;
-
+      // Continue looking for the next escape sequence.
+      pos = nextPos;
+      nextPos = string.indexOf("\\", pos);
+      // If there are no more escape sequences consume the rest of the string.
+      if (nextPos === -1) {
+        result += string.substr(pos);
+      } else if (pos != nextPos) {
+        result += string.substring(pos, nextPos);
+      }
+    }
+    return result;
+  }
 
-/**
- * Parses a line of CSV-encoded values. Returns an array of fields.
- *
- * @param {string} line Input line.
- */
-CsvParser.prototype.parseLine = function(line) {
-  var fieldRe = CsvParser.CSV_FIELD_RE_;
-  var doubleQuoteRe = CsvParser.DOUBLE_QUOTE_RE_;
-  var pos = 0;
-  var endPos = line.length;
-  var fields = [];
-  if (endPos > 0) {
-    do {
-      var fieldMatch = fieldRe.exec(line.substr(pos));
-      if (typeof fieldMatch[1] === "string") {
-        var field = fieldMatch[1];
-        pos += field.length + 3;  // Skip comma and quotes.
-        fields.push(field.replace(doubleQuoteRe, '"'));
+  /**
+   * Parses a line of CSV-encoded values. Returns an array of fields.
+   *
+   * @param {string} line Input line.
+   */
+  parseLine(line) {
+    var pos = 0;
+    var endPos = line.length;
+    var fields = [];
+    if (endPos == 0) return fields;
+    let nextPos = 0;
+    while(nextPos !== -1) {
+      nextPos = line.indexOf(',', pos);
+      let field;
+      if (nextPos === -1) {
+        field = line.substr(pos);
       } else {
-        // The second field pattern will match anything, thus
-        // in the worst case the match will be an empty string.
-        var field = fieldMatch[2];
-        pos += field.length + 1;  // Skip comma.
-        fields.push(field);
+        field = line.substring(pos, nextPos);
       }
-    } while (pos <= endPos);
+      fields.push(this.escapeField(field));
+      pos = nextPos + 1;
+    };
+    return fields
   }
-  return fields;
-};
+}