[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: |
|
||||
| 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 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: (cherry picked from commit d5d0401e518beab5b659c1a01a092286f8a9c792) | |||
| Comment by Githook User [ 26/May/20 ] | |||
|
Author: {'name': 'Pavi Vetriselvan', 'email': 'pvselvan@umich.edu', 'username': 'pvselvan'}Message: (cherry picked from commit d5d0401e518beab5b659c1a01a092286f8a9c792) | |||
| Comment by Githook User [ 26/May/20 ] | |||
|
Author: {'name': 'Pavi Vetriselvan', 'email': 'pvselvan@umich.edu', 'username': 'pvselvan'}Message: (cherry picked from commit d5d0401e518beab5b659c1a01a092286f8a9c792) | |||
| Comment by Githook User [ 12/May/20 ] | |||
|
Author: {'name': 'Pavi Vetriselvan', 'email': 'pvselvan@umich.edu', 'username': 'pvselvan'}Message: (cherry picked from commit d5d0401e518beab5b659c1a01a092286f8a9c792) | |||
| 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: | |||
| 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:
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:
or add:
|