GNOME Bugzilla – Bug 688625
gst-launch: incorrect parsing behaviour with spaces and quotes
Last modified: 2015-03-27 17:40:03 UTC
In 0.10.36 on F17 quoting behavior is incorrect. As a comparison, A) gst-launch-0.10 audiotestsrc num-buffers=1 ! filesink location='12".dump' correctly creates a file called 12".dump but B) gst-launch-0.10 audiotestsrc num-buffers=1 ! filesink location='12" .dump' (space after double quote) creates 12"\ .dump (inserts an actual backslash after the double quote) C) gst-launch-0.10 audiotestsrc num-buffers=1 ! filesink location='".dump' creates .dump (ie, drops the leading quote AND the last letter!) If all the examples are changed to swap single for double quotes, all of them work fine. In Fedora 16 with 0.10.35, A) and B) are correct, and C) is wrong too. So at least B) is a regression going from 0.10.35 to 0.10.36
Crap, example C) was supposed to end with "creates .dum" but muscle memory made me type the p.
Still some problems with git master: B) now fails differently, and I think this failure is somewhat correct because the ' shell quotes don't get passed to gst-launch; although it could probably handle this by treating spaces within argv[x] differently. $ gst-launch-1.0 audiotestsrc num-buffers=1 ! filesink location='12" .dump' 0:00:00.027121488 32286 0x2699560 ERROR GST_PIPELINE ./grammar.y:757:priv_gst_parse_yyparse: unexpected pad-reference "dump" - ignoring WARNING: erroneous pipeline: unexpected pad-reference "dump" - ignoring C) This one is "interesting": $ gst-launch-1.0 audiotestsrc num-buffers=1 ! filesink location='".dump' $ ls -l .dum -rw-r--r-- 1 tpm tpm 2048 Nov 28 11:10 .dum
Can confirm both B and C as Tim explained. Looking into why C happens.
I've pinpointed the problem to happen in the following line of the gstreamer core module: gst/parse/grammar.y:1049: if (yyparse (scanner, &g) != 0) { The scanner has a correct looking pipeline string but the element graph (graph_t g) created by yyparse has the filesink with the truncated location. Investigating further.
Created attachment 300292 [details] [review] parse: check before truncating strings Unless I missunderstand the expected behaviour. If a property has a trailing double quote, which means the last character isn't also a double quote, the parser shouldn't truncate this last character. gst-launch-1.0 audiotestsrc num-buffers=1 ! filesink location='".dump' creates a file: ".dump gst-launch-1.0 audiotestsrc num-buffers=1 ! filesink location='".dump"' creates a file: .dump
Created attachment 300377 [details] [review] Cleaner fix
Created attachment 300378 [details] [review] Don't unwrap strings that aren't delimited with quotes
Created attachment 300379 [details] [review] Add tests to cover the new fixes
Created attachment 300474 [details] [review] Check string needs unwrapping before running gst_string_unwrap()
Comment on attachment 300474 [details] [review] Check string needs unwrapping before running gst_string_unwrap() Looks fine to me. Please elaborate a bit more in the commit message the change of behaviour from before to after, i.e. that before we assumed any string that starts with a " is an escaped one and needs unescaping, and if there's no " at the end that's an error; whereas now we only unescape strings that are fully quoted and if not we assume the string doesn't need unescaping but is fine as is.
Pushed! commit c565c5ecb4f3dd766ff9156a4cf2d5d5adbc0396 commit 555e0211c8c67814546eca1c572af78840902eba commit ae0b2ec46bd91530a9572c220511f055b7caac35 Thanks Tim for all the help :)