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

Transform arrow functions without block #108

Merged
merged 7 commits into from
Dec 12, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 75 additions & 0 deletions src/tests/arrow_func_tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
/**
* Unless explicitly stated otherwise all files in this repository are licensed under the Apache-2.0 License.
* This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2022 Datadog, Inc.
**/

#[cfg(test)]
mod tests {

use crate::{tests::rewrite_js, transform::transform_status::Status};
use speculoos::{assert_that, string::StrAssertions};

#[test]
fn test_arrow_simple_not_modified() -> Result<(), String> {
let original_code = "{const a = (arg) => arg ? true : false}".to_string();
let js_file = "test.js".to_string();
let rewritten = rewrite_js(original_code, js_file).map_err(|e| e.to_string())?;

assert_that(
&rewritten
.transform_status
.is_some_and(|status| status.status == Status::NotModified),
);

Ok(())
}

#[test]
fn test_arrow_paren_not_modified() -> Result<(), String> {
let original_code = "{const a = (arg) => (arg ? true : false)}".to_string();
let js_file = "test.js".to_string();
let rewritten = rewrite_js(original_code, js_file).map_err(|e| e.to_string())?;

assert_that(
&rewritten
.transform_status
.is_some_and(|status| status.status == Status::NotModified),
);

Ok(())
}

#[test]
fn test_arrow_block_not_modified() -> Result<(), String> {
let original_code = "{const a = (arg) => { return arg ? true : false }}".to_string();
let js_file = "test.js".to_string();
let rewritten = rewrite_js(original_code, js_file).map_err(|e| e.to_string())?;

assert_that(
&rewritten
.transform_status
.is_some_and(|status| status.status == Status::NotModified),
);

Ok(())
}

#[test]
fn test_arrow_modified() -> Result<(), String> {
let original_code = "{const a = (arg) => arg ? `hello ${arg}` : '' }".to_string();
let js_file = "test.js".to_string();
let rewritten = rewrite_js(original_code, js_file).map_err(|e| e.to_string())?;

assert_that(
&rewritten
.transform_status
.is_some_and(|status| status.status == Status::Modified),
);
assert_that(&rewritten.code).contains("const a = (arg)=>{
let __datadog_test_0;
return arg ? (__datadog_test_0 = arg, _ddiast.tplOperator(`hello ${__datadog_test_0}`, __datadog_test_0)) : '';
};");

Ok(())
}
}
1 change: 1 addition & 0 deletions src/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use crate::{
use anyhow::Error;
use std::path::PathBuf;

mod arrow_func_tests;
mod binary_assignation_test;
mod binary_expression_test;
mod literal_test;
Expand Down
45 changes: 45 additions & 0 deletions src/tests/template_literal_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,4 +96,49 @@ mod tests {
var f = (__datadog_test_0 = yield 'yielded', _ddiast.tplOperator(`foo${__datadog_test_0}bar`, __datadog_test_0));");
Ok(())
}

#[test]
fn test_template_literal_with_arrow() -> Result<(), String> {
let original_code = "function names(arg) {
const flag = arg;
const addPrefix = (value) => (flag ? value : `\"my_prefix_${value}\"`);
const result = `${addPrefix('NAME_0')}`;
return result;
}"
.to_string();

let js_file = "test.js".to_string();
let rewritten = rewrite_js(original_code, js_file).map_err(|e| e.to_string())?;

assert_that(&rewritten.code).contains(
"let __datadog_test_0;
const flag = arg;
const addPrefix = (value)=>{
let __datadog_test_0;",
);
Ok(())
}

#[test]
fn test_template_literal_with_paren() -> Result<(), String> {
let original_code = "function names(arg) {
const flag = arg;
const addPrefix = (value) => (flag ? value : `\"my_prefix_${value}\"`);
let a, b
const result = (a = 'NAME_0', b = 'NAME_1', `${addPrefix(a)}`);
return result;
}"
.to_string();

let js_file = "test.js".to_string();
let rewritten = rewrite_js(original_code, js_file).map_err(|e| e.to_string())?;

assert_that(&rewritten.code).contains(
"let __datadog_test_0;
const flag = arg;
const addPrefix = (value)=>{
let __datadog_test_0;",
);
Ok(())
}
}
33 changes: 33 additions & 0 deletions src/transform/arrow_transform.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/**
* Unless explicitly stated otherwise all files in this repository are licensed under the Apache-2.0 License.
* This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2022 Datadog, Inc.
**/
use swc_common::{SyntaxContext, DUMMY_SP};
use swc_ecma_ast::*;

use super::transform_status::TransformResult;

pub struct ArrowTransform {}

impl ArrowTransform {
pub fn to_dd_arrow_expr(arrow: &ArrowExpr) -> TransformResult<Expr> {
if arrow.body.is_expr() {
let mut arrow_block = arrow.clone();

let return_stmt = ReturnStmt {
span: DUMMY_SP,
arg: Some(Box::new(*arrow_block.body.expr().unwrap())),
};

arrow_block.body = Box::new(BlockStmtOrExpr::BlockStmt(BlockStmt {
span: DUMMY_SP,
ctxt: SyntaxContext::empty(),
stmts: vec![Stmt::Return(return_stmt)],
}));

return TransformResult::modified(Expr::Arrow(arrow_block));
}

TransformResult::not_modified()
}
}
1 change: 1 addition & 0 deletions src/transform/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* Unless explicitly stated otherwise all files in this repository are licensed under the Apache-2.0 License.
* This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2022 Datadog, Inc.
**/
pub(crate) mod arrow_transform;
pub(crate) mod assign_add_transform;
pub(crate) mod binary_add_transform;
pub(crate) mod call_expr_transform;
Expand Down
9 changes: 9 additions & 0 deletions src/visitor/operation_transform_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use swc_ecma_visit::{Visit, VisitMut, VisitMutWith};
use crate::{
telemetry::Telemetry,
transform::{
arrow_transform::ArrowTransform,
assign_add_transform::AssignAddTransform,
binary_add_transform::BinaryAddTransform,
call_expr_transform::CallExprTransform,
Expand Down Expand Up @@ -162,6 +163,14 @@ impl VisitMut for OperationTransformVisitor<'_> {
}
}

Expr::Arrow(arrow) => {
let transform_result = ArrowTransform::to_dd_arrow_expr(arrow);
if transform_result.is_modified() {
expr.map_with_mut(|e| transform_result.expr.unwrap_or(e));
// do not update status yet
}
}

_ => {
expr.visit_mut_children_with(self);
}
Expand Down
112 changes: 112 additions & 0 deletions test/resources/issue-101.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
'use strict'

function names(arg) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

For all the methods (why is not failing the linter?)

Suggested change
function names(arg) {
function names (arg) {

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

test/resources is ignored

But i'm going to rename the methods

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

const flag = arg
const addPrefix = (value) => (flag ? `"${value}"` : `"my_prefix.${value}"`)
const result = `
${addPrefix('NAME_0')}
${addPrefix('NAME_1')}
${addPrefix('NAME_2')}
${addPrefix('NAME_3')}
`
return result
}

function namesBlock(arg) {
const flag = arg
const addPrefix = (value) => {
return flag ? `"${value}"` : `"my_prefix.${value}"`
}
const result = `
${addPrefix('NAME_0')}
${addPrefix('NAME_1')}
${addPrefix('NAME_2')}
${addPrefix('NAME_3')}
`
return result
}

function namesNested(arg) {
const flag = arg
const addSufix = (value) => `${!value}_suffix`
const addPrefix = (value) => (flag ? `"${value}"` : `"my_prefix.${addSufix(value)}"`)
const result = `
${addPrefix('NAME_0')}
${addPrefix('NAME_1')}
${addPrefix('NAME_2')}
${addPrefix('NAME_3')}
`
return result
}

function namesDoubleResult(arg) {
const flag = arg
const addPrefix = (value) => (flag ? `"${value}"` : `"my_prefix.${value}"`)
const resultFake = `
${addPrefix('NAME_0')}
${addPrefix('NAME_1')}
${addPrefix('NAME_2')}
${addPrefix('NAME_3')}
`
const result = `
${addPrefix('NAME_0')}
${addPrefix('NAME_1')}
${addPrefix('NAME_2')}
${addPrefix('NAME_3')}
`
return result
}

function namesNoFlag() {
const addPrefix = (value) => `"my_prefix.${value}"`
const result = `
${addPrefix('NAME_0')}
${addPrefix('NAME_1')}
${addPrefix('NAME_2')}
${addPrefix('NAME_3')}
`
return result
}

function namesNoFlagDoubleResult() {
const addPrefix = (value) => `"my_prefix.${value}"`
const resultFake = `
${addPrefix('NAME_0')}
${addPrefix('NAME_1')}
${addPrefix('NAME_2')}
${addPrefix('NAME_3')}
`
const result = `
${addPrefix('NAME_0')}
${addPrefix('NAME_1')}
${addPrefix('NAME_2')}
${addPrefix('NAME_3')}
`
return result
}

function paren(arg) {
const flag = arg
let a, b, c
const addPrefix = (value) => (flag ? `"${value}"` : `"my_prefix.${value}"`)
const result =
((a = 'NAME_0'),
(b = 'NAME_1'),
(c = 'NAME_2'),
`
${addPrefix(a)}
${addPrefix(b)}
${addPrefix(c)}
`)
return result
}

module.exports = {
names,
namesBlock,
namesNested,
namesDoubleResult,
namesNoFlag,
namesNoFlagDoubleResult,
paren,
}
26 changes: 26 additions & 0 deletions test/template_literal.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -381,5 +381,31 @@ __datadog_test_0 + __datadog_test_1, __datadog_test_0, __datadog_test_1)), _ddia
rewriteAndCompare(testFunction, [1, 'b'])
rewriteAndCompare(testFunction, ['a', 2])
})

it('issue 101', () => {
// eslint-disable-next-line no-unused-vars
const _ddiast = {
plusOperator: (res) => res,
tplOperator: (res) => res
}

const js = readFileSync(path.join(__dirname, 'resources/issue-101.js')).toString()

// eslint-disable-next-line no-eval
const issue101 = eval(js)

Choose a reason for hiding this comment

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

🟠 Code Vulnerability

'eval' with argument of type Identifier (...read more)

The eval function could execute malicious code if used with non-literal values. The argument provided to the eval method could be used to execute malicious code. If an attacker manages to control the eval argument they can execute arbitrary code.

In JavaScript, the eval() function evaluates or executes an argument if it's a string of JavaScript code. If this argument is influenced by user input or other external sources, it can lead to security vulnerabilities. Specifically, if an attacker can control or manipulate the value of the variable in eval(variable), they can execute arbitrary code.

You should avoid using eval at all costs, but if you face an advanced use case, use literal values that are under your control or sanitize the input. However, even then it is still recommended to avoid the use of eval as it has led to security breaches before.

View in Datadog  Leave us feedback  Documentation


const rewritten = rewriteAst(js)

// eslint-disable-next-line no-eval
const rewrittenIssue101 = eval(rewritten)

Choose a reason for hiding this comment

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

🟠 Code Vulnerability

'eval' with argument of type Identifier (...read more)

The eval function could execute malicious code if used with non-literal values. The argument provided to the eval method could be used to execute malicious code. If an attacker manages to control the eval argument they can execute arbitrary code.

In JavaScript, the eval() function evaluates or executes an argument if it's a string of JavaScript code. If this argument is influenced by user input or other external sources, it can lead to security vulnerabilities. Specifically, if an attacker can control or manipulate the value of the variable in eval(variable), they can execute arbitrary code.

You should avoid using eval at all costs, but if you face an advanced use case, use literal values that are under your control or sanitize the input. However, even then it is still recommended to avoid the use of eval as it has led to security breaches before.

View in Datadog  Leave us feedback  Documentation


expect(issue101.names(false)).to.be.equal(rewrittenIssue101.names(false))
expect(issue101.namesBlock(false)).to.be.equal(rewrittenIssue101.namesBlock(false))
expect(issue101.namesNested(false)).to.be.equal(rewrittenIssue101.namesNested(false))
expect(issue101.namesDoubleResult(false)).to.be.equal(rewrittenIssue101.namesDoubleResult(false))
expect(issue101.namesNoFlag(false)).to.be.equal(rewrittenIssue101.namesNoFlag(false))
expect(issue101.namesNoFlagDoubleResult(false)).to.be.equal(rewrittenIssue101.namesNoFlagDoubleResult(false))
expect(issue101.paren(false)).to.be.equal(rewrittenIssue101.paren(false))
})
})
})
Loading