Forbid this and arguments in server functions

vercel · Nov 21, 2024 · ed06a67 · ed06a67
1 parent 8a22b33
commit ed06a67
Show file tree

Hide file tree

Showing 10 changed files with 440 additions and 0 deletions.
diff --git a/crates/next-custom-transforms/src/transforms/server_actions.rs b/crates/next-custom-transforms/src/transforms/server_actions.rs
@@ -46,12 +46,23 @@ enum DirectiveLocation {
     FunctionBody,
 }
 
+#[derive(Clone, Debug)]
+enum ThisStatus {
+    Allowed,
+    Forbidden { directive: Directive },
+}
+
 #[derive(Clone, Debug)]
 enum ServerActionsErrorKind {
     ExportedSyncFunction {
         span: Span,
         in_action_file: bool,
     },
+    ForbiddenExpression {
+        span: Span,
+        expr: String,
+        directive: Directive,
+    },
     InlineSyncFunction {
         span: Span,
         directive: Directive,
@@ -113,6 +124,7 @@ pub fn server_actions<C: Comments>(file_name: &FileName, config: Config, comment
         in_callee: false,
         has_action: false,
         has_cache: false,
+        this_status: ThisStatus::Allowed,
 
         reference_index: 0,
         in_module_level: true,
@@ -163,6 +175,7 @@ struct ServerActions<C: Comments> {
     in_callee: bool,
     has_action: bool,
     has_cache: bool,
+    this_status: ThisStatus,
 
     reference_index: u32,
     in_module_level: bool,
@@ -1078,6 +1091,12 @@ impl<C: Comments> VisitMut for ServerActions<C> {
         let declared_idents_until = self.declared_idents.len();
         let current_names = take(&mut self.names);
 
+        if let Some(directive) = &directive {
+            self.this_status = ThisStatus::Forbidden {
+                directive: directive.clone(),
+            };
+        }
+
         // Visit children
         {
             let old_in_module = self.in_module_level;
@@ -1217,12 +1236,15 @@ impl<C: Comments> VisitMut for ServerActions<C> {
     }
 
     fn visit_mut_fn_decl(&mut self, f: &mut FnDecl) {
+        let old_this_status = self.this_status.clone();
+        self.this_status = ThisStatus::Allowed;
         let old_in_exported_expr = self.in_exported_expr;
         if self.in_module_level && self.exported_local_ids.contains(&f.ident.to_id()) {
             self.in_exported_expr = true
         }
         let old_fn_decl_ident = self.fn_decl_ident.replace(f.ident.clone());
         f.visit_mut_children_with(self);
+        self.this_status = old_this_status;
         self.in_exported_expr = old_in_exported_expr;
         self.fn_decl_ident = old_fn_decl_ident;
     }
@@ -1238,6 +1260,12 @@ impl<C: Comments> VisitMut for ServerActions<C> {
             },
         );
 
+        if let Some(directive) = &directive {
+            self.this_status = ThisStatus::Forbidden {
+                directive: directive.clone(),
+            };
+        }
+
         let declared_idents_until = self.declared_idents.len();
         let current_names = take(&mut self.names);
 
@@ -1350,20 +1378,27 @@ impl<C: Comments> VisitMut for ServerActions<C> {
             }
             PropOrSpread::Prop(box Prop::Method(MethodProp { key, .. })) => {
                 let key = key.clone();
+
                 if let PropName::Ident(ident_name) = &key {
                     self.arrow_or_fn_expr_ident = Some(ident_name.clone().into());
                 }
+
+                let old_this_status = self.this_status.clone();
+                self.this_status = ThisStatus::Allowed;
                 self.rewrite_expr_to_proxy_expr = None;
                 self.in_exported_expr = false;
                 n.visit_mut_children_with(self);
                 self.in_exported_expr = old_in_exported_expr;
+                self.this_status = old_this_status;
+
                 if let Some(expr) = &self.rewrite_expr_to_proxy_expr {
                     *n = PropOrSpread::Prop(Box::new(Prop::KeyValue(KeyValueProp {
                         key,
                         value: expr.clone(),
                     })));
                     self.rewrite_expr_to_proxy_expr = None;
                 }
+
                 return;
             }
             _ => {}
@@ -2174,6 +2209,28 @@ impl<C: Comments> VisitMut for ServerActions<C> {
         self.arrow_or_fn_expr_ident = old_arrow_or_fn_expr_ident;
     }
 
+    fn visit_mut_this_expr(&mut self, n: &mut ThisExpr) {
+        if let ThisStatus::Forbidden { directive } = &self.this_status {
+            emit_error(ServerActionsErrorKind::ForbiddenExpression {
+                span: n.span,
+                expr: "this".into(),
+                directive: directive.clone(),
+            });
+        }
+    }
+
+    fn visit_mut_ident(&mut self, n: &mut Ident) {
+        if n.sym == *"arguments" {
+            if let ThisStatus::Forbidden { directive } = &self.this_status {
+                emit_error(ServerActionsErrorKind::ForbiddenExpression {
+                    span: n.span,
+                    expr: "arguments".into(),
+                    directive: directive.clone(),
+                });
+            }
+        }
+    }
+
     noop_visit_mut_type!();
 }
 
@@ -2763,6 +2820,23 @@ fn emit_error(error_kind: ServerActionsErrorKind) {
                 }
             },
         ),
+        ServerActionsErrorKind::ForbiddenExpression {
+            span,
+            expr,
+            directive,
+        } => (
+            span,
+            formatdoc! {
+                r#"
+                    {subject} can not use `{expr}`.
+                "#,
+                subject = if let Directive::UseServer = directive {
+                    "Server Actions"
+                } else {
+                    "\"use cache\" functions"
+                }
+            },
+        ),
         ServerActionsErrorKind::InlineUseCacheInClientComponent { span } => (
             span,
             formatdoc! {

diff --git a/crates/next-custom-transforms/tests/errors/server-actions/server-graph/22/input.js b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/22/input.js
@@ -0,0 +1,50 @@
+async function a() {
+  'use cache'
+  // this is not allowed here
+  this.foo()
+  // arguments is not allowed here
+  console.log(arguments)
+
+  const b = async () => {
+    // this is not allowed here
+    this.foo()
+    // arguments is not allowed here
+    console.log(arguments)
+  }
+
+  function c() {
+    // this is allowed here
+    this.foo()
+    // arguments is allowed here
+    console.log(arguments)
+
+    const d = () => {
+      // this is allowed here
+      this.foo()
+      // arguments is allowed here
+      console.log(arguments)
+    }
+
+    const e = async () => {
+      'use server'
+      // this is not allowed here
+      this.foo()
+      // arguments is not allowed here
+      console.log(arguments)
+    }
+  }
+}
+
+export const api = {
+  result: null,
+  product: {
+    async fetch() {
+      'use cache'
+
+      // this is not allowed here
+      this.result = await fetch('https://example.com').then((res) => res.json())
+      // arguments is not allowed here
+      console.log(arguments)
+    },
+  },
+}
diff --git a/crates/next-custom-transforms/tests/errors/server-actions/server-graph/22/output.js b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/22/output.js
@@ -0,0 +1,55 @@
+/*#__TURBOPACK_DISABLE_EXPORT_MERGING__*/ /* __next_internal_action_entry_do_not_use__ {"006a88810ecce4a4e8b59d53b8327d7e98bbf251d7":"$$RSC_SERVER_ACTION_0","8069348c79fce073bae2f70f139565a2fda1c74c74":"$$RSC_SERVER_CACHE_2","80951c375b4a6a6e89d67b743ec5808127cfde405d":"$$RSC_SERVER_CACHE_1"} */ import { registerServerReference } from "private-next-rsc-server-reference";
+import { encryptActionBoundArgs, decryptActionBoundArgs } from "private-next-rsc-action-encryption";
+import { cache as $$cache__ } from "private-next-rsc-cache-wrapper";
+export const /*#__TURBOPACK_DISABLE_EXPORT_MERGING__*/ $$RSC_SERVER_ACTION_0 = async function e() {
+    // this is not allowed here
+    this.foo();
+    // arguments is not allowed here
+    console.log(arguments);
+};
+export var $$RSC_SERVER_CACHE_1 = $$cache__("default", "80951c375b4a6a6e89d67b743ec5808127cfde405d", 0, async function a() {
+    // this is not allowed here
+    this.foo();
+    // arguments is not allowed here
+    console.log(arguments);
+    const b = async ()=>{
+        // this is not allowed here
+        this.foo();
+        // arguments is not allowed here
+        console.log(arguments);
+    };
+    function c() {
+        // this is allowed here
+        this.foo();
+        // arguments is allowed here
+        console.log(arguments);
+        const d = ()=>{
+            // this is allowed here
+            this.foo();
+            // arguments is allowed here
+            console.log(arguments);
+        };
+        const e = registerServerReference($$RSC_SERVER_ACTION_0, "006a88810ecce4a4e8b59d53b8327d7e98bbf251d7", null);
+    }
+});
+Object.defineProperty($$RSC_SERVER_CACHE_1, "name", {
+    "value": "a",
+    "writable": false
+});
+var a = registerServerReference($$RSC_SERVER_CACHE_1, "80951c375b4a6a6e89d67b743ec5808127cfde405d", null);
+export var $$RSC_SERVER_CACHE_2 = $$cache__("default", "8069348c79fce073bae2f70f139565a2fda1c74c74", 0, /*#__TURBOPACK_DISABLE_EXPORT_MERGING__*/ async function fetch1() {
+    // this is not allowed here
+    this.result = await fetch('https://example.com').then((res)=>res.json());
+    // arguments is not allowed here
+    console.log(arguments);
+});
+Object.defineProperty($$RSC_SERVER_CACHE_2, "name", {
+    "value": "fetch",
+    "writable": false
+});
+export const api = {
+    result: null,
+    product: {
+        fetch: registerServerReference($$RSC_SERVER_CACHE_2, "8069348c79fce073bae2f70f139565a2fda1c74c74", null)
+    }
+};
diff --git a/crates/next-custom-transforms/tests/errors/server-actions/server-graph/22/output.stderr b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/22/output.stderr
@@ -0,0 +1,63 @@
+  x "use cache" functions can not use `this`.
+  | 
+   ,-[input.js:4:1]
+ 3 |   // this is not allowed here
+ 4 |   this.foo()
+   :   ^^^^
+ 5 |   // arguments is not allowed here
+   `----
+  x "use cache" functions can not use `arguments`.
+  | 
+   ,-[input.js:6:1]
+ 5 |   // arguments is not allowed here
+ 6 |   console.log(arguments)
+   :               ^^^^^^^^^
+   `----
+  x "use cache" functions can not use `this`.
+  | 
+    ,-[input.js:10:1]
+  9 |     // this is not allowed here
+ 10 |     this.foo()
+    :     ^^^^
+ 11 |     // arguments is not allowed here
+    `----
+  x "use cache" functions can not use `arguments`.
+  | 
+    ,-[input.js:12:1]
+ 11 |     // arguments is not allowed here
+ 12 |     console.log(arguments)
+    :                 ^^^^^^^^^
+ 13 |   }
+    `----
+  x Server Actions can not use `this`.
+  | 
+    ,-[input.js:31:1]
+ 30 |       // this is not allowed here
+ 31 |       this.foo()
+    :       ^^^^
+ 32 |       // arguments is not allowed here
+    `----
+  x Server Actions can not use `arguments`.
+  | 
+    ,-[input.js:33:1]
+ 32 |       // arguments is not allowed here
+ 33 |       console.log(arguments)
+    :                   ^^^^^^^^^
+ 34 |     }
+    `----
+  x "use cache" functions can not use `this`.
+  | 
+    ,-[input.js:45:1]
+ 44 |       // this is not allowed here
+ 45 |       this.result = await fetch('https://example.com').then((res) => res.json())
+    :       ^^^^
+ 46 |       // arguments is not allowed here
+    `----
+  x "use cache" functions can not use `arguments`.
+  | 
+    ,-[input.js:47:1]
+ 46 |       // arguments is not allowed here
+ 47 |       console.log(arguments)
+    :                   ^^^^^^^^^
+ 48 |     },
+    `----
diff --git a/crates/next-custom-transforms/tests/errors/server-actions/server-graph/23/input.js b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/23/input.js
@@ -0,0 +1,29 @@
+'use cache'
+
+// not exported!
+async function a() {
+  // this is allowed here
+  this.foo()
+  // arguments is allowed here
+  console.log(arguments)
+
+  const b = async () => {
+    'use server'
+    // this is not allowed here
+    this.foo()
+    // arguments is not allowed here
+    console.log(arguments)
+  }
+}
+
+export const obj = {
+  foo() {
+    return 42
+  },
+  bar() {
+    // this is allowed here
+    this.foo()
+    // arguments is allowed here
+    console.log(arguments)
+  },
+}
diff --git a/crates/next-custom-transforms/tests/errors/server-actions/server-graph/23/output.js b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/23/output.js
@@ -0,0 +1,28 @@
+/* __next_internal_action_entry_do_not_use__ {"006a88810ecce4a4e8b59d53b8327d7e98bbf251d7":"$$RSC_SERVER_ACTION_0"} */ import { registerServerReference } from "private-next-rsc-server-reference";
+import { encryptActionBoundArgs, decryptActionBoundArgs } from "private-next-rsc-action-encryption";
+import { cache as $$cache__ } from "private-next-rsc-cache-wrapper";
+export const /*#__TURBOPACK_DISABLE_EXPORT_MERGING__*/ $$RSC_SERVER_ACTION_0 = async function b() {
+    // this is not allowed here
+    this.foo();
+    // arguments is not allowed here
+    console.log(arguments);
+};
+// not exported!
+async function a() {
+    // this is allowed here
+    this.foo();
+    // arguments is allowed here
+    console.log(arguments);
+    const b = registerServerReference($$RSC_SERVER_ACTION_0, "006a88810ecce4a4e8b59d53b8327d7e98bbf251d7", null);
+}
+export const obj = {
+    foo () {
+        return 42;
+    },
+    bar () {
+        // this is allowed here
+        this.foo();
+        // arguments is allowed here
+        console.log(arguments);
+    }
+};