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

Blank CORS_ORIGIN in .env does not default to false - crashes app instead. #1384

Closed
3 tasks done
NHS-juju opened this issue Mar 31, 2023 · 2 comments · Fixed by Somerset-NHS-Solutions-Development/docsmith#1 or #1385
Labels
bug Something isn't working

Comments

@NHS-juju
Copy link
Contributor

Prerequisites

  • I have written a descriptive issue title

  • I have searched existing issues to ensure it has not already been reported

  • I agree to follow the Code of Conduct that this project adheres to

API/app/plugin version

10.0.12

Node.js version

v18.15.0

Operating system

Windows

Operating system version (i.e. 20.04, 11.3, 10)

Windows 10

Description

Attempting to run the app while leaving the "CORS_ORIGN" parameter blank in .env will result in a crash in src/config/index.js

Unlike Line 186 which grabs the parameter direct from .env, Line 187 makes use of the parseCorsParameter function to parse string booleans into actual booleans.

If the value "CORS_MAX_AGE" from .env is undefined, the short circuit in Line 186 can take effect. However, a null parameter passed into the function on Line 187 will fail when attempting to evaluate the undefined, so will never make use of the subsequent short circuit

Steps to Reproduce

Run the app with "CORS_ORIGIN" left blank in .env

Expected Behaviour

No response

@NHS-juju
Copy link
Contributor Author

Accidentally marked as closed, but is only resolved in local fork.

@Fdawgs Fdawgs reopened this Mar 31, 2023
@Fdawgs
Copy link
Owner

Fdawgs commented Mar 31, 2023

Cheers @NHS-juju, this week I've been revisiting the plugins and routes for all my APIs, and their unit tests, and have flushed out a few bugs.

Will take a look at the config and main server pieces of them all next week!

@Fdawgs Fdawgs linked a pull request Apr 3, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants