Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate string attrs have opening and closing quotes #160

Closed
wants to merge 1 commit into from

Conversation

nvanexan
Copy link
Contributor

@nvanexan nvanexan commented Aug 5, 2022

Closes #114

Copy link
Contributor

@mfix-stripe mfix-stripe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sweet! This looks good to me, but want to pass it to @rpaul-stripe for a quick review.

@rpaul-stripe
Copy link
Contributor

Hey, I'm sorry it's taken so long to respond to this PR. Your change to the validator raised some false positives on sample content and I had to set aside some time to dig into it in order to understand why.

I think the main problem is that it's checking every string-type attribute value instead of just the content attribute of raw text nodes. This results in false positives in several places, namely any fenced code block that contains a tag.

The real underlying problem here is with the way that Markdoc identifies the end of a tag construct in content. When it sees a {% in the text, it scans forward each character until it finds a matching %} that is not inside of a string. It does this in order to avoid prematurely identifying the tag end when it encounters tag-like syntax inside of a string attribute value. This means that if a tag has an unterminated string, the parser assumes that the actual closing is still inside of that string and it declines to match it as a tag.

What we should probably do is solve this in the markdown-it plugin and make the parser generate an error for the whole block any time it sees the tag start syntax without a matched end. I think this would be cleaner than trying to identify malformed tags during the validation process. I will tackle fixing this myself at some point in the next week or two.

For now, if you want to be able to catch these cases reliably during validation without a large surface area of potential false positives, I would suggest overriding the text schema node and doing the validation check there. Here's a quick-and-dirty example of something you can include in your Markdoc config to achieve this:

export const text: Schema = {
  attributes: {
    content: { type: String, required: true },
  },
  transform(node) {
    return node.attributes.content;
  },
  validate(node: Node, config: Config) {
    if (node.attributes?.content?.match?.(/{%[^%}]+%}/)) {
      return [{
        id: 'attribute-value-invalid',
        level: 'error',
        message:
          'The string attribute must have an opening and closing quote',
      }]

    return [];
  }

Thanks for raising this issue!

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

Successfully merging this pull request may close these issues.

Tag is ignored when double quote is missing in tag attribute
3 participants