List like [[":,"]] is not parsed properly

[rix0rrr]

I wrote a property-based check for the yaml library using fast-check (https://github.com/dubzzz/fast-check) and it uncovered the following error:

> const YAML = require('yaml');
undefined
> YAML.stringify([[':,']])
'- - :,\n'
> YAML.parse('- - :,\n')
YAMLSyntaxError: Document is not valid YAML (bad indentation?)

I don't think this issue will impact us anytime soon, but you might want to be aware of it.

---

Here's my code (TypeScript) if you want to experiment more. Right now it will trigger mostly on the [[":,"]] bug and so will not make a lot of progress. I can only continue testing once that bug is fixed.

import fc = require('fast-check');
import YAML = require('yaml');
import deepEqual = require('deep-equal');
import { Arbitrary } from 'fast-check';

// Unfortunately it's hard to generate fully recursive type definitions
// so we'll unroll by hand a couple of times

function makeRecord<T, U>(a: Arbitrary<T>) {
  return fc.record({
    key1: a,
    key2: a
  });
}

const arbitraryJson0 = fc.oneof<string|number|boolean|null>(
  fc.string(),
  fc.integer(),
  fc.float(),
  fc.boolean(),
  fc.constant(null),
);


const arbitraryJson1 = fc.oneof<any>(
  arbitraryJson0,
  fc.array(arbitraryJson0),
  makeRecord(arbitraryJson0)
);

const arbitraryJson2 = fc.oneof<any>(
  arbitraryJson1,
  fc.array(arbitraryJson1),
  makeRecord(arbitraryJson1),
);

const arbitraryJson3 = fc.oneof<any>(
  arbitraryJson2,
  fc.array(arbitraryJson2),
  makeRecord(arbitraryJson2),
);


// Property #1: validate that stringifying and parsing are each other's inverse
// Do a manual sampling and test so that we can continue even if we find failing cases.
function isReversible(original: any) {
    const rendered = YAML.stringify(original);
    try {
      const parsed = YAML.parse(rendered);
      return deepEqual(original, parsed);
    } catch(e) {
      // Throwing is also bad
      return false;
    }
}

const N = 10000;

fc.assert(fc.property(arbitraryJson3, isReversible), {
  numRuns: N
});

[rix0rrr]

To be clear, here's my package.json:

{
  "name": "testyaml",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "keywords": [],
  "author": "",
  "license": "ISC",
  "dependencies": {
    "@types/node": "^10.12.12",
    "deep-equal": "^1.0.1",
    "fast-check": "^1.8.1",
    "typescript": "^3.2.2",
    "yaml": "^1.0.3"
  }
}

When source saved to index.ts, run as:

tsc index && node index

[eemeli]

Thank you for finding this! This was a bug in the parser, which was not properly checking the flow-in/flow-out context when parsing plain values that start with :,, which are only allowed outside of flow collections.

[dubzzz]

@rix0rrr @eemeli
Following the recent issues spotted by @rix0rrr and to prevent eventual regressions, I adapted a test of js-yaml using property based for your repo: dubzzz@3a8cb0a (I can issue a PR if you consider it can help detect problems earlier in the future)
Regards ;)

[eemeli]

@dubzzz Sure, do send in a PR: The test looks pretty good, and appears not to take too long to run. Presumably the inputs will vary between runs? As a small stylistic fix, the yaml prefixes on all the vars are a bit useless.

[dubzzz]

@eemeli

Indeed the inputs will vary from one run to another.
In case the test fails, it will give you back the seed causing the problem. You can reproduce the faulty run by simply re-using this seed - fc.assert(/* property */, { seed }).

In case of failure, the aim of such technic is to give you the minimal failing case and not horrible objects with thousands entries. So you might expect to have very clear and simple corner case to investigate if it happens to find an issue in the future.