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

fix(vdom): avoid executing root level script tags #11487

Merged
merged 1 commit into from Mar 30, 2021

Conversation

shadowings-zy
Copy link
Contributor

@shadowings-zy shadowings-zy commented Jul 3, 2020

What kind of change does this PR introduce? (check at least one)

  • [ x ] Bugfix
  • Feature
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change? (check one)

  • Yes
  • [ x ] No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

Other information:
Root level <script> tags should not be executed, for consistent behavior. So I remove the code in <script> tag when the <script> tag is the root element of the template.

fix #11483

Copy link
Member

@posva posva left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I think this should be done at the compilation level (like the other removal of script). Right now Vue.compile('<script />') still creates a render function that renders the script, but it should just be a render function that returns null

@shadowings-zy
Copy link
Contributor Author

shadowings-zy commented Jul 3, 2020

Aha! I've thought about fix this bug at the compilation level!
But when I read the code I found that if a render function that returns null, the template will render a div as a default element (related codes in file 'src/compiler/codegen/index.js', in 'generate' function). Does this meet our expectations?

If so, I will work on it.

@posva
Copy link
Member

posva commented Jul 3, 2020

if the render function returns null, Vue will output a comment node. But still, it is better that way, yes

@shadowings-zy
Copy link
Contributor Author

shadowings-zy commented Jul 3, 2020

Hi, now I fix this bug at the compilation level.
and Vue.compile('<script />') will creates a render function that returns null.

The output of the render function is as follows:

function anonymous(
) {
with(this){return null}
}

: )

@@ -45,7 +45,7 @@ export function generate (
options: CompilerOptions
): CodegenResult {
const state = new CodegenState(options)
const code = ast ? genElement(ast, state) : '_c("div")'
const code = ast ? (ast.tag === 'script' ? 'null' : genElement(ast, state) ) : '_c("div")'
Copy link
Member

Choose a reason for hiding this comment

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

can you add a comment above referencing the original issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, I will add the issue

@shadowings-zy
Copy link
Contributor Author

I had added a comment referencing the original issue in the PR.

: )

Copy link
Member

@posva posva left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link

@larson-carter larson-carter left a comment

Choose a reason for hiding this comment

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

These are great changes!

@posva posva removed the semver:minor label Sep 4, 2020
@posva posva changed the title fix(vdom): fix the bug 'root level <script> tags will be executed' in issue#11483 fix(vdom): avoid executing root level script tags Feb 24, 2021
@posva posva added this to In Review in 2.6.13 Feb 24, 2021
@posva posva moved this from In Review to Reviewed once, needs another review in 2.6.13 Mar 9, 2021
@posva posva merged commit fb16d7b into vuejs:dev Mar 30, 2021
@posva posva moved this from Reviewed once, needs another review to Done in 2.6.13 Mar 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Root level script tag should not be executed
5 participants