On Mon, 29 Apr 2013 16:30:20 +0200 Thomas Fehr <[email protected]> wrote: > Hi all, > > Last week I was confronted with an L3 from Intel regarding autoyast. > After some investigations I found out it was caused by them using > "<fstopt config:type="symbol">pri=1</fstopt>" > instead of correct > "<fstopt>pri=1</fstopt>" > in partitioning section of autoyast xml code (since they let autoyast > xml code itself been generated by scripts this was not completely > easy to see). Ok, there are many ways to break an autoyast > installation with wrong xml entries but the effects of above bug was > quite surprising. YaST2 scr crashed at end of installation when > autoyast writes autoyast settings with SCR::Write( .target.ycp, ...) > into installed system. At this point in installation SCR has already > been restarted below /mnt and the chrooted SCR gets the ycp code via > parsing stdin. > > Here the problem really starts: > > Above xml leads to followingq map entry: $["fstopt" : `pri=1], but > the YCP parser will not accept this. > This leads to chrooted SCR at end of installation to crash which > of course wreaks furter havoc afterwards. > > YCP parser in core/libycp/src/scanner.ll > defines "SYMBOL ([[:alpha:]_][[:alnum:]_]+|[[:alpha:]][[:alnum:]_]*)". > > While xml/src/XmlAgent.cc accepts any string: > > else if (!xmlStrcmp(confAttr, (const xmlChar *)"symbol")) > { > y2debug("Symbol found"); > YCPSymbol sym((const char *)lastChild->content); > return YCPSymbol(sym); > } > > IHMO, we should make that a little more robust. I am not sure if > autoyast is the only part of yast2 where xml code provided by user is > parsed into ycp. > > Any suggestions? > > Tschuess, > Thomas Fehr
Well, I read that code quite lot in recent weeks due to ruby conversion project and it is not surprise that there is such inconsistencies, there are even more inconsistencies in ycp, its parser and implementation of other components that builds ycp values from own values ( like perl, ruby bindings or packager component ) I think that in this case the most correct solution is let YCPSymbol constructor decide what is correct symbol ( downside is that it is not possible to change return value to nil if something is wrong, as we do not use factory to build values and also we do not support exceptions in constructor ). So how to have it? Because symbol allows any string even containing spaces, \r, beep sound or even more strange chars, it is not easy to add it to parser in plain parts after backslash. But solution can be inspired by ruby, that already have symbol and if symbol contains strange characters you can enclose symbol into quotes...so you can have :"tr=1". To allow such change it needs to modify 1) parser to recognize string after backslash (something like `<string>: rule) 2) modify toString method to properly construct ycp code ( BTW that reminds me, that due to low usage of toString method, the method contains bugs for some constructs like switch statement ) Just my two cents Josef -- To unsubscribe, e-mail: [email protected] To contact the owner, e-mail: [email protected]
