add certs support

[subversivo58]

Based on this: issue 196 comment 315537598

[subversivo58]

Travis seems to have failed to identify (not camel-case) and indentation...

[coveralls]

Coverage decreased (-14.3%) to 85.714% when pulling 304887e on subversivo58:cert-support into bf06a9c on motdotla:master.

[maxbeatty]

have you considered keeping certs and other multiline variables in dedicated files and using their paths as the environment variables?

CERT_PATH=/path/to/private.cert

require('dotenv').config()
const path = require('path')
const fs = require('fs')

const certPath = path.resolve(process.env.CERT_PATH)
const cert = fs.readFileSync(certPath)

If you're loading lots of multiline variables, you could come up with your own namespace to identify external content.

require('dotenv').config()
const path = require('path')
const fs = require('fs')

Object.keys(process.env)
  .filter(e => e.indexOf("MY_CERT_PREFIX_") === 0)
  .forEach(e => {
    const certPath = path.resolve(process.env[e])
    process.env[e] = fs.readFileSync(certPath)
})

That approach feels simpler to me than the additional proposed parsing logic.

[subversivo58]

I understand the succinct and straightforward aspect of a clean and minimalist code.

Setting the path to the certificate was my first approach when using the module, but if the module itself does read to disk and a certificate is nothing more than a string-based structure I believe that reading (loading) the certificate from the file .env is the most natural and least redundant.

Apart from the design aspect of the code, is there any counterpoint that prevents reading a certificate from the .env file being a feature?

[maxbeatty]

dotenv maintains a core set of features and encourages the community to extend it (e.g. validation and type casting). Supporting specific multiline variables such as certificates is another good extension point. I'm hesitant to add this because of the limited demand in #196, the complexity is adds to parsing, and additional test cases that'll need to be maintained. I'm very open to adding more documentation to deal with this particular use case.

[subversivo58]

All right, I get the point.

I also believe that the community should express interest after all may arise the need to test multiple certificates.

Could you keep it open until you define this interest? Is it possible to open this discussion to the community (a discussion not attached to the scope of #196)?

[rolodato]

This seems like an oddly specific feature to have in a very generic library such as dotenv. I agree with @maxbeatty here that this adds unnecessary complexity which can be easily avoided by pointing to a filename instead of including an entire file. That being said, I've never ran into this need so maybe there's something about your use case that I'm missing.

I don't develop Node on Windows but there could be an issue with very long environment variables on it, something else to keep in mind: https://superuser.com/questions/1070272/why-does-windows-have-a-limit-on-environment-variables-at-all

[subversivo58]

@rolodato the answer on supersuser.com is very qualified thank you but, all platforms have inherent limitation on the length of Environment Variables.

I can not imagine how a public or private key (certificate) can reach a maximum length within the usual platforms, however, the module itself encounters this limitation even if it is not documented.

I am writing an application that uses node in windows64 using NW.js and I was able to test the limitation of 32766 characters per key using process.env a larger number would return undefined.

I believe that reading (loading) the certificate from the .env file is the most natural and least redundant than pointing paths to read back from the disk once the module already does a read on disk.

The same application may need to store keys for external apis as well as their own authentication certificates this is a common use case and reading from process.env seems more intuitive although it is not a rule ... I saw some issues in this repository of users who are having difficulty getting this result using newline support.