[GODRIVER-1524] Unreachable code and confusing error message in loadCert Created: 07/Mar/20  Updated: 28/Oct/23  Resolved: 12/Mar/20

Status: Closed
Project: Go Driver
Component/s: Options & Configuration
Affects Version/s: None
Fix Version/s: 1.4.0

Type: Improvement Priority: Major - P3
Reporter: David Bartley Assignee: Divjot Arora (Inactive)
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Backwards Compatibility: Fully Compatible

 Description   

func loadCert(data []byte) ([]byte, error) {
    var certBlock *pem.Block
 
    for certBlock == nil {
        if data == nil || len(data) == 0 {
            return nil, errors.New(".pem file must have both a CERTIFICATE and an RSA PRIVATE KEY section")
        }
 
        block, rest := pem.Decode(data)
        if block == nil {
            return nil, errors.New("invalid .pem file")
        }
 
        switch block.Type {
        case "CERTIFICATE":
            if certBlock != nil {
                return nil, errors.New("multiple CERTIFICATE sections in .pem file")
            }
 
            certBlock = block
        }
 
        data = rest
    }
 
    return certBlock.Bytes, nil
}

The very first error is confusing; it actually indicates that no CERTIFICATE block was found (possibly because the file is empty).

The "if certBlock != nil" block is actually unreachable; the for loop ensures that certBlock is always nil. That case could actually be simplified to just "return certBlock.Bytes", and the for loop could just be "for {".

Finally, "loadCert" is actually implemented in two files, along with "addCACertFromFile"; these should be consilidated.



 Comments   
Comment by Githook User [ 12/Mar/20 ]

Author:

{'name': 'Divjot Arora', 'username': 'divjotarora', 'email': 'divjot.arora@10gen.com'}

Message: GODRIVER-1524 Improve error checking for loading CA certificate (#329)
Branch: master
https://github.com/mongodb/mongo-go-driver/commit/2c54694807d7ce33cbf35e1f10c78b5f6c4ffe6f

Comment by Divjot Arora (Inactive) [ 10/Mar/20 ]

bartle Thanks for the usability suggestions. I addressed the confusing error and the unreachable code in the PR I linked. The loadCert function is duplicated in topology_options.go because it's used by topology.WithConnString option, which we're hoping to remove in the 1.4.0 cycle as it is dead code. I've filed GODRIVER-1530 for it's removal.

Comment by Divjot Arora (Inactive) [ 10/Mar/20 ]

https://github.com/mongodb/mongo-go-driver/pull/329#pullrequestreview-372350981

Generated at Thu Feb 08 08:36:34 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.