Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update autossl.lua #55

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Update autossl.lua #55

wants to merge 1 commit into from

Conversation

jackshang
Copy link

Did I understand it wrong with here?

Did I understand it wrong with here?
@@ -455,7 +454,9 @@ function AUTOSSL.ssl_certificate()
end
end

if domain_key_types_count ~= chains_set then
chains_set_count = #chains_set
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

chains_set could be in some case being { 2 = true }, so this won't work as chains_set is a sparse array, in lua using # to get length of a table
that has index doesn't start with 1 will be unspecified (in LuaJIT it returns 0)

Copy link
Author

@jackshang jackshang Nov 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code snippet "domain_key_types_count ~= chains_set" is always true, so every client https request will invoke code update_cert ->update_cert_handler->AUTOSSL.client:order_certificate(pkey, domain),it's not necessary.

Only "domain_key_types_count" was not equal to the length of "chains_set", must actively apply for a certificate.

By default, domain_key_types = { 'rsa'}, the "domain_key_types_count" is 1:

  1. If the length of "chains_set" is 0, need invoke update_cert() actively apply for a certificate.
  2. If the length of "chains_set"is 1, indicates that we has obtained the certificate, unnecessary and return.

When config domain_key_types = { 'rsa', 'ecc' }, the "domain_key_types_count" is 2:

  1. if the length of "chains_set" = 0 ,actively apply for rsa and ecc certificate;
  2. if the length of "chains_set" = 1 ,actively apply for ecc or rsa certificate;
  3. if the length of "chains_set" = 2, unnecessary and return.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I failed to send one of my comment 🤦 . The change you made on Line 459 is indeed a good
fix. Although we do have another check on line 462 so the original code only creates redundant timer but not duplicate certs.

But the above is not correct as how Lua due with tables. If rsa cert is not
created but ecc does, chains_set becomes { [2] = true} and #chains_set is 0 not 1.

Let's keep only the change of line 459.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmm, I ignored check on line 462, indeed only an empty timer will be generated. When I was studying your project, this snippet of code was very confusing for me, so I tried to modify it. Maybe create a new method for "timer" snippet of code, make it easier to understand. I will try it again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants