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

Trim the environment variables keys and values #10024

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

darinpope
Copy link
Contributor

Add in trimming of the node environment variables key and value values. If you enter values in either fields with leading spaces, trailing spaces or leading and trailing spaces, those spaces would stay and the environment variables would not load. This becomes a real problem when adding BASE+EXTRA values, such as PATH+LOCAL_BIN.

This change trims the key and value values.

Testing done

  • mvn -am -pl war,bom -Pquick-build clean install
  • mvn -Dtest=EnvironmentVariableNodePropertyTest verify -Dskip.yarn -Dskip.corepack

Proposed changelog entries

  • Trim the key and value values for Node environment variables.

Proposed upgrade guidelines

N/A

Submitter checklist

Preview Give feedback

Desired reviewers

@mention

Before the changes are marked as ready-for-merge:

Maintainer checklist

Preview Give feedback

Add in trimming of the node environment variables key and value values. If you enter values in either fields with leading spaces, trailing spaces or leading and trailing spaces, those spaces would stay and the environment variables would not load. This becomes a real problem when adding `BASE+EXTRA` values, such as `PATH+LOCAL_BIN`.

This change trims the `key` and `value` values.
@darinpope darinpope marked this pull request as ready for review December 4, 2024 02:16
After reviewing the help, the last sentence states:

```
If the Value is empty or whitespace-only, it will not be added to the environment, nor will it override or unset any environment variable with the same name that may already exist (e.g. a variable defined by the system).
```

This wasn't true prior to my changes nor any of the commits up to this point. I think this covers all the cases.
@darinpope darinpope marked this pull request as draft December 5, 2024 17:53
Copy link

@A1exKH A1exKH left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't notice the label WIP.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants