fix!: prevent bad chars in session names#368
Conversation
ethanpailes
force-pushed
the
fix-bad-sess-name-chars
branch
from
f675658 to
ae90ff5
Compare
| eprintln!("session name '{}' may not have whitespace", resolved_name); | ||
| return Ok(AttachResult::Done); | ||
| } | ||
| if resolved_name.chars().any(|c| '/' == c) { |
There was a problem hiding this comment.
it seems like it'd be useful to have slashes for grouping session names. e.g. something like fixing-bugs/1 fixing-bugs/2 etc. I guess we're doing this because the session name gets passed directly into some file operations. Have you considered URL encoding the names for filesystem purposes?
There was a problem hiding this comment.
I kinda feel like there are lots of other potential seperators people can use, and I would rather not make the rules about which files go where overly complicated.
There was a problem hiding this comment.
If you take out period, white space and slash there's not really any seperators left (I mean I guess dash?) If you use URL encoding you can get rid of all these other checks you have to do too, so the code would probably become simpler if anything. The only downside is that the filepath that gets used will be ugly, but people can just not use special characters in their session name if they care about that.
This patch adds a guard against bad chars in session names. It also adds a check for bad session names into the daemon just for extra defensiveness, though the attach proc is probably good enough. Closes #362
ethanpailes
force-pushed
the
fix-bad-sess-name-chars
branch
from
ae90ff5 to
41b52d2
Compare
maxhbooth
left a comment
maxhbooth
left a comment
There was a problem hiding this comment.
I'll go ahead and approve this, but I do think you should consider using URL encoding (or whatever other encoding you like.)
|
If people complain we can relax the rule later and add directory encoding logic, but let's try the simpler approach first. |
2 participants
Issue Link
#362
AI Policy Ack
Ack
This PR was:
Description
This patch adds a guard against bad chars in session names. It also adds a check for bad session names into the daemon just for extra defensiveness, though the attach proc is probably good enough.
Closes #362