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.

Sign up for GitHub

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

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions lib/resty/acme/autossl.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -443,9 +443,8 @@ function AUTOSSL.ssl_certificate()
elseif certkey == null then
log(ngx_DEBUG, "negative cached domain cert")
elseif certkey then
if chains_set_count == 0 then
if i == 1 then
ssl.clear_certs()
chains_set_count = chains_set_count + 1
end
chains_set[i] = true

Expand All @@ -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 •
edited
Loading

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.


if domain_key_types_count ~= chains_set_count then
ngx.timer.at(0, function()
for i, typ in ipairs(domain_key_types) do
if not chains_set[i] then
Expand Down