Skip to content
This repository has been archived by the owner on Oct 17, 2020. It is now read-only.

regex for sanitising is invalid #4

Closed
bgotink opened this issue Mar 7, 2016 · 7 comments
Closed

regex for sanitising is invalid #4

bgotink opened this issue Mar 7, 2016 · 7 comments

Comments

@bgotink
Copy link
Contributor

bgotink commented Mar 7, 2016

Regex /[",'].*\..*[",']:/ matches stuff like the following yaml file incorrectly:

'"this is": a': test

In yaml:

> require('js-yaml').safeLoad(require('fs').readFileSync('test.yml'))
{ '"this.is": a': 'test' }

In caml:

$ caml -f test

/usr/local/lib/node_modules/caml/node_modules/yaml-js/lib/scanner.js:504
            throw new exports.ScannerError(null, null, 'mapping values are not allowed here', this.get_mark());
            ^
mapping values are not allowed here
  on line 1, column 15
    at ScannerError.YAMLError [as constructor] (/usr/local/lib/node_modules/caml/node_modules/yaml-js/lib/errors.js:70:46)
    at ScannerError.MarkedYAMLError [as constructor] (/usr/local/lib/node_modules/caml/node_modules/yaml-js/lib/errors.js:90:45)
    at new ScannerError (/usr/local/lib/node_modules/caml/node_modules/yaml-js/lib/scanner.js:23:49)
    at Constructor.__dirname.Scanner.Scanner.fetch_value (/usr/local/lib/node_modules/caml/node_modules/yaml-js/lib/scanner.js:504:19)
    at Constructor.__dirname.Scanner.Scanner.fetch_more_tokens (/usr/local/lib/node_modules/caml/node_modules/yaml-js/lib/scanner.js:214:21)
    at Constructor.__dirname.Scanner.Scanner.check_token (/usr/local/lib/node_modules/caml/node_modules/yaml-js/lib/scanner.js:115:14)
    at Constructor.__dirname.Parser.Parser.parse_block_mapping_key (/usr/local/lib/node_modules/caml/node_modules/yaml-js/lib/parser.js:421:16)
    at Constructor.__dirname.Parser.Parser.check_event (/usr/local/lib/node_modules/caml/node_modules/yaml-js/lib/parser.js:61:48)
    at Constructor.__dirname.Composer.Composer.compose_mapping_node (/usr/local/lib/node_modules/caml/node_modules/yaml-js/lib/composer.js:143:20)
    at Constructor.__dirname.Composer.Composer.compose_node (/usr/local/lib/node_modules/caml/node_modules/yaml-js/lib/composer.js:91:21)
@kevin-smets
Copy link
Owner

Why would you create such a weird identifier :P. But yes, point taken. Caml is being reworked from the ground up to properly parse such exotic constructions (branch parser).

@kevin-smets kevin-smets added this to the 0.9.0 milestone Mar 7, 2016
@asgoth
Copy link
Contributor

asgoth commented Mar 8, 2016

To hijack the issue: sanitize is too eager. Using comments after a property causes the property to dissapear, e.g.

path:
  to:
    some: "value" # some explanation what this does
    other: "value"

Results in

{
  path: {
    to: {
      other: "value"
    }
  }
}

Property some is gone.

@kevin-smets
Copy link
Owner

Yes, the sanitizer is overly aggressive. For now a workaround is to make sure your comments are on a dedicated line.

@kevin-smets kevin-smets removed this from the 0.9.0 milestone May 11, 2016
@kevin-smets
Copy link
Owner

I've parked the parser for now, without proper documentation of yaml-js (connec/yaml-js#12) I simply do not have the time to rework this from the ground up for now.

@asgoth could you log this into a new issue? This might still be tackled in the current regex based setup.

@asgoth
Copy link
Contributor

asgoth commented May 12, 2016

Just use this one, no?

Edit: or you mean about the comments?

@bgotink
Copy link
Contributor Author

bgotink commented May 14, 2016

I believe the original issue here was already solved in #7 (by explicitly disallowing nested quotes in object keys.

@kevin-smets
Copy link
Owner

kevin-smets commented May 14, 2016

Alright, will close this one, I'll rise another issue from this one's ashes. Closed by #7

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants