Skip to content
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

util.parseEnv creates keys from invalid, newline-separated lines #56775

Open
jonschlinkert opened this issue Jan 26, 2025 · 10 comments · May be fixed by #56778
Open

util.parseEnv creates keys from invalid, newline-separated lines #56775

jonschlinkert opened this issue Jan 26, 2025 · 10 comments · May be fixed by #56778
Labels
confirmed-bug Issues with confirmed bugs. dotenv Issues and PRs related to .env file parsing

Comments

@jonschlinkert
Copy link

jonschlinkert commented Jan 26, 2025

Version

v23.6.1

Platform

Darwin Mac.lan1 24.1.0 Darwin Kernel Version 24.1.0: Thu Oct 10 21:03:11 PDT 2024; root:xnu-11215.41.3~2/RELEASE_ARM64_T6020 arm64

Subsystem

No response

What steps will reproduce the bug?

Do:

import { parseEnv } from 'node:util';

Then create a string with some invalid lines:

foo


bar
baz=whatever

And parse it:

const input = `      foo


bar
baz=whatever
`;

const env = parseEnv(input);
console.log(env);
// Outputs => { 'foo\n\n\nbar\nbaz': 'whatever' }

How often does it reproduce? Is there a required condition?

Tested on node versions v21x-v23.6.1

What is the expected behavior? Why is that the expected behavior?

Expected behavior, IMHO, would be throw an invalid syntax error.

What do you see instead?

Invalid output.

Additional information

No response

@anonrig
Copy link
Member

anonrig commented Jan 26, 2025

How does dotenv handle this? I think this is expected behavior.

@anonrig anonrig added the dotenv Issues and PRs related to .env file parsing label Jan 26, 2025
@AugustinMauroy
Copy link
Member

AugustinMauroy commented Jan 26, 2025

IMO node should output:

{
  foo: true,
  bar: true,
  baz: 'whatever',
}

and dotenv do that

const dotenv = require('dotenv');

const input = `      foo


bar
baz=whatever
`;

const output = dotenv.parse(input);

console.log(output); // => { baz: 'whatever' }

Tested on v24.0.0-pre

@anonrig
Copy link
Member

anonrig commented Jan 26, 2025

I think dotenv's behavior is more correct. But dotenv also supports multiline arguments. I think we shouldn't accept newline characters when parsing key's

@AugustinMauroy
Copy link
Member

AugustinMauroy commented Jan 26, 2025

supports multiline arguments

I never heard about that. Could you give me an example.

you mean that ?

@anonrig
Copy link
Member

anonrig commented Jan 26, 2025

supports multiline arguments

I never heard about that. Could you give me an example.

you mean that ?

Line 72 of https://github.com/nodejs/node/blob/main/test/parallel/test-dotenv.js

@AugustinMauroy
Copy link
Member

AugustinMauroy commented Jan 26, 2025

+1 for Dotenv behavior. We should use same fixtures

Can I fix this issue ? i know the answers is yes But I mean is it an easy task.
And where is the affected code ?

@anonrig
Copy link
Member

anonrig commented Jan 26, 2025

+1 for Dotenv behavior. We should use same fixtures

Can I fix this issue ? i know the answers is yes But I mean is it an easy task.
And where is the affected code ?

Yes, node_dotenv.cc

@anonrig anonrig added the confirmed-bug Issues with confirmed bugs. label Jan 26, 2025
@AugustinMauroy AugustinMauroy linked a pull request Jan 26, 2025 that will close this issue
@jonschlinkert
Copy link
Author

But dotenv also supports multiline arguments

I think multiline arguments are more common than multiline keys

@jonschlinkert
Copy link
Author

IMO node should output:

{
foo: true,
bar: true,
baz: 'whatever',
}

I'll go with whatever the consensus is, but IMHO foo and bar in that example shouldn't set to true, since there is no =. sign. It seems like stricter behavior would be safer.

@jonschlinkert
Copy link
Author

Since one doesn't exist, I thought i would get a spec started for .env: https://github.com/env-lang/env/blob/main/env.md

I'm open to any level of contributions if anyone is interested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. dotenv Issues and PRs related to .env file parsing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants