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

Jump to bottom

popen doesn't report errors related to ls globbing too many arguments. #109

Closed
mklauber opened this issue Dec 11, 2017 · 2 comments
Closed

popen doesn't report errors related to ls globbing too many arguments. #109

mklauber opened this issue Dec 11, 2017 · 2 comments
Milestone
v0.12.0

Comments

@mklauber
Copy link
Contributor

mklauber commented Dec 11, 2017 •
edited
Loading

https://github.com/GUI/lua-resty-auto-ssl/blob/8227d90bfa7e6abee5ecf516721df986ef2ab702/lib/resty/auto-ssl/storage_adapters/file.lua#L76-L80

In the line above, if globbing *:latest results in ls being sent too many arguments, ls returns an error in stderr, and the variable out gets no results. This is invisibly handled as 0 domains, rather than an error. Should check stderr or the return code to determine if this occurred.

Possible fix would include switching to using find.
@GUI GUI added this to the v0.12.0 milestone Jan 29, 2018
GUI added a commit that referenced this issue Jan 29, 2018
@GUI
Fix renewal checks for file adapter if directory contains too many files
3b8adea 
#109

If the file storage adapter was being used and the directory contained
too many files to perform glob matching on, the file adapter would
report no files when checking for renewals.

This should fix it by using "find" instead of shell globbing. Any errors
should also now be reported.
@GUI
Copy link
Collaborator

GUI commented Feb 5, 2018

@mklauber: Thanks for reporting this! This should be fixed in the v0.12.0 release, since we've switched to using find, as you suggested. But give a shout if you're still seeing any issue.

@GUI GUI closed this as completed Feb 5, 2018
@mklauber
Copy link
Contributor Author

mklauber commented Feb 5, 2018

@GUI Thanks for the fix, and for the shout. We actually migrated to the redis backend as a workaround for this, so I can't confirm the fix, but the code looks right to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v0.12.0
Development

No branches or pull requests
2 participants
@GUI @mklauber