[SERVER-47798] Audit isMaster response validation for mongod and mongos Created: 27/Apr/20  Updated: 29/Oct/23  Resolved: 06/May/20

Status: Closed
Project: Core Server
Component/s: None
Affects Version/s: None
Fix Version/s: 4.0.19, 3.6.19, 4.2.8, 4.4.0-rc6, 4.7.0

Type: Improvement Priority: Major - P3
Reporter: Pavithra Vetriselvan Assignee: Pavithra Vetriselvan
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Backports
Backwards Compatibility: Fully Compatible
Backport Requested:
v4.4, v4.2, v4.0, v3.6
Sprint: Repl 2020-05-18
Participants:

 Description   

This came out of the conversation in SERVER-47750. We should have tests that validate the fields of isMaster responses to ensure that the server is returning the correct types.

For mongod, this may already be covered in our unittests and the nature of using setter functions (which would throw compiler errors if we passed an incorrect type as a parameter).

Mongos' isMaster responses fields are set directly on the response object, and we don't seem to have any test coverage for validating the types returned.

Ideally, we would use the IDL for isMaster responses, but until then, it might make sense to add an integration test verifying that isMaster responses are returning the types that the drivers expect.



 Comments   
Comment by Githook User [ 26/May/20 ]

Author:

{'name': 'Pavi Vetriselvan', 'email': 'pvselvan@umich.edu', 'username': 'pvselvan'}

Message: SERVER-47798 ismaster.js tests should ensure ismaster field is a boolean

(cherry picked from commit d5d0401e518beab5b659c1a01a092286f8a9c792)
(cherry picked from commit 9c1ce38d7c87f625be1c2d5d7f5a6b709ebfb72e)
Branch: v4.2
https://github.com/mongodb/mongo/commit/545ea895bfd90ea274a2b8da34c4534d6dc9c0af

Comment by Githook User [ 26/May/20 ]

Author:

{'name': 'Pavi Vetriselvan', 'email': 'pvselvan@umich.edu', 'username': 'pvselvan'}

Message: SERVER-47798 ismaster.js tests should ensure ismaster field is a boolean

(cherry picked from commit d5d0401e518beab5b659c1a01a092286f8a9c792)
(cherry picked from commit 9c1ce38d7c87f625be1c2d5d7f5a6b709ebfb72e)
Branch: v4.0
https://github.com/mongodb/mongo/commit/28e42416f579122eca7309e547e33641093b7b32

Comment by Githook User [ 26/May/20 ]

Author:

{'name': 'Pavi Vetriselvan', 'email': 'pvselvan@umich.edu', 'username': 'pvselvan'}

Message: SERVER-47798 ismaster.js tests should ensure ismaster field is a boolean

(cherry picked from commit d5d0401e518beab5b659c1a01a092286f8a9c792)
(cherry picked from commit 9c1ce38d7c87f625be1c2d5d7f5a6b709ebfb72e)
Branch: v3.6
https://github.com/mongodb/mongo/commit/928320fcd24806a8292216fb17642cd69410459c

Comment by Githook User [ 12/May/20 ]

Author:

{'name': 'Pavi Vetriselvan', 'email': 'pvselvan@umich.edu', 'username': 'pvselvan'}

Message: SERVER-47798 ismaster.js tests should ensure ismaster field is a boolean

(cherry picked from commit d5d0401e518beab5b659c1a01a092286f8a9c792)
Branch: v4.4
https://github.com/mongodb/mongo/commit/9c1ce38d7c87f625be1c2d5d7f5a6b709ebfb72e

Comment by Pavithra Vetriselvan [ 06/May/20 ]

Sounds good!

Comment by Tess Avitabile (Inactive) [ 06/May/20 ]

Thanks for investigating this! We no longer push code to versions prior to 3.6, so we should backport as far back as 3.6.

Comment by Pavithra Vetriselvan [ 06/May/20 ]

tess.avitabile I think this change is very simple to backport and will give us the correct coverage. I've tracked it down to 3.2, but wasn't sure what our policy was for backporting before 3.6. For now, I've requested backports to 4.4, 4.2, 4.0, and 3.6.

Comment by Githook User [ 06/May/20 ]

Author:

{'name': 'Pavi Vetriselvan', 'email': 'pvselvan@umich.edu', 'username': 'pvselvan'}

Message: SERVER-47798 ismaster.js tests should ensure ismaster field is a boolean
Branch: master
https://github.com/mongodb/mongo/commit/d5d0401e518beab5b659c1a01a092286f8a9c792

Comment by Pavithra Vetriselvan [ 06/May/20 ]

Code review url: https://mongodbcr.appspot.com/599970002/

Comment by Pavithra Vetriselvan [ 04/May/20 ]

As it turns out, we do have this type of testing for mongod and mongos.

However, in both tests, we don't explicitly assert the type of the ismaster field:

assert(res.ismaster, "ismaster missing or false:" + tojson(res));

This is a flaw in javascript comparison, since this will return true as long as ismaster is a truthy value (non-empty string, True, etc). I think all this ticket needs to do is change that line to be something like:

assert(res.ismaster === true, ...)

or add:

assert.eq("boolean", typeof res.ismaster)

Generated at Thu Feb 08 05:15:16 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.