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.

Sign up for GitHub

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

Jump to bottom

【WIP】Refactor/remove vgrammar #3748

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
Open

【WIP】Refactor/remove vgrammar #3748

wants to merge 7 commits into from

Conversation

xile611
Copy link
Contributor

@xile611 xile611 commented Feb 20, 2025

[中文版模板 / Chinese template]

🤔 This is a ...

  • New feature
  • Bug fix
  • TypeScript definition update
  • Bundle size optimization
  • Performance optimization
  • Enhancement feature
  • Refactoring
  • Update dependency
  • Code style optimization
  • Test Case
  • Branch merge
  • Release
  • Site / documentation update
  • Demo update
  • Workflow
  • Other (about what?)

🔗 Related issue link

🔗 Related PR link

🐞 Bugserver case id

💡 Background and solution

📝 Changelog

Language Changelog
🇺🇸 English
🇨🇳 Chinese

☑️ Self-Check before Merge

⚠️ Please check all items below before requesting a reviewing. ⚠️

  • Doc is updated/provided or not needed
  • Demo is updated/provided or not needed
  • TypeScript definition is updated/provided or not needed
  • Changelog is provided or not needed

🚀 Summary

copilot:summary

🔍 Walkthrough

copilot:walkthrough

@xile611 xile611 force-pushed the refactor/remove-vgrammar branch 5 times, most recently from 56136e9 to 2d22eb9 Compare February 24, 2025 08:18
@xile611 xile611 force-pushed the refactor/remove-vgrammar branch 9 times, most recently from 1cd8f99 to 5eefaad Compare February 26, 2025 07:59
github-advanced-security[bot]
github-advanced-security bot found potential problems Feb 26, 2025
View reviewed changes
packages/vchart/src/mark/base/base-mark.ts
const style = stateStyle[state] ?? { [styleKey]: {} };
stateStyle[state][styleKey] = {
const style = this.stateStyle[state] ?? { [styleKey]: {} };
this.stateStyle[state][styleKey] = {

Check warning

Code scanning / CodeQL

Prototype-polluting assignment Medium

This assignment may alter Object.prototype if a malicious '__proto__' string is injected from
library input
Loading
.
packages/vchart/src/mark/base/base-mark.ts
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
stateStyle[state][attr] = {
this.stateStyle[state][attr] = {

Check warning

Code scanning / CodeQL

Prototype-polluting assignment Medium

This assignment may alter Object.prototype if a malicious '__proto__' string is injected from
library input
Loading
.
packages/vchart/src/mark/base/base-mark.ts
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
stateStyle[state][key as keyof T] = stateStyle.normal[key];
this.stateStyle[state][key as keyof T] = this.stateStyle.normal[key];

Check warning

Code scanning / CodeQL

Prototype-polluting assignment Medium

This assignment may alter Object.prototype if a malicious '__proto__' string is injected from
library input
Loading
.

Copilot Autofix AI about 11 hours ago

To fix the prototype pollution issue, we need to ensure that the state parameter cannot be a special property name like __proto__, constructor, or prototype. We can achieve this by adding a check to validate the state parameter before using it as a key in the this.stateStyle object.

The best way to fix this problem without changing existing functionality is to add a validation check at the beginning of the setAttribute method. If the state parameter is one of the special property names, we can throw an error or handle it appropriately.

Suggested changeset 1
packages/vchart/src/mark/base/base-mark.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/vchart/src/mark/base/base-mark.ts b/packages/vchart/src/mark/base/base-mark.ts
--- a/packages/vchart/src/mark/base/base-mark.ts
+++ b/packages/vchart/src/mark/base/base-mark.ts
@@ -681,2 +681,5 @@
   ) {
+    if (state === '__proto__' || state === 'constructor' || state === 'prototype') {
+      throw new Error('Invalid state value');
+    }
     if (this.stateStyle[state] === undefined) {
EOF
@@ -681,2 +681,5 @@
) {
if (state === '__proto__' || state === 'constructor' || state === 'prototype') {
throw new Error('Invalid state value');
}
if (this.stateStyle[state] === undefined) {
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
@xile611 xile611 force-pushed the refactor/remove-vgrammar branch 6 times, most recently from 373bf46 to ad58075 Compare February 27, 2025 09:39
@xile611 xile611 force-pushed the refactor/remove-vgrammar branch from ad58075 to 168aff6 Compare February 27, 2025 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
typescript vchart
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@xile611