Bug #3967
closedChange request cannot be accepted: multiline text cause merge incompatibility
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
Updated by Vincent MEMBRÉ about 11 years ago
- 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
Updated by Vincent MEMBRÉ about 11 years ago
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;
Updated by François ARMAND about 11 years ago
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...)
Updated by François ARMAND about 11 years ago
- 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...
Updated by Vincent MEMBRÉ about 11 years ago
Here is a stack overflow answer about invalid characters in XML: http://stackoverflow.com/questions/730133/invalid-characters-in-xml
Char ::= #x9 | #xA | #xD | [#x20-#xD7FF] | [#xE000-#xFFFD] | [#x10000-#x10FFFF] /* any Unicode character, excluding the surrogate blocks, FFFE, and FFFF. */
\r is in this list #xD (\n too #xA)
Updated by Vincent MEMBRÉ about 11 years ago
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.
Updated by Vincent MEMBRÉ about 11 years ago
- 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
New pull request here: https://github.com/Normation/rudder/pull/326
Updated by Vincent MEMBRÉ about 11 years ago
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.
Updated by François ARMAND about 11 years ago
- 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.
Updated by Vincent MEMBRÉ about 11 years ago
- Assignee changed from Vincent MEMBRÉ to François ARMAND
I updated the PR.
Updated by Vincent MEMBRÉ about 11 years ago
- Status changed from Discussion to Pending technical review
Updated by Vincent MEMBRÉ about 11 years ago
- Status changed from Pending technical review to Pending release
- % Done changed from 0 to 100
Applied in changeset 676362d8b559aef5a314cc8ae74e49d6d1f18b09.
Updated by Anonymous about 11 years ago
Applied in changeset f8a6ed0efeea418848f39e1382d04a4f1a891e87.
Updated by Nicolas PERRON about 11 years ago
- Status changed from Pending release to Released
Updated by François ARMAND over 7 years ago
- Related to Architecture #3972: Scala XML library accepts invalid XML characters added
Updated by François ARMAND almost 2 years ago
- Related to Bug #22208: Directives cannot be deleted because change request say state diverged even when there are no change request activated added