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
Cloud Functions: Incorrect/incomplete transformation of escape sequences in .env files #4270
Comments
yellcorp
changed the title
Incorrect/incomplete transformation of escape sequences in .env files
Cloud Functions: Incorrect/incomplete transformation of escape sequences in .env files
Mar 9, 2022
taeold
pushed a commit
that referenced
this issue
Mar 25, 2022
Fixes #4270 and a little bit more. The bug results from the use of `String.prototype.replace` with a first (search) argument of type `string`. This is an unintuitive corner of JavaScript that only replaces the *first* occurrence of the search string. [See MDN](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/replace). To replace all occurrences, either the search argument must be expressed as an equivalent regex with the `/g` flag, or [.replaceAll](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/replaceAll) must be used. .replaceAll isn't supported below Node 15, and given this package's dependency on Node >= 12, the regex approach has been used here. Quick demonstration: ``` Welcome to Node.js v16.13.2. Type ".help" for more information. > const aaaa = "aaaa"; undefined > aaaa.replace("a", "b") // string argument; replaces only the first occurrence 'baaa' > aaaa.replace(/a/, "b") // regex argument *without* /g flag; replaces only the first occurrence 'baaa' > aaaa.replace(/a/g, "b") // regex argument *with* /g flag; replaces all occurrences 'bbbb' > aaaa.replaceAll("a", "b") // replaceAll method in Node >= 15; replaces all occurrences 'bbbb' ``` There's another reason to use a regex approach - running the string through repeated invocations of .replace(All) processes the escapes in an implementation-defined order, whereas the string really should be scanned from start to end in order to correctly convert (for example) `\\n` to `\` `n`. The popular npm package [dotenv](https://www.npmjs.com/package/dotenv) gets tripped up by this as well. The changes outside `.env` files result from manually searching the repo for uses of `String.prototype.replace` having a non-regex first argument. There are more occurrences than the ones fixed here that probably should be audited, but 1. Not all instances are immediately obvious to me whether they intend to replace *all* occurrences of the search argument 2. I wanted to keep this PR focused.
@yellcorp Thank you again for your wonderful contribution. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
[REQUIRED] Environment info
firebase-tools: 10.2.2
Platform: macOS (11.6.2 Big Sur)
[REQUIRED] Test case
Create a new Node project
Add this package as a devDependency
Init a new Firebase project
Edit
functions/index.js
to have the following content:Create
functions/.env
with the following content:ENVTEST_SAMPLE="foo\nbar\nbaz"
At this point it's sufficient to run the Cloud Functions emulator to reproduce the issue.
[REQUIRED] Steps to reproduce
Open the indicated URL for the
envParseTest
function in a browser, or some terminal-based HTTP client.curl 'http://localhost:5001/$GCLOUD_PROJECT/us-central1/envParseTest'
[REQUIRED] Expected behavior
A response with the content
[REQUIRED] Actual behavior
A response with the content
Note in the
"text"
property - the first\n
from the.env
has been correctly interpreted as a newline; the second remains as a literal \ followed by a literal n. This test also outputs abytes
property to show the parse result unambiguously. A newline appears in the array as decimal10
, a literal \ followed by literal n appears as decimal92,110
.Debug log from
firebase emulators:start
is attached.firebase-debug.log
The text was updated successfully, but these errors were encountered: