Confusing behavior when allow_fetching is True

[atmenta]

The docstring of context.ValidationContext states that if the value of the allow_fetching parameter is True "and certificates contain the location of a CRL or OCSP responder, an HTTP request will be made to obtain information for revocation checking". The word "allow" seems to indicate that this HTTP request is merely an additional way of obtaining information about revocation status. Moreover, also the following comment (especially the word "also") suggests that CRL or OCSP data requested via HTTP are not the only source of revocation status information if allow_fetching is True:

certvalidator/certvalidator/context.py

Lines 87 to 90 in 5bc5c39

	# By default, any CRLs or OCSP responses that are passed to the constructor
	# are chccked. If _allow_fetching is True, any CRLs or OCSP responses that
	# can be downloaded will also be checked. The next two attributes change
	# that behavior.

The docstring of the crls and ocsps parameters explain that pre-fetched/cached CRL and/or OCSP data can be passed to ValidationContext.

Putting all together, it seems that setting the allow_fetching parameter to True may result in fetching CRL or OCSP data in addition to checking pre-fetched/cached data.

Contrary to this expectation, if ValidationContext._allow_fetching is True, pre-fetched/cached CRL and OCSP data (passed by the crls and ocsps parameters) are completely ignored. (See the crls and ocsps properties and the retrieve_crls and retrieve_ocsps methods of ValidationContext.) If that is the intended behavior, it should be clarified in the documentation. Otherwise - and I would except this more intuitive - the two sources of revocation information should be merged when this is appropriate.

[wbond]

I do believe the wording in the comment is incorrect - there should be no "also".

It's been a while since I've worked on this project, so I don't have everything off of the top of my head. That said, if allow fetching was True, and the fetch failed, but there was a cached CRL, I would probably expect it to use the cached CRL. Which I don't believe it currently does. It appears to only used the cached CRL/OCSP info when fetching is disabled.

[atmenta]

I do believe the wording in the comment is incorrect - there should be no "also".

It's been a while since I've worked on this project, so I don't have everything off of the top of my head. That said, if allow fetching was True, and the fetch failed, but there was a cached CRL, I would probably expect it to use the cached CRL. Which I don't believe it currently does. It appears to only used the cached CRL/OCSP info when fetching is disabled.

Then we seem to agree on the expected behavior. Should I try to fix it and provide a pull request?