-
-
Notifications
You must be signed in to change notification settings - Fork 900
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
Add functionality to write to a temp file through url arguments #747
base: main
Are you sure you want to change the base?
Changes from all commits
ed9511a
cde0982
25f91ea
53565c7
5a3ad2f
4be5009
a487955
092dc3d
b9b962b
c01676c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -101,8 +101,37 @@ static char **build_args(struct pss_tty *pss) { | |
argv[n++] = server->argv[i]; | ||
} | ||
|
||
for (i = 0; i < pss->argc; i++) { | ||
argv[n++] = pss->args[i]; | ||
if (server->url_arg) { | ||
for (i = 0; i < pss->argc; i++) { | ||
argv[n++] = pss->args[i]; | ||
} | ||
} else if (server->arg_file != NULL) { | ||
int fd = -1; | ||
int file_path_len = strlen(server->arg_file) + 6 /*XXXXXX*/ + 1 /*null character*/; | ||
char *filePath = xmalloc(file_path_len); | ||
snprintf(filePath, file_path_len, "%sXXXXXX", server->arg_file); | ||
|
||
if ((fd = mkstemp(filePath)) == -1) { | ||
lwsl_err("Creation of temp file failed with error: %d (%s)\n", errno, strerror(errno)); | ||
free(filePath); | ||
return false; | ||
} | ||
|
||
for (i = 0; i < pss->argc; i++) { | ||
if (dprintf(fd, "%s\n", pss->args[i]) < 0) { | ||
lwsl_err("Write to temp file failed with error: %d (%s)\n", errno, strerror(errno)); | ||
close(fd); | ||
free(filePath); | ||
return false; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this will leak fd too. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed |
||
} | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. need to close the file |
||
if (close(fd) != 0) { | ||
lwsl_err("Close temp file failed with error: %d (%s)\n", errno, strerror(errno)); | ||
free(filePath); | ||
return false; | ||
} | ||
argv[n++] = filePath; | ||
} | ||
|
||
argv[n] = NULL; | ||
|
@@ -219,7 +248,7 @@ int callback_tty(struct lws *wsi, enum lws_callback_reasons reason, void *user, | |
pss->wsi = wsi; | ||
pss->lws_close_status = LWS_CLOSE_STATUS_NOSTATUS; | ||
|
||
if (server->url_arg) { | ||
if (server->url_arg || server->arg_file != NULL) { | ||
while (lws_hdr_copy_fragment(wsi, buf, sizeof(buf), WSI_TOKEN_HTTP_URI_ARGS, n++) > 0) { | ||
if (strncmp(buf, "arg=", 4) == 0) { | ||
pss->args = xrealloc(pss->args, (pss->argc + 1) * sizeof(char *)); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -70,6 +70,7 @@ static const struct option options[] = {{"port", required_argument, NULL, 'p'}, | |
{"ssl-key", required_argument, NULL, 'K'}, | ||
{"ssl-ca", required_argument, NULL, 'A'}, | ||
{"url-arg", no_argument, NULL, 'a'}, | ||
{"arg-file", required_argument, NULL, 'f'}, | ||
{"readonly", no_argument, NULL, 'R'}, | ||
{"terminal-type", required_argument, NULL, 'T'}, | ||
{"client-option", required_argument, NULL, 't'}, | ||
|
@@ -81,7 +82,7 @@ static const struct option options[] = {{"port", required_argument, NULL, 'p'}, | |
{"version", no_argument, NULL, 'v'}, | ||
{"help", no_argument, NULL, 'h'}, | ||
{NULL, 0, 0, 0}}; | ||
static const char *opt_string = "p:i:c:H:u:g:s:w:I:b:P:6aSC:K:A:Rt:T:Om:oBd:vh"; | ||
static const char *opt_string = "p:i:c:H:u:g:s:w:I:b:P:6af:SC:K:A:Rt:T:Om:oBd:vh"; | ||
|
||
static void print_help() { | ||
// clang-format off | ||
|
@@ -100,6 +101,7 @@ static void print_help() { | |
" -s, --signal Signal to send to the command when exit it (default: 1, SIGHUP)\n" | ||
" -w, --cwd Working directory to be set for the child program\n" | ||
" -a, --url-arg Allow client to send command line arguments in URL (eg: http://localhost:7681?arg=foo&arg=bar)\n" | ||
" -f, --arg-file File prefix for a unique generated temp file that URL arguments are written to (ex. /tmp/prefix); the generated file's full path is then passed in as a command line argument (ex. /tmp/prefix{unique string}). The command is responsible for deleting the file.\n" | ||
" -R, --readonly Do not allow clients to write to the TTY\n" | ||
" -t, --client-option Send option to client (format: key=value), repeat to add more options\n" | ||
" -T, --terminal-type Terminal type to report, default: xterm-256color\n" | ||
|
@@ -146,6 +148,7 @@ static void print_config() { | |
if (server->auth_header != NULL) lwsl_notice(" auth header: %s\n", server->auth_header); | ||
if (server->check_origin) lwsl_notice(" check origin: true\n"); | ||
if (server->url_arg) lwsl_notice(" allow url arg: true\n"); | ||
if (server->arg_file != NULL) lwsl_notice(" temp file name prefix: %s\n", server->arg_file); | ||
if (server->readonly) lwsl_notice(" readonly: true\n"); | ||
if (server->max_clients > 0) lwsl_notice(" max clients: %d\n", server->max_clients); | ||
if (server->once) lwsl_notice(" once: true\n"); | ||
|
@@ -197,6 +200,7 @@ static struct server *server_new(int argc, char **argv, int start) { | |
|
||
static void server_free(struct server *ts) { | ||
if (ts == NULL) return; | ||
if (ts->arg_file != NULL) free(ts->arg_file); | ||
if (ts->credential != NULL) free(ts->credential); | ||
if (ts->auth_header != NULL) free(ts->auth_header); | ||
if (ts->index != NULL) free(ts->index); | ||
|
@@ -346,6 +350,11 @@ int main(int argc, char **argv) { | |
break; | ||
case 'a': | ||
server->url_arg = true; | ||
server->arg_file = NULL; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think we might want to add a comment in help text, documentation that if both are set, behaviour is non deterministic. Alternatively we can make a choice that if both are set url_arg will win (or vice versa) Based on that you want to structure your There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The behaviour shouldn't be non deterministic here; the last specified argument of |
||
break; | ||
case 'f': | ||
server->arg_file = strdup(optarg); | ||
server->url_arg = false; | ||
break; | ||
case 'R': | ||
server->readonly = true; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if this would make it clear that the argument file prefix needs to first be passed in and that the contents would be the URL arguments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should also say that the child process can do whatever it wants with the file (including deleting it)
(how you say it is a matter of taste and given that im not a maintainer of this repo, i will not give my suggestion :p )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with @dotslash. and we need to tell user that the temp file will not be deleted by ttyd, or add some logic to delete it automatically on websocket closing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added a note to the end