Liang Gong1, Michael Pradel2, Manu Sridharan3 and Koushik Sen1

1 UC Berkeley 2 TU Darmstadt 3 Samsung Research America

DLint: Dynamically Checking Bad Coding Practices in JavaScript

Why JavaScript?

… 2

• The RedMonk Programming Language Rankings (1st) – Based on GitHub and StackOverflow

• Web assembly language • Web applications, DSL, Desktop App, Mobile App

3

• Designed and Implemented in 10 days • Not all decisions were well thought • Problematic language features

– Error prone – Inefficient code – Security loophole

• Problematic features are retained – backward compatibility

Problematic JavaScript

4

Problematic JavaScript

• Good coding practices • informal rules • improve quality

• Better quality means: • Less correctness issues • Better performance • Better usability • Better maintainability • Less security loopholes • Less surprises • …

5

What is coding practice?

6

var sum = 0, value; var array = [11, 22, 33]; for (value in array) { sum += value; } > sum ?

Rule: avoid using for..in over arrays

7

var sum = 0, value; var array = [11, 22, 33]; for (value in array) { sum += value; } > sum ?

11 + 22 + 33 => 66 array index

(not array value) 0 + 1 + 2 => 3 array index : string 0+"0"+"1"+"2" => "0012"

Rule: avoid using for..in over arrays

• Cross-browser issues • Result depends on the Array prototype object

8

var sum = 0, value; var array = [11, 22, 33]; for (value in array) { sum += value; } > sum ?

11 + 22 + 33 => 66 array index

(not array value) 0 + 1 + 2 => 3 array index : string 0+"0"+"1"+"2" => "0012"

> "0012indexOftoString..."

Rule: avoid using for..in over arrays

9

var sum = 0, value; var array = [11, 22, 33]; for (value in array) { sum += value; } > sum ?

for (i=0; i < array.length; i++) { sum += array[i]; } function addup(element, index, array) { sum += element; } array.forEach(addup);

Rule: avoid using for..in over arrays

10

var sum = 0, value; var array = [11, 22, 33]; for (value in array) { sum += value; } > sum ?

for (i=0; i < array.length; i++) { sum += array[i]; } function addup(element, index, array) { sum += element; } array.forEach(addup);

Rule: avoid using for..in over arrays

Coding Practices and Lint Tools

• Existing Lint-like checkers – Inspect source code – Rule-based checking – Detect common mistakes – Enforce coding conventions

• Limitations: – Approximates behavior – Unknown aliases – Lint tools favor precision over soundness

• Difficulty: Precise static program analysis

11

12

• Dynamic Linter checking code quality rules for JS • Open-source, robust and extensible framework • Formalized and implemented 28 rules

– Counterparts of static rules – Additional rules

• Empirical study – Compare static and dynamic checking

DLint

a.f = b.g PutField(Read("a", a), "f", GetField(Read("b", b), "g")) if (a.f()) ... if (Branch(Method(Read("a", a), "f")()) ... x = y + 1 x = Write("x", Binary(‘+’,Read("y", y), Literal(1))

analysis.Literal(c) analysis.Branch(c) analysis.Read(n, x) analysis.Write(n, x) analysis.PutField(b, f, v) analysis.Binary(op, x, y) analysis.Function(f, isConstructor) analysis.GetField(b,f) analysis.Method(b, f, isConstructor) analysis.Unary(op, x) ...

13

Jalangi: A Dynamic Analysis Framework for JavaScript Koushik Sen, Swaroop Kalasapur, Tasneem Brutch, and Simon Gibbs

• Single-event: Stateless checking • Multi-event: Stateful checking

14

Runtime Patterns

15

Language Misuse

Avoid setting properties of primitives, which has no effect. var fact = 42; fact.isTheAnswer = true; console.log(fact.isTheAnswer); > undefined DLint Checker Predicate: propWrite(base,*,*) Λ isPrim(base)

16

Avoid producing NaN (Not a Number). var x = 23 – "five"; > NaN DLint Checker Predicate: unOp(*, val, NaN) Λ val ≠ NaN

binOp(*, left, right, NaN) Λ left ≠ NaN

Λ right ≠ NaN

call(*, *, args,NaN, *) Λ NaN ≠ args

Uncommon Values

17

Avoid concatenating undefined to string. var value; ... var str = "price: "; ... var result = str + value; > "price: undefined" DLint Checker Predicate: binOp(+, left, right, res) Λ (left = undefined ∨ right = undefined) Λ isString(res)

Uncommon Values

18

Beware that all wrapped primitives coerce to true. var b = false; if (new Boolean(b)) { console.log("true"); } > true

DLint Checker Predicate: cond(val) Λ isWrappedBoolean(val) Λ val.valueOf() = false

API Misuse

19

20

21

• Instrument SpiderMonkey to intercept JavaScript files • Transpile JavaScript code with Jalangi [Sen et al. FSE 2013] • DLint checks runtime states and find issues • Report reason and code location

22

JS program Instrumented JS program

Behavior Information Final Report

DLint Checkers

Jalangi

DLint Overview

23

Evaluation

Research Questions • DLint warning vs. JSHint warning? • Additional warnings from DLint? • Coding convention vs. page popularity?

Experimental Setup • 200 web sites (top 50 + others) • Comparison to JSHint

24

• Some sites: One approach finds all • Most sites: Better together

0% 20% 40% 60% 80% 100%

% of warnings on each site

Web

sites

JSHint Unique Common DLint Unique

% of Warnings: DLint vs. JSHint

25

• 53 warnings per page • 49 are missed by JSHint

0

1

2

3

4

I5 T6 L3 T5 A2 V2 L4 A5 T1 L2 L1 A6 A8 A3 T2 A4 I1 I4 V3 L5 I2 T4 L6 A7 T3Loga

rithm

ic a

vera

ge. #

w

arni

ng /

site

(bas

e 2)

DLint checkers

8

4

2

1

0

Checker shares warning with JSHint Checker shares no warnings with JSHint

Additional Warnings Reported by DLint

26

Correlation between Alexa popularity and number of DLint warnings: 0.6

00.0025

0.0050.0075

0.010.0125

0.0150.0175

0.02

# DL

int W

arn.

/ #C

ov. O

p.

Websites traffic ranking

Quartile 1-3

Mean

00.020.040.060.08

0.10.120.140.16

# JS

Hint

War

ning

/ LO

C

Websites traffic ranking

Quartile 1-3

Mean

Coding Convention vs. Page Popularity

Liang Gong, Electric Engineering & Computer Science, University of California, Berkeley. 27

Liang Gong, Electric Engineering & Computer Science, University of California, Berkeley. 28

29

Liang Gong, Electric Engineering & Computer Science, University of California, Berkeley. 30

31

var decode64 = ""; if (dataLength > 3 && dataLength % 4 == 0) { while (index < dataLength) { decode64 += String.fromCharCode(...); } if (sixbits[3] >= 0x40) { decode64.length -= 1; } }

From Google Octane Game Boy Emulator benchmark:

Rule: avoid setting field on primitive values

32

var decode64 = ""; if (dataLength > 3 && dataLength % 4 == 0) { while (index < dataLength) { decode64 += String.fromCharCode(...); } if (sixbits[3] >= 0x40) { decode64.length -= 1; } }

From Google Octane Game Boy Emulator benchmark:

No effect because decode64 is a primitive string.

Rule: avoid setting field on primitive values

33

window.onbeforeunload= "Twitch.player.getPlayer().pauseVideo();" window.onunload= "Twitch.player.getPlayer().pauseVideo();"

Rule: avoid no effect operations

34

window.onbeforeunload= "Twitch.player.getPlayer().pauseVideo();" window.onunload= "Twitch.player.getPlayer().pauseVideo();"

window.onbeforeunload = function () { Twitch.player.getPlayer().pauseVideo(); }

Rule: avoid no effect operations

35

Takeaways

Dynamic lint-like checking for JavaScript • Static checkers are not sufficient, DLint complements • DLint is a open-source, robust and extensible tool

– Works on real-world websites – Found 19 clear bugs on most popular websites

More information: • Paper: “DLint: Dynamically Checking Bad Coding Practices in JavaScript” • Source Code: https://github.com/Berkeley-Correctness-Group/DLint • Google “DLint Berkeley”

36

Takeaways

Thanks!

Dynamic lint-like checking for JavaScript • Static checkers are not sufficient, DLint complements • DLint is a open-source, robust and extensible tool

– Works on real-world websites – Found 19 clear bugs on most popular websites

More information: • Paper: “DLint: Dynamically Checking Bad Coding Practices in JavaScript” • Source Code: https://github.com/Berkeley-Correctness-Group/DLint • Google “DLint Berkeley”

• propWrite(base, name, val) • propRead(base, name, val) • cond(val) • unOp(op, val, res) • binOp(op, left, right, res) • call(base, f, args, ret, isConstr)

1. Predicates over runtime events

Example: propWrite(*, "myObject", val) | isPrim(val)

Formalization: declarative specification

37

38

74

359

191

416

1401

62

202

2335

4933

79

205

62

125

167

6

15

26 8

0% 10% 20% 30% 40% 50% 60% 70% 80% 90% 100%

Do not use Number, Boolean, String as a constructor.

The Function constructor is a form of eval.

The object literal notation {} is preferable.

The array literal notation [] is preferable.

document.write can be a form of eval.

Do not override built-in variables.

Implied eval (string instead of function as argument).

eval can be harmful.

Missing 'new' prefix when invoking a constructor.

Fount by JSHint Only Common with DLint

39

E.g., 181 calls of eval(), Function(), etc. missed by JSHint

31

77

181

74

833

79

187

413

6

8

0% 20% 40% 60% 80% 100%

WrappedPrimitives

Literals

DoubleEvaluation

ArgumentsVariable

ConstructorFunctions

A8

L4A

1L1

T5

Found by Dlint Only Common with JSHint

Liang Gong, Electric Engineering & Computer Science, University of California, Berkeley. 40

Liang Gong, Electric Engineering & Computer Science, University of California, Berkeley. 41

42

43

NaN case study for IKEA

44

<prices> <normal> <priceNormal unformatted="9.9">$9.90</priceNormal> <pricePrevious /> <priceNormalPerUnit /> <pricePreviousPerUnit /> </normal> </prices>

previousPrice = getElementValue("pricePrevious" + suffix, normal);

parseFloat(previousPrice).toFixed(2).replace(...) .replace('.00', ''));

XML: Empty Element

JS: undefined

JS: NaN

Avoid accessing the undefined property.

var x; // undefined var y = {}; y[x] = 23; // { undefined: 23 }

propWrite(*, "undefined",*) propRead(*, "undefined", *)

45

Type Related Checker

Chained Analysis

46

PutField(Read("a", a), "f", GetField(Read("b", b), "g"))

a.f = b.g

functions

Chained Analysis

PutField

Read

…

functions

Checker-1

PutField

Read

… functions

Checker-2

PutField

Read

… functions

Checker-n

PutField

Read

…

…

www.google.com/chrome, included code from Modernizr:

https://github.com/Modernizr/Modernizr/pull/1419

47

Rule: avoid using for..in on arrays

for (i in props) { // props is an array prop = props[i]; before = mStyle.style[prop]; ... }

eval is evil, do not use eval.

48

var fun = eval; ... fun("var a = 1;");

API Misuse

call(builtin, eval, ∗, ∗, ∗) call(builtin, Function, ∗, ∗, ∗) call(builtin, setTimeout, args, ∗, ∗) | isString(args[0]) call(builtin, setInterval, args, ∗, ∗) | isString(args[0]) call(document, write, ∗, ∗, ∗)

49