Skip to content
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

"expr: syntax error" in juis_check #19

Closed
goligo opened this issue Dec 14, 2018 · 22 comments
Closed

"expr: syntax error" in juis_check #19

goligo opened this issue Dec 14, 2018 · 22 comments

Comments

@goligo
Copy link

goligo commented Dec 14, 2018

Tried juis on my Mac (Mojave 10.14.2) with my 6490, but the following error occurs when running it:

debug: -------------------------------------------------------
debug: Reading values from 'fritz.box:80/jason_boxinfo.xml': .
debug: Read response from device:
debug: -------------------------------------------------------
HTTP/1.1 200 OK
Cache-Control: max-age=120
Connection: close
Content-Type: text/xml;charset=utf-8
Date: Fri, 14 Dec 2018 17:38:40 GMT
ETag: "D008DE7B64CF95EFB"
Expires: Fri, 14 Dec 2018 17:40:40 GMT
Last-Modified: Thu, 01 Jan 1970 00:01:33 GMT
Mime-Version: 1.0

   <j:BoxInfo xmlns:j="http://jason.avm.de/updatecheck/">
   <j:Name>FRITZ!Box 6490 Cable</j:Name>
   <j:HW>213</j:HW>
   <j:Version>141.06.51</j:Version>
   <j:Revision>34089</j:Revision>
   <j:Serial>entfernt</j:Serial>
   <j:OEM>avm</j:OEM>
   <j:Lang>de</j:Lang>
   <j:Annex>Kabel</j:Annex>
   <j:Lab></j:Lab>
   <j:Country>049</j:Country>
   <j:Flag>crashreport</j:Flag>
   <j:UpdateConfig>1</j:UpdateConfig></j:BoxInfo>

debug: Splitting compound version number '141.06.51-34089' to:
expr: syntax error
debug: Major=141
debug: Minor=06
debug: Patch=51
debug: Buildnumber=34089
debug: Compound version number used for request: '34089.00.00-'

debug: -------------------------------------------------------
debug: Variables set:
debug: -------------------------------------------------------
debug: Name="FRITZ!Box 6490 Cable"
debug: HW="213"
debug: OEM="avm"
debug: Lang="de"
debug: Annex="Kabel"
debug: Country="049"
debug: Serial="entfernt"
debug: Major=""
debug: Minor=""
debug: Patch=""
debug: Buildnumber="34089"
[...]

The request to the update service is called without version info, causing a 500 response:

           <q:Name>FRITZ!Box 6490 Cable</q:Name>
           <q:HW>213</q:HW>
           <q:Major></q:Major>
           <q:Minor></q:Minor>
           <q:Patch></q:Patch>
           <q:Buildnumber>34089</q:Buildnumber>
           <q:Buildtype>1001</q:Buildtype>
@goligo goligo changed the title "expr: syntax error" in juis "expr: syntax error" in juis_check Dec 14, 2018
@goligo
Copy link
Author

goligo commented Dec 14, 2018

The "expr: syntax error" seems not to do any harm - the empty Major/Minor/Patch version is caused by the last change

eaa8e3b

When checking out the previous version it works as expected.

@PeterPawn
Copy link
Owner

PeterPawn commented Dec 14, 2018

Possibly the MacOS version of "expr" uses a different syntax ... once again.

It was necessary to apply the latest changes, because the new version number "07.08" raised an error.

If you look at your debug output, the error seems to sit between the "split_version_number" call (https://github.com/PeterPawn/YourFritz/blob/master/juis/juis_check#L1356) and the immediately following debug output. Therefore the error has to occur in the subfunction call ... but the output values (shown in the debug lines after this call) are looking good.

It surely a MacOS related problem, because the code runs without error on a "native" Linux system:

debug: Splitting compound version number '141.06.51-34089' to:
debug: Major=141
debug: Minor=06
debug: Patch=51
debug: Buildnumber=34089
debug: Compound version number used for request: '141.06.51-34089'

I have no idea, why the last version swaps the values on your system:

debug: Compound version number used for request: '34089.00.00-'

... that's obviously the wrong order and any later error parsing this string isn't really unexpected.

But at the same time I can't see any interrelation between the latest patch and the code used for parsing this version number ... as a result, I can't understand, why the previous version works for you.

If you want to get this error analyzed by me, please run the script using an explicit shell call with "-x" option and show me the trace output produced by your shell. I'm interested in fixing the problem - but I haven't a MacOS-based system anymore and the existing code runs in "pure POSIX mode" with a "dash" shell without errors.

The line, where the final version string is prepared (it's line 1458 in the latest version and the content of line 639 gets evaluated there - no "expr" call for miles around here), was not changed in the last commit.

@goligo
Copy link
Author

goligo commented Dec 14, 2018

Thanks for your response. I also tried on a FreeBSD 11.2 system which is showing the same issue. As I wrote above it seems to be unrelated to the expr error, which also occurs in the working version, but is just caused by the latest change.

There is a lot of output when doing the shell call with -x, the relevant part seems to be

+ eval 'v=$Major'
+ v=141
+ get_decimal_value 141
+ expr 141 : '0*\([0-9]\+\)'
+ eval 'Major='
+ Major=''
+ eval 'v=$Minor'
+ v=06
+ get_decimal_value 06
+ expr 06 : '0*\([0-9]\+\)'
+ eval 'Minor='
+ Minor=''
+ eval 'v=$Patch'
+ v=51
+ get_decimal_value 51
+ expr 51 : '0*\([0-9]\+\)'
+ eval 'Patch='
+ Patch=''
+ [ 0 -eq 1 ]
+ eval 'Version=$(printf %d.%02d.%02d-%s $Major $Minor $Patch $Buildnumber)'
+ printf %d.%02d.%02d-%s 34089
+ Version=34089.00.00-

As far I understand get_decimal_value does just return empty values.

@goligo
Copy link
Author

goligo commented Dec 14, 2018

POSIX basic regular expressions do not support + or ?, instead you need to write {1,} or {0,1}

get_decimal_value()
(
        expr "$1" : "0*\([0-9]\{1,\}\)"
)

does fix it.

@PeterPawn
Copy link
Owner

That's correct (let's ignore the "expr" error for now - but I want to fix this later, too) ... the real question is, why the regex is annoying the "expr" utility on BSD derivates.

Please try it again with the script from new branch 'juis' ... I've changed the content of the regex operand slightly. Maybe it's working now on BSD systems, too.

@PeterPawn
Copy link
Owner

PeterPawn commented Dec 14, 2018

BTW ... {0,1} fails (using "dash") on all-zeros input:

expr "0000" : "0*\([0-9]\{0,1\}\)"

The wanted result is "0" here. But I didn't see the additional message, before I committed the new version.

EDIT: Sorry, got it really late, but I see it now ... the {0,1} part was only the alternative to the question mark.

The other option {1,} could be used, but there's no problem with the expression [1-9]*[0-9] instead.


If the new version works as expected and you're able to locate the statement, which raises the earlier error message regarding the "expr" call (it should be a different problem), I'd like to fix this error, too.

@goligo
Copy link
Author

goligo commented Dec 14, 2018

This is where the syntax error occurs

+ __first_char -%Buildnumber%]
+ expr '(' -%Buildnumber%] : '\(.\).*' ')'
+ c=-
+ __remove_first_char -%Buildnumber%]
+ expr '(' -%Buildnumber%] : '.\(.*\)' ')'
+ m=%Buildnumber%]
+ [ - '=' % ]
+ expr '(' - : '\([A-Za-z]\)' ')'
expr: syntax error

@PeterPawn
Copy link
Owner

Hmm ... the outer parentheses should protect the single 'dash' character (as input) from being recognized as an "option starts here" marker.

Could you please try this as a single command:

c='-'; o=$(expr -- \( "$c" : "\([A-Za-z]\)" \) ); echo "${#o} $o"

and compare it with the version without the "double-dashes" in front of the other arguments?

The problem is, that some "expr" implementations (e.g. the one from BusyBox) do not accept a double-dash in front of the other arguments, because the "expr" command does not take any options usually (http://pubs.opengroup.org/onlinepubs/9699919799/) and the BSD version is a big exception here. Most other "expr" implementations haven't own options, but the double-dash will be detected and processed as expected:

peh@vidar:~> expr -- - : "\(.\)"
-
peh@vidar:~> busybox expr -- - : "\(.\)"
expr: non-numeric argument
peh@vidar:~> busybox expr - : "\(.\)"
-
peh@vidar:~>

As mentioned above, the parentheses around the whole expression were an alternative here, because they should start the evaluation of the "inner expression" and there can't be a valid option within the parentheses anymore.

I'm surprised and a bit helpless, why it does not work as expected in this case.

@goligo
Copy link
Author

goligo commented Dec 14, 2018

The problem seems to be with the standalone minus on the left side. I couldn't figure out any reasonable way to quote or to escape this - easiest way to solve seems to add some prefix on both sides, like

elif [ -z "$(expr \( "_$c" : "\(_[A-Za-z]\)" \) )" ]; then

@goligo
Copy link
Author

goligo commented Dec 14, 2018

c='-'; o=$(expr -- ( "$c" : "([A-Za-z])" ) ); echo "${#o} $o"
Illegal variable name.

expr -- - : "(.)"
expr: syntax error

expr - : "(.)"
expr: syntax error

@goligo
Copy link
Author

goligo commented Dec 15, 2018

As far I understand the issue is not that expr is trying to interprete the minus as a command line option, but the minus is a valid operator, and the documentation of expr states

"An operand which is lexically identical to an operator will be considered a syntax error."

@PeterPawn
Copy link
Owner

peh@vidar:~> c='-'; o=$(expr -- \( "$c" : "\([A-Za-z]\)" \) ); echo "${#o} $o"
0
peh@vidar:~> c='-'; o=$(expr \( "$c" : "\([A-Za-z]\)" \) ); echo "${#o} $o"
0
peh@vidar:~> c='A'; o=$(expr \( "$c" : "\([A-Za-z]\)" \) ); echo "${#o} $o"
1 A
peh@vidar:~> c='A'; o=$(expr -- \( "$c" : "\([A-Za-z]\)" \) ); echo "${#o} $o"
1 A
peh@vidar:~>

It's a known problem to me (ed40574 and PeterPawn/YourFreetz@f7d6aec) ... but I do not understand, why the version with outer parentheses will not work in this case.

Usually I'm restrained and do not blame other programs, they're working "wrong" ... but in this case it looks like an error by "expr" on BSD systems. Why should it be a syntax error, if the expression to evaluate starts with a group of parentheses? It's a "grouping" of operands, as described by the POSIX specification.

@PeterPawn
Copy link
Owner

Does it change anything, if you define a "EXPR_COMPAT" environment variable prior to executing the commands?

@goligo
Copy link
Author

goligo commented Dec 15, 2018

Well, it is very old, always worked like this, and is unlikely to be changed.

@PeterPawn
Copy link
Owner

I'm looking for a portable solution on all platforms. The last resort would be an appended prefix or suffix or an additional test, whether the character is a single dash, with the result, that the "expr" call is not made in this case.

@goligo
Copy link
Author

goligo commented Dec 15, 2018

$ set | grep EXPR_COMPAT
EXPR_COMPAT=1
$ c='-'; o=$(expr ( "$c" : "([A-Za-z])" ) ); echo "${#o} $o"
expr: syntax error
0

@goligo
Copy link
Author

goligo commented Dec 15, 2018

expr first appeared in Unix V7, released in 1979 ;-)

@PeterPawn
Copy link
Owner

Ok, I've published a new version with an additional check for the hyphen ... it's in the "juis" branch now.

Could you try this version once more please?

Thanks for your patience.

@PeterPawn
Copy link
Owner

PeterPawn commented Dec 15, 2018

I was sixteen at this time and started working with computers. :-)

@goligo
Copy link
Author

goligo commented Dec 15, 2018

Looks good now, only the response is not satisfying me... but that is a different issue.

juis_check: No newer version found, check was made with source version '141.06.51-34089'.

@PeterPawn
Copy link
Owner

Thanks again, I'll move the new version from its own branch to "master" and close this issue afterwards.

@PeterPawn
Copy link
Owner

Solved with new version from 3a5570a - closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants