Remove redundant chmod -R on /tmp + avoid body buffering for response size#854
Open
urbantech wants to merge 1 commit intogarrytan:mainfrom
Open
Remove redundant chmod -R on /tmp + avoid body buffering for response size#854urbantech wants to merge 1 commit intogarrytan:mainfrom
urbantech wants to merge 1 commit intogarrytan:mainfrom
Conversation
Dockerfile.ci (garrytan#709): Line 63 'chmod -R 1777 /tmp' redundantly applied sticky bit to all files inside /tmp (not just the directory). Line 61 already correctly sets 'chmod 1777 /tmp'. Remove the duplicate -R variant. browser-manager.ts (garrytan#711): requestfinished handler called res.body() to measure response size, buffering entire responses (videos, images, downloads) into memory. Replace with content-length header read — zero memory allocation, same metric quality (falls back to 0 for chunked/missing header). Fixes garrytan#709, fixes garrytan#711
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
#709 — Docker: chmod -R 1777 /tmp sets sticky bit on files
Line 61 of Dockerfile.ci correctly does
chmod 1777 /tmp. Line 63 redundantly doeschmod -R 1777 /tmp, recursively applying the sticky bit to all files inside /tmp (meaningless on Linux files, confusing).Fix: Remove the redundant line 63.
#711 — Performance: res.body() buffers entire response for size
The
requestfinishedhandler calledawait res.body()on every completed request to measurebody.length. For large assets (videos, images, downloads), this buffers the entire response into memory unnecessarily.Fix: Read
content-lengthheader instead — zero memory allocation, same metric quality. Falls back to 0 for chunked/missing headers (acceptable for a size metric).Test Plan
Fixes #709, fixes #711