fix #311: source map crash with windows newlines

evanw · Aug 3, 2020 · 0805dd6 · 0805dd6
1 parent ddf78d0
commit 0805dd6
Show file tree

Hide file tree

Showing 3 changed files with 68 additions and 43 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,10 @@
 
     This makes it possible to use esbuild as a faster TypeScript-to-JavaScript frontend for Webpack, which has special [magic comments](https://webpack.js.org/api/module-methods/#magic-comments) inside `import()` expressions that affect Webpack's behavior.
 
+* Fix crash for source files beginning with `\r\n` when using source maps ([#311](https://github.com/evanw/esbuild/issues/311))
+
+    The source map changes in version 0.6.13 introduced a regression that caused source files beginning with `\r\n` to crash esbuild when source map generation was enabled. This was not caught during testing both because not many source files begin with a newline and not many source files have Windows-style line endings in them. This regression has been fixed and Windows-style line endings now have test coverage.
+
 ## 0.6.14
 
 * Add support for parsing top-level await ([#253](https://github.com/evanw/esbuild/issues/253))

diff --git a/internal/printer/printer.go b/internal/printer/printer.go
@@ -471,6 +471,13 @@ func (p *printer) addSourceMapping(loc ast.Loc) {
 	}
 	originalLine--
 
+	if originalLine == -1 {
+		fmt.Printf("crash in file %s\n", p.options.SourceForSourceMap.PrettyPath)
+		fmt.Printf("lookup %d\n", loc.Start)
+		fmt.Printf("length %d\n", len(lineOffsetTables))
+		fmt.Printf("first %d\n", lineOffsetTables[0].byteOffsetToStartOfLine)
+	}
+
 	// Use the line to compute the column
 	line := &lineOffsetTables[originalLine]
 	originalColumn := int(loc.Start - line.byteOffsetToStartOfLine)
@@ -583,6 +590,7 @@ func generateLineOffsetTables(contents string) []lineOffsetTable {
 			// Handle Windows-specific "\r\n" newlines
 			if c == '\r' {
 				if i+1 < len(contents) && contents[i+1] == '\n' {
+					column++
 					continue
 				}
 			}

diff --git a/scripts/verify-source-map.js b/scripts/verify-source-map.js
@@ -194,7 +194,7 @@ const toSearchPartialMappings = [
   'entry',
 ]
 
-async function check(kind, testCase, toSearch, { flags, entryPoints }) {
+async function check(kind, testCase, toSearch, { flags, entryPoints, crlf }) {
   let failed = 0
 
   try {
@@ -211,8 +211,10 @@ async function check(kind, testCase, toSearch, { flags, entryPoints }) {
     for (const name in testCase) {
       if (name !== '<stdin>') {
         const tempPath = path.join(tempDir, name)
+        let code = testCase[name]
         mkdirp.sync(path.dirname(tempPath))
-        await writeFileAsync(tempPath, testCase[name])
+        if (crlf) code = code.replace(/\n/g, '\r\n')
+        await writeFileAsync(tempPath, code)
       }
     }
 
@@ -328,47 +330,58 @@ async function check(kind, testCase, toSearch, { flags, entryPoints }) {
 
 async function main() {
   const promises = []
-  for (const minify of [false, true]) {
-    const flags = minify ? ['--minify'] : []
-    const suffix = minify ? '-min' : ''
-    promises.push(
-      check('commonjs' + suffix, testCaseCommonJS, toSearchBundle, {
-        flags: flags.concat('--outfile=out.js', '--bundle'),
-        entryPoints: ['a.js'],
-      }),
-      check('es6' + suffix, testCaseES6, toSearchBundle, {
-        flags: flags.concat('--outfile=out.js', '--bundle'),
-        entryPoints: ['a.js'],
-      }),
-      check('ts' + suffix, testCaseTypeScriptRuntime, toSearchNoBundle, {
-        flags: flags.concat('--outfile=out.js'),
-        entryPoints: ['a.ts'],
-      }),
-      check('stdin-stdout' + suffix, testCaseStdin, toSearchNoBundle, {
-        flags: flags.concat('--sourcefile=<stdin>'),
-        entryPoints: [],
-      }),
-      check('empty' + suffix, testCaseEmptyFile, toSearchEmptyFile, {
-        flags: flags.concat('--outfile=out.js', '--bundle'),
-        entryPoints: ['entry.js'],
-      }),
-      check('non-js' + suffix, testCaseNonJavaScriptFile, toSearchNonJavaScriptFile, {
-        flags: flags.concat('--outfile=out.js', '--bundle'),
-        entryPoints: ['entry.js'],
-      }),
-      check('splitting' + suffix, testCaseCodeSplitting, toSearchCodeSplitting, {
-        flags: flags.concat('--outdir=.', '--bundle', '--splitting', '--format=esm'),
-        entryPoints: ['out.ts', 'other.ts'],
-      }),
-      check('unicode' + suffix, testCaseUnicode, toSearchUnicode, {
-        flags: flags.concat('--outfile=out.js', '--bundle'),
-        entryPoints: ['entry.js'],
-      }),
-      check('dummy' + suffix, testCasePartialMappings, toSearchPartialMappings, {
-        flags: flags.concat('--outfile=out.js', '--bundle'),
-        entryPoints: ['entry.js'],
-      }),
-    )
+  for (const crlf of [false, true]) {
+    for (const minify of [false, true]) {
+      const flags = minify ? ['--minify'] : []
+      const suffix = (crlf ? '-crlf' : '') + (minify ? '-min' : '')
+      promises.push(
+        check('commonjs' + suffix, testCaseCommonJS, toSearchBundle, {
+          flags: flags.concat('--outfile=out.js', '--bundle'),
+          entryPoints: ['a.js'],
+          crlf,
+        }),
+        check('es6' + suffix, testCaseES6, toSearchBundle, {
+          flags: flags.concat('--outfile=out.js', '--bundle'),
+          entryPoints: ['a.js'],
+          crlf,
+        }),
+        check('ts' + suffix, testCaseTypeScriptRuntime, toSearchNoBundle, {
+          flags: flags.concat('--outfile=out.js'),
+          entryPoints: ['a.ts'],
+          crlf,
+        }),
+        check('stdin-stdout' + suffix, testCaseStdin, toSearchNoBundle, {
+          flags: flags.concat('--sourcefile=<stdin>'),
+          entryPoints: [],
+          crlf,
+        }),
+        check('empty' + suffix, testCaseEmptyFile, toSearchEmptyFile, {
+          flags: flags.concat('--outfile=out.js', '--bundle'),
+          entryPoints: ['entry.js'],
+          crlf,
+        }),
+        check('non-js' + suffix, testCaseNonJavaScriptFile, toSearchNonJavaScriptFile, {
+          flags: flags.concat('--outfile=out.js', '--bundle'),
+          entryPoints: ['entry.js'],
+          crlf,
+        }),
+        check('splitting' + suffix, testCaseCodeSplitting, toSearchCodeSplitting, {
+          flags: flags.concat('--outdir=.', '--bundle', '--splitting', '--format=esm'),
+          entryPoints: ['out.ts', 'other.ts'],
+          crlf,
+        }),
+        check('unicode' + suffix, testCaseUnicode, toSearchUnicode, {
+          flags: flags.concat('--outfile=out.js', '--bundle'),
+          entryPoints: ['entry.js'],
+          crlf,
+        }),
+        check('dummy' + suffix, testCasePartialMappings, toSearchPartialMappings, {
+          flags: flags.concat('--outfile=out.js', '--bundle'),
+          entryPoints: ['entry.js'],
+          crlf,
+        }),
+      )
+    }
   }
 
   const failed = (await Promise.all(promises)).reduce((a, b) => a + b, 0)