From ff76ef78c6906f5c1ae4241c7d4430cefbe799db Mon Sep 17 00:00:00 2001
From: Evan Wallace <evan.exe@gmail.com>
Date: Fri, 20 Aug 2021 15:33:39 -0400
Subject: [PATCH] fix #1536: http range requests now use less memory

---
 CHANGELOG.md            |  8 ++++++
 internal/fs/fs.go       | 23 +++++++++++++++++
 internal/fs/fs_mock.go  |  7 ++++++
 internal/fs/fs_real.go  | 51 +++++++++++++++++++++++++++++++++++++
 pkg/api/serve_other.go  | 56 +++++++++++++++++++++++++++++------------
 scripts/js-api-tests.js | 46 ++++++++++++++++++++++++++++++---
 6 files changed, 172 insertions(+), 19 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 3410aebe3b6..b8ba6302c81 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,13 @@
 # Changelog
 
+## Unreleased
+
+* Make HTTP range requests more efficient ([#1536](https://github.com/evanw/esbuild/issues/1536))
+
+    The local HTTP server built in to esbuild supports [range requests](https://developer.mozilla.org/en-US/docs/Web/HTTP/Range_requests), which are necessary for video playback in Safari. This means you can now use `<video>` tags in your HTML pages with esbuild's local HTTP server.
+
+    Previously this was implemented inefficiently for files that aren't part of the build, but that are read from the underlying fallback directory. In that case the entire file was being read even though only part of the file was needed. In this release, only the part of the file that is needed is read so using HTTP range requests with esbuild in this case will now use less memory.
+
 ## 0.12.21
 
 * Add support for native esbuild on Windows 64-bit ARM ([#995](https://github.com/evanw/esbuild/issues/995))
diff --git a/internal/fs/fs.go b/internal/fs/fs.go
index cf5240cc116..e85f75614bf 100644
--- a/internal/fs/fs.go
+++ b/internal/fs/fs.go
@@ -89,11 +89,34 @@ func (entries DirEntries) UnorderedKeys() (keys []string) {
 	return
 }
 
+type OpenedFile interface {
+	Len() int
+	Read(start int, end int) ([]byte, error)
+	Close() error
+}
+
+type InMemoryOpenedFile struct {
+	Contents []byte
+}
+
+func (f *InMemoryOpenedFile) Len() int {
+	return len(f.Contents)
+}
+
+func (f *InMemoryOpenedFile) Read(start int, end int) ([]byte, error) {
+	return []byte(f.Contents[start:end]), nil
+}
+
+func (f *InMemoryOpenedFile) Close() error {
+	return nil
+}
+
 type FS interface {
 	// The returned map is immutable and is cached across invocations. Do not
 	// mutate it.
 	ReadDirectory(path string) (entries DirEntries, canonicalError error, originalError error)
 	ReadFile(path string) (contents string, canonicalError error, originalError error)
+	OpenFile(path string) (result OpenedFile, canonicalError error, originalError error)
 
 	// This is a key made from the information returned by "stat". It is intended
 	// to be different if the file has been edited, and to otherwise be equal if
diff --git a/internal/fs/fs_mock.go b/internal/fs/fs_mock.go
index e0969ed3e03..1bfffd8fc18 100644
--- a/internal/fs/fs_mock.go
+++ b/internal/fs/fs_mock.go
@@ -62,6 +62,13 @@ func (fs *mockFS) ReadFile(path string) (string, error, error) {
 	return "", syscall.ENOENT, syscall.ENOENT
 }
 
+func (fs *mockFS) OpenFile(path string) (OpenedFile, error, error) {
+	if contents, ok := fs.files[path]; ok {
+		return &InMemoryOpenedFile{Contents: []byte(contents)}, nil, nil
+	}
+	return nil, syscall.ENOENT, syscall.ENOENT
+}
+
 func (fs *mockFS) ModKey(path string) (ModKey, error) {
 	return ModKey{}, errors.New("This is not available during tests")
 }
diff --git a/internal/fs/fs_real.go b/internal/fs/fs_real.go
index 7ff340c2c99..d4831dfe30b 100644
--- a/internal/fs/fs_real.go
+++ b/internal/fs/fs_real.go
@@ -211,6 +211,57 @@ func (fs *realFS) ReadFile(path string) (contents string, canonicalError error,
 	return fileContents, canonicalError, originalError
 }
 
+type realOpenedFile struct {
+	handle *os.File
+	len    int
+}
+
+func (f *realOpenedFile) Len() int {
+	return f.len
+}
+
+func (f *realOpenedFile) Read(start int, end int) ([]byte, error) {
+	bytes := make([]byte, end-start)
+	remaining := bytes
+
+	_, err := f.handle.Seek(int64(start), os.SEEK_SET)
+	if err != nil {
+		return nil, err
+	}
+
+	for len(remaining) > 0 {
+		n, err := f.handle.Read(remaining)
+		if err != nil && n <= 0 {
+			return nil, err
+		}
+		remaining = remaining[n:]
+	}
+
+	return bytes, nil
+}
+
+func (f *realOpenedFile) Close() error {
+	return f.handle.Close()
+}
+
+func (fs *realFS) OpenFile(path string) (OpenedFile, error, error) {
+	BeforeFileOpen()
+	defer AfterFileClose()
+
+	f, err := os.Open(path)
+	if err != nil {
+		return nil, fs.canonicalizeError(err), err
+	}
+
+	info, err := f.Stat()
+	if err != nil {
+		f.Close()
+		return nil, fs.canonicalizeError(err), err
+	}
+
+	return &realOpenedFile{f, int(info.Size())}, nil, nil
+}
+
 func (fs *realFS) ModKey(path string) (ModKey, error) {
 	BeforeFileOpen()
 	defer AfterFileClose()
diff --git a/pkg/api/serve_other.go b/pkg/api/serve_other.go
index f11da582663..4abdf5d2c8a 100644
--- a/pkg/api/serve_other.go
+++ b/pkg/api/serve_other.go
@@ -131,7 +131,7 @@ func (h *apiHandler) ServeHTTP(res http.ResponseWriter, req *http.Request) {
 		}
 
 		var kind fs.EntryKind
-		var fileContents []byte
+		var fileContents fs.OpenedFile
 		dirEntries := make(map[string]bool)
 		fileEntries := make(map[string]bool)
 
@@ -141,7 +141,9 @@ func (h *apiHandler) ServeHTTP(res http.ResponseWriter, req *http.Request) {
 			if strings.HasPrefix(outdirQueryPath, "/") {
 				outdirQueryPath = outdirQueryPath[1:]
 			}
-			kind, fileContents = h.matchQueryPathToResult(outdirQueryPath, &result, dirEntries, fileEntries)
+			resultKind, inMemoryBytes := h.matchQueryPathToResult(outdirQueryPath, &result, dirEntries, fileEntries)
+			kind = resultKind
+			fileContents = &fs.InMemoryOpenedFile{Contents: inMemoryBytes}
 		} else {
 			// Create a fake directory entry for the output path so that it appears to be a real directory
 			p := h.outdirPathPrefix
@@ -169,8 +171,9 @@ func (h *apiHandler) ServeHTTP(res http.ResponseWriter, req *http.Request) {
 			if absDir := h.fs.Dir(absPath); absDir != absPath {
 				if entries, err, _ := h.fs.ReadDirectory(absDir); err == nil {
 					if entry, _ := entries.Get(h.fs.Base(absPath)); entry != nil && entry.Kind(h.fs) == fs.FileEntry {
-						if contents, err, _ := h.fs.ReadFile(absPath); err == nil {
-							fileContents = []byte(contents)
+						if contents, err, _ := h.fs.OpenFile(absPath); err == nil {
+							defer contents.Close()
+							fileContents = contents
 							kind = fs.FileEntry
 						} else if err != syscall.ENOENT {
 							go h.notifyRequest(time.Since(start), req, http.StatusInternalServerError)
@@ -220,9 +223,9 @@ func (h *apiHandler) ServeHTTP(res http.ResponseWriter, req *http.Request) {
 		// Serve a "index.html" file if present
 		if kind == fs.DirEntry && fallbackIndexName != "" {
 			queryPath += "/" + fallbackIndexName
-			contents, err, _ := h.fs.ReadFile(h.fs.Join(h.servedir, queryPath))
-			if err == nil {
-				fileContents = []byte(contents)
+			if contents, err, _ := h.fs.OpenFile(h.fs.Join(h.servedir, queryPath)); err == nil {
+				defer contents.Close()
+				fileContents = contents
 				kind = fs.FileEntry
 			} else if err != syscall.ENOENT {
 				go h.notifyRequest(time.Since(start), req, http.StatusInternalServerError)
@@ -234,23 +237,44 @@ func (h *apiHandler) ServeHTTP(res http.ResponseWriter, req *http.Request) {
 
 		// Serve a file
 		if kind == fs.FileEntry {
-			if contentType := helpers.MimeTypeByExtension(path.Ext(queryPath)); contentType != "" {
-				res.Header().Set("Content-Type", contentType)
-			}
+			// Default to serving the whole file
+			status := http.StatusOK
+			fileContentsLen := fileContents.Len()
+			begin := 0
+			end := fileContentsLen
+			isRange := false
 
 			// Handle range requests so that video playback works in Safari
-			status := http.StatusOK
-			if begin, end, ok := parseRangeHeader(req.Header.Get("Range"), len(fileContents)); ok && begin < end {
+			if rangeBegin, rangeEnd, ok := parseRangeHeader(req.Header.Get("Range"), fileContentsLen); ok && rangeBegin < rangeEnd {
 				// Note: The content range is inclusive so subtract 1 from the end
-				res.Header().Set("Content-Range", fmt.Sprintf("bytes %d-%d/%d", begin, end-1, len(fileContents)))
-				fileContents = fileContents[begin:end]
+				isRange = true
+				begin = rangeBegin
+				end = rangeEnd
 				status = http.StatusPartialContent
 			}
 
-			res.Header().Set("Content-Length", fmt.Sprintf("%d", len(fileContents)))
+			// Try to read the range from the file, which may fail
+			fileBytes, err := fileContents.Read(begin, end)
+			if err != nil {
+				go h.notifyRequest(time.Since(start), req, http.StatusInternalServerError)
+				res.WriteHeader(http.StatusInternalServerError)
+				res.Write([]byte(fmt.Sprintf("500 - Internal server error: %s", err.Error())))
+				return
+			}
+
+			// If we get here, the request was successful
+			if contentType := helpers.MimeTypeByExtension(path.Ext(queryPath)); contentType != "" {
+				res.Header().Set("Content-Type", contentType)
+			} else {
+				res.Header().Set("Content-Type", "application/octet-stream")
+			}
+			if isRange {
+				res.Header().Set("Content-Range", fmt.Sprintf("bytes %d-%d/%d", begin, end-1, fileContentsLen))
+			}
+			res.Header().Set("Content-Length", fmt.Sprintf("%d", len(fileBytes)))
 			go h.notifyRequest(time.Since(start), req, status)
 			res.WriteHeader(status)
-			res.Write(fileContents)
+			res.Write(fileBytes)
 			return
 		}
 
diff --git a/scripts/js-api-tests.js b/scripts/js-api-tests.js
index 625c0cc2067..07e6a853220 100644
--- a/scripts/js-api-tests.js
+++ b/scripts/js-api-tests.js
@@ -2412,17 +2412,19 @@ require("/assets/file.png");
   },
 }
 
-function fetch(host, port, path) {
+function fetch(host, port, path, headers) {
   return new Promise((resolve, reject) => {
-    http.get({ host, port, path }, res => {
+    http.get({ host, port, path, headers }, res => {
       const chunks = []
       res.on('data', chunk => chunks.push(chunk))
       res.on('end', () => {
         const content = Buffer.concat(chunks)
         if (res.statusCode < 200 || res.statusCode > 299)
           reject(new Error(`${res.statusCode} when fetching ${path}: ${content}`))
-        else
+        else {
+          content.headers = res.headers
           resolve(content)
+        }
       })
     }).on('error', reject)
   })
@@ -2879,6 +2881,44 @@ let serveTests = {
     result.stop();
     await result.wait;
   },
+
+  async serveRange({ esbuild, testDir }) {
+    const big = path.join(testDir, 'big.txt')
+    const byteCount = 16 * 1024 * 1024
+    const buffer = require('crypto').randomBytes(byteCount)
+    await writeFileAsync(big, buffer)
+
+    const result = await esbuild.serve({
+      host: '127.0.0.1',
+      servedir: testDir,
+    }, {})
+
+    // Test small to big ranges
+    const minLength = 1
+    const maxLength = buffer.length
+
+    for (let i = 0, n = 16; i < n; i++) {
+      const length = Math.round(minLength + (maxLength - minLength) * i / (n - 1))
+      const start = Math.floor(Math.random() * (buffer.length - length))
+      const fetched = await fetch(result.host, result.port, '/big.txt', {
+        // Subtract 1 because range headers are inclusive on both ends
+        Range: `bytes=${start}-${start + length - 1}`,
+      })
+      delete fetched.headers.date
+      const expected = buffer.slice(start, start + length)
+      expected.headers = {
+        'access-control-allow-origin': '*',
+        'content-length': `${length}`,
+        'content-range': `bytes ${start}-${start + length - 1}/${byteCount}`,
+        'content-type': 'application/octet-stream',
+        'connection': 'close',
+      }
+      assert.deepStrictEqual(fetched, expected)
+    }
+
+    result.stop();
+    await result.wait;
+  },
 }
 
 async function futureSyntax(esbuild, js, targetBelow, targetAbove) {