YunoHost / package_linter

Linter for YunoHost applications packages
https://yunohost.org/#/packaging_apps
GNU Affero General Public License v3.0
17 stars 13 forks source link

Rework script parsing : line-per-line (still using shlex) #54

Closed alexAubin closed 5 years ago

alexAubin commented 5 years ago

Sooo, again some refactoring / reworking ... because shlex was still doing some weird stuff as reported by @kay0u (see what you make me do ! :stuck_out_tongue_closed_eyes: ) and all those check with script["shlex"] and script["raw"] were weird...

So I went for feeding line one-by-one to shlex, and implementing a contains method for the new Script class ... Now it should be technically cleaner ... though I discovered that parsing lines one-by-one are not 100% a good idea if you have multiple-line string. So for instance, running on lstu will display :

 [Upgrade Script]

! Could not parse this line (No closing quotation) : echo "The new version of LSTU provide an admin and a stats area which required a password. 
! Could not parse this line (No closing quotation) : This password is: $password" > mail_to_send 

But I guess it's okay ...? At least an easy "fix" (though not a real issue for bash) is to add a \ at the end of the line.

Otherwise, this PR is mainly technical and should not impact the results.

Mickael-Martin commented 5 years ago

I found some informations about shlex, you must respect POSIX format. I had this bug too and adding double quotes on my vars was my fix. Can you past the code with errors ?

kay0u commented 5 years ago

The code with errors is here: https://github.com/YunoHost-Apps/lstu_ynh/blob/master/scripts/upgrade#L74

Mickael-Martin commented 5 years ago

I found no issue actually :

Analyzing package lstu_ynh/

 [Missing Files]

 [Sources Management]

 [Manifest]

 [Install Script]

 [Remove Script]

 [Upgrade Script]

✘ [YEP-2.4] ynh_abort_if_errors is missing. For details, look at https://github.com/YunoHost/issues/issues/419 

 [Backup Script]

 [Restore Script]
kay0u commented 5 years ago

The Upgrade Script analysis fail, and It shouldn't

Mickael-Martin commented 5 years ago

OK, I see, package_linter.py use non-posix mode at this step, so if I read the definition : Parsing Rules

When operating in non-POSIX mode, shlex will try to obey to the following rules.

Quote characters are not recognized within words (Do"Not"Separate is parsed as the single word Do"Not"Separate);
Escape characters are not recognized;
Enclosing characters in quotes preserve the literal value of all characters within the quotes;
Closing quotes separate words ("Do"Separate is parsed as "Do" and Separate);
If whitespace_split is False, any character not declared to be a word character, whitespace, or a quote will be returned as a single-character token. If it is True, shlex will only split words in whitespaces;
EOF is signaled with an empty string ('');
**It’s not possible to parse empty strings, even if quoted.**

For me, cut -d' ' -f1 cannot work in this mode, so, we set posix mode at True or we adapt our code (cut -d ' ' -f1 , with space after d seams work)

Psycojoker commented 5 years ago

That's a lot of work but maybe switch to a stronger parser remove those weird situations https://github.com/idank/bashlex (but that's quite a low level tool)