Trim whitespace and quotes in variable substitution

christiaangoossens commented 3 months ago

Proposed change

The new code trims quotes ( ' and " ) and surrounding whitespace off any environment variables that are to be replaced.

This prevents a failure case of the JSON.parse() for Traefik substitution that would fail if you have accidentally added whitespace around your environment variable in Docker/Kubernetes (same for accidental quotes).

Closes discussion https://github.com/gethomepage/homepage/discussions/3535

Note that this might be a regression for those that rely on quotes or whitespace being the first or last character of an API key.

Type of change

[ ] New service widget
[X] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Documentation only
[ ] Other (please explain)

Checklist:

[ ] If applicable, I have added corresponding documentation changes.
[ ] If applicable, I have reviewed the feature and / or service widget guidelines.
[x] I have checked that all code style checks pass using pre-commit hooks and linting checks.
[x] If applicable, I have tested my code for new features & regressions on both mobile & desktop devices, using the latest version of major browsers.

shamoon commented 3 months ago

Thanks but as you mentioned this may be a regression for others so this PR is not really acceptable. You’re also the only person to comment on the discussion so hard to know if others are affected by / care about this.

If there is interest (ie a feature request) in implementing stripping of whitespace that would probably be ok

christiaangoossens commented 3 months ago

I am sad to hear your response as I feel this fixes a bug in the original implementation, but someone else is welcome to pitch it again. I have left instructions in the discussion on how to deal with it in the meantime.

github-actions[bot] commented 2 months ago

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new discussion for related concerns. See our contributing guidelines for more details.

gethomepage / homepage