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: legacy octal escape is not permitted in strict mode #1736

Merged
merged 2 commits into from
Dec 27, 2024

Conversation

xusd320
Copy link
Contributor

@xusd320 xusd320 commented Dec 26, 2024

Close #1735

Summary by CodeRabbit

  • 新功能

    • 增强了错误处理逻辑,改进了AST解析过程中的错误过滤。
    • 新增测试用例以验证对旧八进制转义序列的处理。
    • 改进了内联CSS处理逻辑,确保CSS内容以JSON字符串格式嵌入JavaScript输出。
    • 新增CSS规则.a:before,为元素添加伪元素内容。
  • Bug修复

    • 确保在缺少依赖的情况下,返回适当的解析错误。
    • 更新了测试中的字符串检查方式,以确保正确匹配导入语句。

Copy link
Contributor

coderabbitai bot commented Dec 26, 2024

演练

此拉取请求主要修改了三个文件:crates/mako/src/ast/js_ast.rscrates/mako/src/build/parse.rse2e/fixtures/config.inline_css/src/a.css。更改涉及 JavaScript AST 解析的错误处理逻辑,特别是对旧八进制转义序列的处理。同时,还修改了内联 CSS 处理方式,增加了对 CSS 导入的处理和 JSON 序列化。此外,新增了 CSS 规则以增强样式。

变更

文件 变更摘要
crates/mako/src/ast/js_ast.rs 修改 JsAst::new 方法的错误处理,增加对 SyntaxError::LegacyOctal 的过滤,并新增测试 test_legacy_octal
crates/mako/src/build/parse.rs 调整内联 CSS 处理逻辑,移除 @imports,并使用 JSON 序列化 CSS 代码
e2e/fixtures/config.inline_css/src/a.css 添加 .a:before 伪元素,设置 content 属性为 "\002F"
e2e/fixtures/config.inline_css/expect.js 修改字符串检查的转义方式

序列图

sequenceDiagram
    participant Parser
    participant JsAst
    participant CSSHandler
    
    Parser->>JsAst: 解析 JavaScript 代码
    JsAst-->>Parser: 过滤特定语法错误
    Parser->>CSSHandler: 处理内联 CSS
    CSSHandler->>CSSHandler: 移除 @imports
    CSSHandler-->>Parser: 序列化 CSS 代码
Loading

对关联问题的评估

目标 是否解决 说明
处理严格模式下的旧八进制转义 [#1735]

可能相关的 PR

建议的审阅者

  • stormslowly
  • sorrycc

诗歌

🐰 代码如流水,错误如浪花
八进制转义,悄然被驯服
CSS 导入消散,JSON 闪耀光华
解析的魔法,兔子轻轻歌唱

Tip

CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command @coderabbitai generate docstrings to have CodeRabbit automatically generate docstrings for your pull request. We would love to hear your feedback on Discord.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 728e28f and 563a087.

📒 Files selected for processing (1)
  • e2e/fixtures/config.inline_css/expect.js (1 hunks)
🔇 Additional comments (1)
e2e/fixtures/config.inline_css/expect.js (1)

21-21: 建议检查转义方式在严格模式下的兼容性

请确保此处的双反斜杠 \\"//c\\" 是实际需要的转义形式,并且不会在严格模式下产生不兼容或意外的转义行为,可结合测试进一步验证。


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

Walkthrough

This pull request addresses an issue where legacy octal escapes were not permitted in strict mode. The changes involve modifying the handling of syntax errors to include SyntaxError::LegacyOctal and updating the code to ensure compatibility with strict mode. Additionally, a test case is added to verify the handling of legacy octal escapes.

Changes

Files Changed Summary
crates/mako/src/ast/js_ast.rs Updated to handle SyntaxError::LegacyOctal in strict mode and added a test case for legacy octal escape.
crates/mako/src/build/parse.rs Modified the moduleToDom function call to use serde_json::to_string for code.
e2e/fixtures/config.inline_css/src/a.css Added a CSS rule to include content with a legacy octal escape.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
crates/mako/src/ast/js_ast.rs (1)

279-286: 新增测试 test_legacy_octal 提升了对老式八进制转义的覆盖。

该测试用于验证在含有老式八进制转义的情况下不会崩溃,符合修复需求。若有可能,可再补充更多使用场景(如更复杂的字符串、模板字符串等)来提升健壮性。

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3dd6d9d and 728e28f.

📒 Files selected for processing (3)
  • crates/mako/src/ast/js_ast.rs (2 hunks)
  • crates/mako/src/build/parse.rs (1 hunks)
  • e2e/fixtures/config.inline_css/src/a.css (1 hunks)
🔇 Additional comments (3)
crates/mako/src/build/parse.rs (1)

126-129: 确保 format! 的占位符顺序与参数传递一致,并检查潜在注入风险。

这里使用 format! 同时填充 deps 与经序列化的 CSS 内容,请确认是否需要对 deps 做额外的转义或校验,以避免可能的注入风险。整体实现逻辑看起来符合内联 CSS 的需求,可以进一步验证在复杂场景下是否符合业务预期。

crates/mako/src/ast/js_ast.rs (1)

79-84: 注意忽略 SyntaxError::LegacyOctal 可能引发的兼容性影响。

在严格模式下,忽略该错误可以避免解析时的异常,但仍需确认是否会导致其他潜在问题,例如错误的语义被忽略。若业务需要全面禁止此类用法,或考虑后续添加更严格的检查。

e2e/fixtures/config.inline_css/src/a.css (1)

7-10: 引入 \002F 内容可能影响用户显示习惯。

该新增选择器使用 content: "\002F"; 会在元素前插入斜杠。若有多语言或样式定制需求,需确保此符号在各环境下可正确显示,或考虑在必要时灵活调整。

Copy link

codecov bot commented Dec 26, 2024

Codecov Report

Attention: Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.

Project coverage is 54.82%. Comparing base (3dd6d9d) to head (563a087).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
crates/mako/src/build/parse.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1736      +/-   ##
==========================================
+ Coverage   54.80%   54.82%   +0.01%     
==========================================
  Files         180      180              
  Lines       18045    18053       +8     
==========================================
+ Hits         9890     9897       +7     
- Misses       8155     8156       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@xusd320 xusd320 merged commit 7757bfe into master Dec 27, 2024
23 checks passed
@xusd320 xusd320 deleted the fix/legacy-octal branch December 27, 2024 02:59
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.

[BUG]: Legacy octal escape is not permitted in strict mode
2 participants