Bug #3967
closed
Change request cannot be accepted: multiline text cause merge incompatibility
Added by Vincent MEMBRÉ about 11 years ago.
Updated about 11 years ago.
Category:
Web - Maintenance
Description
Sometimes, when creating a Change request about a Directive with multiple lines parameters (ie in enforce file content ) the change request cannot be validated due to a changed initial state.
But the initial state has not changed, the change request is not mergeable directly after the creating it.
the following examples throw the bug :
aaa
aaa
aaa
Sometimes the same text does not throw an error, but I can't undertand why
- Status changed from New to Pending technical review
- Assignee changed from Vincent MEMBRÉ to François ARMAND
I finally understood what at stake here:
- Sometimes (not every time, still don't know why though, ), some line breaks are saved with '\r\n' (directly from LDAP)
- when saved in dabatase as an xml, the text stays (so still having the \r\n)
- The problem is : \r\n is not a valid xml line break, and when getting back this value as an xml it will be replaced by \n (see http://stackoverflow.com/questions/2265966/xml-carriage-return-encoding) the valid representation of a line break in a xml
- so all those texts were missing some characters (the \r, even if they look like they havent changed)
the solution is to transform \r before saving it to the databse to their valid xml value ->
I included debug logs for change requests, easing the process to understand what cause the merge conflict
Pull request here : https://github.com/Normation/rudder/pull/325
to repair a broken change request run this psql query :
update changerequest set content = xmlparse( document replace(XMLSERIALIZE (DOCUMENT content as text), chr(13), ' ')) where id = x;
Are we sure we are escaping all invalid XML chars ? There is no point on escaping only "\r" if other chars could lead to the same behaviour.
In fact, there is in Scala scala.xml.Utility.escape / unescape. And we shoudl use it when we are saving in XML... And unescaping on deserialisation.
Whell, so the bug seems far more involved than what is shown here. I don't have a clear vision about how to adress that for now (inpact is huge... We use XML in a lot of place...)
- Status changed from Pending technical review to 8
- Assignee changed from François ARMAND to Vincent MEMBRÉ
Well, perhaps there is a bug on merge that should ignore control chars, and an other bug that is that we don't do clean XML serialisation / deserialisation...
List in previous comment is about valid characters,
so \r and \n are valid characters, but numerous xml parsers replace \r by an \n (see www.w3.org/TR/xml/#sec-common-syn)
To fix our problem here, we will normalize datas by serializing into xml datas from LDAP and deserializing then right after to get them into the same state than datas in change request.
- Status changed from 8 to Pending technical review
- Assignee changed from Vincent MEMBRÉ to François ARMAND
- Pull Request set to https://github.com/Normation/rudder/pull/326
I added a commit to trim directive parameters(if an \r was inserted it was trimmed in LDAP but not in psql...)
François, if it's ok, don't merge so i can rebase it.
- Status changed from Pending technical review to Discussion
- Assignee changed from François ARMAND to Vincent MEMBRÉ
Some comments.
I really don't think that you have to restrict directive check to Directive. If we choose to embed all of (Technique, Directive, RootSpec) in the change request, we should allows to check on the three of them.
All other parts seem good.
- Assignee changed from Vincent MEMBRÉ to François ARMAND
- Status changed from Discussion to Pending technical review
- Status changed from Pending technical review to Pending release
- % Done changed from 0 to 100
- Status changed from Pending release to Released
- Related to Bug #22208: Directives cannot be deleted because change request say state diverged even when there are no change request activated added
Also available in: Atom
PDF