Allow for longer command line strings. #5
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously, if the command line string exceeded 504 characters, the command would fail due to the command not being read in properly. This increases the limit to 2040 characters, adds a test, and adds a reference to this limit to the README.
GUI
added a commit
to auto-ssl/lua-resty-auto-ssl
that referenced
this pull request
Jul 21, 2016
This cropped up in our CI environment, since due to the recent changes to the letsencrypt_hooks arguments, we needed to pass additional command line arguments in. This worked fine in my local environment, but this ended up breaking on our CI environment. This uncovered the fact that sockproc limits the length of the command line string. Since some of the file paths happened to be longer in the CI environment, we ended up exceeding this limit. This resolves it by using a fork of sockproc to allow for longer command line strings: juce/sockproc#5
|
Nick, thanks for the patch! |
|
Thank you, much appreciated! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, if the command line string exceeded 504 characters, the command would fail due to the command not being read in properly. This would result in odd errors being returned, like
/bin/sh: 7cho: command not found.This increases the limit to 2040 characters, adds a test, and adds a reference to this limit to the README.
For this pull request, I took the very simplistic approach and just increased the existing hard-coded limit. However, this limit is still fairly arbitrary. While this should help if you happen to have longer commands (as I was encountering), there's still the possibility of overflowing this limit if you have super-duper long commands. If you'd prefer to allow for even longer strings, then reading in the data with something like
getline(which handles reallocs), might be better (although some limit may still need to be enforced). But I thought I'd get your feedback and thoughts before trying any larger changes.Thanks!