Skip to content

Commit e2e36cc

Browse files
W-A-Jamesdurran
andauthored
feat(NODE-3989)!: only accept true and false for boolean options (#3791)
Co-authored-by: Durran Jordan <[email protected]>
1 parent 9268b35 commit e2e36cc

File tree

3 files changed

+46
-33
lines changed

3 files changed

+46
-33
lines changed

Diff for: src/connection_string.ts

+7-21
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ import type { TagSet } from './sdam/server_description';
3333
import {
3434
DEFAULT_PK_FACTORY,
3535
emitWarning,
36-
emitWarningOnce,
3736
HostAddress,
3837
isRecord,
3938
matchesParentDomain,
@@ -162,29 +161,16 @@ function checkTLSOptions(allOptions: CaseInsensitiveMap): void {
162161
check('tlsAllowInvalidCertificates', 'tlsDisableOCSPEndpointCheck');
163162
check('tlsDisableCertificateRevocationCheck', 'tlsDisableOCSPEndpointCheck');
164163
}
165-
166-
const TRUTHS = new Set(['true', 't', '1', 'y', 'yes']);
167-
const FALSEHOODS = new Set(['false', 'f', '0', 'n', 'no', '-1']);
168164
function getBoolean(name: string, value: unknown): boolean {
169165
if (typeof value === 'boolean') return value;
170-
const valueString = String(value).toLowerCase();
171-
if (TRUTHS.has(valueString)) {
172-
if (valueString !== 'true') {
173-
emitWarningOnce(
174-
`deprecated value for ${name} : ${valueString} - please update to ${name} : true instead`
175-
);
176-
}
177-
return true;
178-
}
179-
if (FALSEHOODS.has(valueString)) {
180-
if (valueString !== 'false') {
181-
emitWarningOnce(
182-
`deprecated value for ${name} : ${valueString} - please update to ${name} : false instead`
183-
);
184-
}
185-
return false;
166+
switch (value) {
167+
case 'true':
168+
return true;
169+
case 'false':
170+
return false;
171+
default:
172+
throw new MongoParseError(`${name} must be either "true" or "false"`);
186173
}
187-
throw new MongoParseError(`Expected ${name} to be stringified boolean value, got: ${value}`);
188174
}
189175

190176
function getIntFromOptions(name: string, value: unknown): number {

Diff for: test/unit/assorted/uri_options.spec.test.ts

+7-5
Original file line numberDiff line numberDiff line change
@@ -14,16 +14,18 @@ describe('URI option spec tests', function () {
1414
'tlsDisableCertificateRevocationCheck can be set to true',
1515
'tlsDisableCertificateRevocationCheck can be set to false',
1616
'tlsDisableOCSPEndpointCheck can be set to true',
17-
'tlsDisableOCSPEndpointCheck can be set to false'
17+
'tlsDisableOCSPEndpointCheck can be set to false',
18+
19+
// NOTE(NODE-3989): Skipped because we now only accept true/false for boolean options. The spec expects us to
20+
// warn on other accepted but deprecated values, but as of NODE-3989, we now throw on these
21+
// values
22+
'Invalid loadBalanced value'
1823
];
1924

2025
const testsThatDoNotThrowOnWarn = [
2126
// TODO(NODE-3923): compression option validation
2227
'Too high zlibCompressionLevel causes a warning',
23-
'Too low zlibCompressionLevel causes a warning',
24-
25-
// TODO(NODE-3989): Fix legacy boolean parsing
26-
'Invalid loadBalanced value'
28+
'Too low zlibCompressionLevel causes a warning'
2729
];
2830

2931
for (const suite of suites) {

Diff for: test/unit/connection_string.test.ts

+32-7
Original file line numberDiff line numberDiff line change
@@ -211,13 +211,38 @@ describe('Connection String', function () {
211211
});
212212
});
213213

214-
it('should parse boolean values', function () {
215-
let options = parseOptions('mongodb://hostname?retryWrites=1');
216-
expect(options.retryWrites).to.equal(true);
217-
options = parseOptions('mongodb://hostname?retryWrites=false');
218-
expect(options.retryWrites).to.equal(false);
219-
options = parseOptions('mongodb://hostname?retryWrites=t');
220-
expect(options.retryWrites).to.equal(true);
214+
context('boolean options', function () {
215+
const valuesExpectations: { value: string; expectation: 'error' | boolean }[] = [
216+
{ value: 'true', expectation: true },
217+
{ value: 'false', expectation: false },
218+
{ value: '-1', expectation: 'error' },
219+
{ value: '1', expectation: 'error' },
220+
{ value: '0', expectation: 'error' },
221+
{ value: 't', expectation: 'error' },
222+
{ value: 'f', expectation: 'error' },
223+
{ value: 'n', expectation: 'error' },
224+
{ value: 'y', expectation: 'error' },
225+
{ value: 'yes', expectation: 'error' },
226+
{ value: 'no', expectation: 'error' },
227+
{ value: 'unknown', expectation: 'error' }
228+
];
229+
for (const { value, expectation } of valuesExpectations) {
230+
const connString = `mongodb://hostname?retryWrites=${value}`;
231+
context(`when provided '${value}'`, function () {
232+
if (expectation === 'error') {
233+
it('throws MongoParseError', function () {
234+
expect(() => {
235+
parseOptions(connString);
236+
}).to.throw(MongoParseError);
237+
});
238+
} else {
239+
it(`parses as ${expectation}`, function () {
240+
const options = parseOptions(connString);
241+
expect(options).to.have.property('retryWrites', expectation);
242+
});
243+
}
244+
});
245+
}
221246
});
222247

223248
it('should parse compression options', function () {

0 commit comments

Comments
 (0)