bugGNU Screen - Bugs: bug #25813, [REGRESSION] Not working: screen...

 
 
Show feedback again

bug #25813: [REGRESSION] Not working: screen -X stuff "a b"

Submitted by:  Micah Cowan <micahcowan>
Submitted on:  Tue 10 Mar 2009 05:41:22 PM UTC  
Votes:  15  
 
Category: Program LogicSeverity: 4 - Important
Priority: 5 - NormalStatus: Confirmed
Privacy: PublicAssigned to: None
Open/Closed: OpenRelease: None
Fixed Release: NonePlanned Release: 4.1.0
Work Required: 0 - Hours

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Fri 28 Aug 2009 06:50:52 AM UTC, comment #8:

I've attached a patch that should do the trick.

Essentially, it double quotes all -X arguments and escapes any already-present double quotes, before sending them.

(file #18652)

Max Kalashnikov <maxkalashnikov>
Fri 28 Aug 2009 04:09:27 AM UTC, comment #7:

I see what you mean, and, of course (as a sysadmin by trade), I agree.

Sadly, one can't have both full parsing (which strips quotes) and preservation of quotes.

However, since you don't necessarily want full parsing, perhaps a modified Parse() would do the trick, as might a quote-escaper on the SendCmdMessage() frontend. I'll see which, if either, is easy.

Max Kalashnikov <maxkalashnikov>
Thu 27 Aug 2009 05:54:51 AM UTC, comment #6:

Thanks very much for your analysis, Max.

Actually, I think we knew much of this already (but it's good to have the details all here for reference). I think Sadrul and I have both considered this a regression in behavior, especially since it has the potential to break pre-existing scripts.

I think the ideal solution would be to fix this behavior so it matches previous behavior, but still let ^a et friends have the same special meaning they do in an rc file. That is, parsing should be as it is for the rc file (which is what the #24924 fix did), except that it should pretend (where necessary) that we're already in the midst of a string literal (not require extra quotes).

This might be more work than it's worth; I think Sadrul indicated he was considering pulling his #24924 fix. I'd be bummed in this case, but I agree it's better than breaking pre-existing behavior. One can always insert a literal ^a if necessary: the shell always provides tricks for this; but breaking historical syntax shouldn't happen.

Micah Cowan <micahcowan>
Project Member
Thu 27 Aug 2009 05:30:15 AM UTC, comment #5:

98b6b410 fixing #24924 did this, so this is, perhaps, not a regression, in that the code now correctly parses commands sent in via -X.

This should probably be just a documentation change, making it clear that whatever is passed to -X is subsequently parsed by the backend screen, as in .screenrc, as a string, without regard to original argument tokenizing.

Max Kalashnikov <maxkalashnikov>
Thu 27 Aug 2009 04:54:15 AM UTC, comment #4:

Oops.. sorry

Max Kalashnikov <maxkalashnikov>
Thu 27 Aug 2009 04:44:56 AM UTC, comment #3:

It's definitely in this change:

<pre>
@@ -1339,8 +1565,10 @@ { char *args[MAXARGS];
int argl[MAXARGS];
- int n, *lp;
- register char *pp = args, p = mp->m.command.cmd;
+ char fullcmd[MAXSTR];+ register char *fc;
+ int n;
+ register char *p = mp->m.command.cmd;
struct acluser *user;
#ifdef MULTIUSER extern struct acluser EffectiveAclUser; / acls.c /@@ -1348,17 +1576,21 @@ extern struct acluser users; /* acls.c */
#endif

- lp = argl;
n = mp->m.command.nargs;
if (n > MAXARGS - 1)
n = MAXARGS - 1;
- for (; n > 0; n--)
+ for (fc = fullcmd; n > 0; n--)
{
- *pp++ = p;
- *lp = strlen(p);
- p += *lp++ + 1;
- }
- *pp = 0;
+ int len = strlen(p);
+ strncpy(fc, p, fullcmd + sizeof(fullcmd) - fc - 1);
+ p += len + 1;
+ fc += len;
+ *fc++ = ' ';
+ }
+ if (fc != fullcmd)
+ *--fc = 0;
+ if (Parse(fullcmd, fc - fullcmd, args, argl) <= 0)
+ return;
</pre>

Max Kalashnikov <maxkalashnikov>
Thu 27 Aug 2009 04:11:56 AM UTC, comment #2:

I also tweaked the MSG_VERSION in the current git sources, and the behavior with a 4.0.3 receiver is unchanged.

Max Kalashnikov <maxkalashnikov>
Thu 27 Aug 2009 04:03:36 AM UTC, comment #1:

I found it instructive to try running this from the colon, as this is somewhere the behavior does not differ, despite the fact that screen itself will strip quotes.

Max Kalashnikov <maxkalashnikov>
Tue 10 Mar 2009 05:41:22 PM UTC, original submission:

In current git sources,
screen -X stuff "a b"
is rejected with "invalid option 'a'", whereas
screen -X "stuff 'a b'"
or
screen -X stuff "'a b'"
will stuff
a b

In 4.0.3 (at least on Ubuntu), however,
screen -X "stuff 'a b'"
fails with "Unknown command: 'stuff 'a b''",
and
screen -X stuff "'a b'"
will stuff
'a b'
(with the quotes)

In Ubuntu 4.0.3 (should check a canonical 4.0.3),
screen -X stuff "a b" c
fails with "invalid option 'a b'", which suggests that the change isn't in number-of-args supported, but just in how shell args get processed by screen.

Micah Cowan <micahcowan>
Project Member

 

(Note: upload size limit is set to 512 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #18652:  screen-attacher.diff added by maxkalashnikov (1kB - application/octet-stream - proposed patch, without backing out parsing change)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by maxkalashnikov (Voted in favor of this item)
  • -unavailable- added by maxkalashnikov (Posted a comment)
  • -unavailable- added by micahcowan (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 15 encouragements so far.

    Only logged-in users can vote.

     

    Please enter 421 here (basic anti-spam test):

     

     

    Follow 2 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Mon 07 Sep 2009 11:14:10 PM UTCmaxkalashnikovCarbon-Copy-=>Added maxkalashnikov
    Fri 28 Aug 2009 06:50:52 AM UTCmaxkalashnikovAttached File-=>Added screen-attacher.diff, #18652
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup