Skip to content

feat(NODE-3989)!: only accept true and false for boolean options #3791

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

Merged
merged 9 commits into from
Aug 4, 2023
28 changes: 7 additions & 21 deletions src/connection_string.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ import type { TagSet } from './sdam/server_description';
import {
DEFAULT_PK_FACTORY,
emitWarning,
emitWarningOnce,
HostAddress,
isRecord,
matchesParentDomain,
Expand Down Expand Up @@ -162,29 +161,16 @@ function checkTLSOptions(allOptions: CaseInsensitiveMap): void {
check('tlsAllowInvalidCertificates', 'tlsDisableOCSPEndpointCheck');
check('tlsDisableCertificateRevocationCheck', 'tlsDisableOCSPEndpointCheck');
}

const TRUTHS = new Set(['true', 't', '1', 'y', 'yes']);
const FALSEHOODS = new Set(['false', 'f', '0', 'n', 'no', '-1']);
function getBoolean(name: string, value: unknown): boolean {
if (typeof value === 'boolean') return value;
const valueString = String(value).toLowerCase();
if (TRUTHS.has(valueString)) {
if (valueString !== 'true') {
emitWarningOnce(
`deprecated value for ${name} : ${valueString} - please update to ${name} : true instead`
);
}
return true;
}
if (FALSEHOODS.has(valueString)) {
if (valueString !== 'false') {
emitWarningOnce(
`deprecated value for ${name} : ${valueString} - please update to ${name} : false instead`
);
}
return false;
switch (value) {
case 'true':
return true;
case 'false':
return false;
default:
throw new MongoParseError(`${name} must be either "true" or "false"`);
}
throw new MongoParseError(`Expected ${name} to be stringified boolean value, got: ${value}`);
}

function getIntFromOptions(name: string, value: unknown): number {
Expand Down
5 changes: 4 additions & 1 deletion test/unit/assorted/uri_options.spec.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@ describe('URI option spec tests', function () {
'tlsDisableCertificateRevocationCheck can be set to true',
'tlsDisableCertificateRevocationCheck can be set to false',
'tlsDisableOCSPEndpointCheck can be set to true',
'tlsDisableOCSPEndpointCheck can be set to false'
'tlsDisableOCSPEndpointCheck can be set to false',

// Skipped because we now only accept true/false for boolean options
'Invalid loadBalanced value'
];

const testsThatDoNotThrowOnWarn = [
Expand Down
41 changes: 34 additions & 7 deletions test/unit/connection_string.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -208,13 +208,40 @@ describe('Connection String', function () {
});
});

it('should parse boolean values', function () {
let options = parseOptions('mongodb://hostname?retryWrites=1');
expect(options.retryWrites).to.equal(true);
options = parseOptions('mongodb://hostname?retryWrites=false');
expect(options.retryWrites).to.equal(false);
options = parseOptions('mongodb://hostname?retryWrites=t');
expect(options.retryWrites).to.equal(true);
context('boolean options', function () {
const valuesExpectations: { value: string; expectation: 'error' | boolean }[] = [
{ value: 'true', expectation: true },
{ value: 'false', expectation: false },
{ value: '-1', expectation: 'error' },
{ value: '1', expectation: 'error' },
{ value: '0', expectation: 'error' },
{ value: 't', expectation: 'error' },
{ value: 'f', expectation: 'error' },
{ value: 'n', expectation: 'error' },
{ value: 'y', expectation: 'error' },
{ value: 'yes', expectation: 'error' },
{ value: 'no', expectation: 'error' },
{ value: 'unknown', expectation: 'error' }
];
for (const { value, expectation } of valuesExpectations) {
const connString = `mongodb://hostname?retryWrites=${value}`;
context(`when provided '${value}'`, function () {
switch (expectation) {
case 'error':
it('throws MongoParseError', function () {
expect(() => {
parseOptions(connString);
}).to.throw(MongoParseError);
});
break;
default:
it(`parses as ${expectation}`, function () {
const options = parseOptions(connString);
expect(options).to.have.property('retryWrites', expectation);
});
}
});
}
});

it('should parse compression options', function () {
Expand Down