Hello
> The way we typically ship extensions in contrib/ is to not create a new base
> version .sql file for smaller changes like adding a few functions. For this
> patch we should keep --1.2.sql and instead supply a 1.2--1.3.sql with the new
> functions.
Thank you for pointing this out. It makes sense to me.
> + errmsg("failed to convert tm to timestamp")));
>
> I think this error is too obscure for the user to act on, what we use
> elsewhere
> is "timestamp out of range" and I think thats more helpful. I do wonder if
> there is ever a legitimate case when this can fail while still having a
> authenticated client connection?
My bad here, you are right. "timestamp out of range" is a much better error
message. However, in an authenticated client connection, there should not be a
legitimate case where the not before and not after can fall out of range. The
"not before" and "not after" timestamps in a X509 certificate are normally
represented by ASN1, which has maximum timestamp of December 31, 9999. The
timestamp data structure in PostgreSQL on the other hand can support year up to
June 3, 5874898. Assuming the X509 certificate is generated correctly and no
data corruptions happening (which is unlikely), the conversion from ASN1 to
timestamp shall not result in out of range error.
Perhaps calling "tm2timestamp(&pgtm_time, 0, NULL, &ts)" without checking the
return code would be just fine. I see some other usages of tm2timstamp() in
other code areas also skip checking the return code.
> I have addressed the issues above in a new v5 patchset which includes a new
> patch for setting stable validity on the test certificates (the notBefore
> time
> was arbitrarily chosen to match the date of opening up the tree for v17 - we
> just need a date in the past). Your two patches are rolled into a single one
> with a commit message added to get started on that part as well.
thank you so much for addressing the ssl tests to make "not before" and "not
after" timestamps static in the test certificate and also adjusting
003_sslinfo.pl to expect the new static timestamps in the v5 patches. I am able
to apply both and all tests are passing. I did not know this test certificate
could be changed by `cd src/test/ssl && make -f sslfiles.mk`, but now I know,
thanks to you :p.
Best regards
Cary Huang
-------------
HighGo Software Inc. (Canada)
cary.hu...@highgo.ca
www.highgo.ca
