-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
tasks-runner component doesn't validate sock path #26936
Comments
Hey @vergilfromadyen thanks for the report and the detailed investigation. It seems reasonable to provide a better experience here... we already fall back to the workspace-folder socket dir if we can't access the socket dir. Otherwise a validation step might work as well though I'll have to research the proper limits. |
Thanks @MaxKless for the quick response. Setting To help your research:
Let me know if I can help in any other way. |
for sure but if it just fails, you'll have to set the env var anyways right. If both the temp dir and the workspace path are too long, you won't get around setting it manually. |
When throwing an error I'd still differentiate between a path potentially too long, and a generic "can't create sock file" message, because the former isn't really common knowledge. Other than that it sounds great. |
If you'd be willing to contribute a fix, I think this shouldn't be too complicated and I'll gladly help guide you in the right direction. But seeing as you've already identified a place for a possible fix, you probably won't even need it :P |
Sure, I'll make some time; you guys have a very cool product, might as well give back a bit :) |
Current Behavior
NX_DAEMON_SOCKET_DIR
to a path longer than 108 characters leads to anEADDRINUSE
errorTMPDIR
for its own purposesExpected Behavior
Steps to Reproduce
NX_DAEMON_SOCKET_DIR
to something long, e.g./tmp/xxxxxxxx-xxxxxxxxxxxxx-xxxxxxxxxx-xxxxxxxxxxxxx-xxxxxxxxxxx-xxxxxxxxxxxxxxx-xxxxxxxxxxxxxx-xxxxxxxxxxx
Operating System
Additional Information
Linux has a 108-char path limit, Windows has a MAX_PATH constant which is 256ish characters.
Extracted from this issue and addresses a subset of the problems in there.
Fix should be placed around here.
The text was updated successfully, but these errors were encountered: