https://issues.rudder.io/https://issues.rudder.io/themes/rudder7/favicon/favicon.ico?17096450182013-09-27T15:43:35ZIssue TrackerRudder - Bug #3967: Change request cannot be accepted: multiline text cause merge incompatibilityhttps://issues.rudder.io/issues/3967?journal_id=207602013-09-27T15:43:35ZVincent MEMBRÉvme@rudder.io
<ul><li><strong>Status</strong> changed from <i>New</i> to <i>Pending technical review</i></li><li><strong>Assignee</strong> changed from <i>Vincent MEMBRÉ</i> to <i>François ARMAND</i></li></ul><p>I finally understood what at stake here:</p>
<p>- Sometimes (not every time, still don't know why though, ), some line breaks are saved with '\r\n' (directly from LDAP)<br />- when saved in dabatase as an xml, the text stays (so still having the \r\n)<br />- 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 <a class="external" href="http://stackoverflow.com/questions/2265966/xml-carriage-return-encoding">http://stackoverflow.com/questions/2265966/xml-carriage-return-encoding</a>) the valid representation of a line break in a xml</p>
<p>- so all those texts were missing some characters (the \r, even if they look like they havent changed)</p>
<p>the solution is to transform \r before saving it to the databse to their valid xml value -> <strong> </strong></p>
<p>I included debug logs for change requests, easing the process to understand what cause the merge conflict</p>
<p>Pull request here : <a class="external" href="https://github.com/Normation/rudder/pull/325">https://github.com/Normation/rudder/pull/325</a></p> Rudder - Bug #3967: Change request cannot be accepted: multiline text cause merge incompatibilityhttps://issues.rudder.io/issues/3967?journal_id=207612013-09-27T15:45:23ZVincent MEMBRÉvme@rudder.io
<ul></ul><p>to repair a broken change request run this psql query :</p>
<pre>
update changerequest set content = xmlparse( document replace(XMLSERIALIZE (DOCUMENT content as text), chr(13), '&#13;')) where id = x;
</pre> Rudder - Bug #3967: Change request cannot be accepted: multiline text cause merge incompatibilityhttps://issues.rudder.io/issues/3967?journal_id=207632013-09-27T16:26:56ZFrançois ARMANDfrancois.armand@rudder.io
<ul></ul><p>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.</p>
<p>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.</p>
<p>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...)</p> Rudder - Bug #3967: Change request cannot be accepted: multiline text cause merge incompatibilityhttps://issues.rudder.io/issues/3967?journal_id=207642013-09-27T16:28:35ZFrançois ARMANDfrancois.armand@rudder.io
<ul><li><strong>Status</strong> changed from <i>Pending technical review</i> to <i>8</i></li><li><strong>Assignee</strong> changed from <i>François ARMAND</i> to <i>Vincent MEMBRÉ</i></li></ul><p>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...</p> Rudder - Bug #3967: Change request cannot be accepted: multiline text cause merge incompatibilityhttps://issues.rudder.io/issues/3967?journal_id=207652013-09-27T16:32:16ZVincent MEMBRÉvme@rudder.io
<ul></ul><p>Here is a stack overflow answer about invalid characters in XML: <a class="external" href="http://stackoverflow.com/questions/730133/invalid-characters-in-xml">http://stackoverflow.com/questions/730133/invalid-characters-in-xml</a></p>
<pre>
Char ::= #x9 | #xA | #xD | [#x20-#xD7FF] | [#xE000-#xFFFD] | [#x10000-#x10FFFF] /* any Unicode character, excluding the surrogate blocks, FFFE, and FFFF. */
</pre>
<p>\r is in this list #xD (\n too #xA)</p> Rudder - Bug #3967: Change request cannot be accepted: multiline text cause merge incompatibilityhttps://issues.rudder.io/issues/3967?journal_id=207662013-09-30T10:09:12ZVincent MEMBRÉvme@rudder.io
<ul></ul><p>List in previous comment is about valid characters,</p>
<p>so \r and \n are valid characters, but numerous xml parsers replace \r by an \n (see <a class="external" href="http://www.w3.org/TR/xml/#sec-common-syn">www.w3.org/TR/xml/#sec-common-syn</a>)</p>
<p>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.</p> Rudder - Bug #3967: Change request cannot be accepted: multiline text cause merge incompatibilityhttps://issues.rudder.io/issues/3967?journal_id=207782013-09-30T16:46:50ZVincent MEMBRÉvme@rudder.io
<ul><li><strong>Status</strong> changed from <i>8</i> to <i>Pending technical review</i></li><li><strong>Assignee</strong> changed from <i>Vincent MEMBRÉ</i> to <i>François ARMAND</i></li><li><strong>Pull Request</strong> set to <i>https://github.com/Normation/rudder/pull/326</i></li></ul><p>New pull request here: <a class="external" href="https://github.com/Normation/rudder/pull/326">https://github.com/Normation/rudder/pull/326</a></p> Rudder - Bug #3967: Change request cannot be accepted: multiline text cause merge incompatibilityhttps://issues.rudder.io/issues/3967?journal_id=207812013-10-01T09:45:24ZVincent MEMBRÉvme@rudder.io
<ul></ul><p>I added a commit to trim directive parameters(if an \r was inserted it was trimmed in LDAP but not in psql...)</p>
<p>François, if it's ok, don't merge so i can rebase it.</p> Rudder - Bug #3967: Change request cannot be accepted: multiline text cause merge incompatibilityhttps://issues.rudder.io/issues/3967?journal_id=208842013-10-02T08:28:48ZFrançois ARMANDfrancois.armand@rudder.io
<ul><li><strong>Status</strong> changed from <i>Pending technical review</i> to <i>Discussion</i></li><li><strong>Assignee</strong> changed from <i>François ARMAND</i> to <i>Vincent MEMBRÉ</i></li></ul><p>Some comments.</p>
<p>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.</p>
<p>All other parts seem good.</p> Rudder - Bug #3967: Change request cannot be accepted: multiline text cause merge incompatibilityhttps://issues.rudder.io/issues/3967?journal_id=209012013-10-02T13:12:10ZVincent MEMBRÉvme@rudder.io
<ul><li><strong>Assignee</strong> changed from <i>Vincent MEMBRÉ</i> to <i>François ARMAND</i></li></ul><p>I updated the PR.</p> Rudder - Bug #3967: Change request cannot be accepted: multiline text cause merge incompatibilityhttps://issues.rudder.io/issues/3967?journal_id=209242013-10-02T14:17:29ZVincent MEMBRÉvme@rudder.io
<ul><li><strong>Status</strong> changed from <i>Discussion</i> to <i>Pending technical review</i></li></ul> Rudder - Bug #3967: Change request cannot be accepted: multiline text cause merge incompatibilityhttps://issues.rudder.io/issues/3967?journal_id=209552013-10-02T15:01:55ZVincent MEMBRÉvme@rudder.io
<ul><li><strong>Status</strong> changed from <i>Pending technical review</i> to <i>Pending release</i></li><li><strong>% Done</strong> changed from <i>0</i> to <i>100</i></li></ul><p>Applied in changeset <a class="changeset" title="Fixes #3967 : Improve change request merge check: Ldap datas are transformed in XML, then checke..." href="https://issues.rudder.io/projects/rudder/repository/rudder/revisions/676362d8b559aef5a314cc8ae74e49d6d1f18b09">676362d8b559aef5a314cc8ae74e49d6d1f18b09</a>.</p> Rudder - Bug #3967: Change request cannot be accepted: multiline text cause merge incompatibilityhttps://issues.rudder.io/issues/3967?journal_id=209562013-10-02T15:01:55ZAnonymous
<ul></ul><p>Applied in changeset <a class="changeset" title="Merge pull request #326 from VinceMacBuche/bug_3967/serialize_deserialize_ldap_into_xmll Fixes #..." href="https://issues.rudder.io/projects/rudder/repository/rudder/revisions/f8a6ed0efeea418848f39e1382d04a4f1a891e87">f8a6ed0efeea418848f39e1382d04a4f1a891e87</a>.</p> Rudder - Bug #3967: Change request cannot be accepted: multiline text cause merge incompatibilityhttps://issues.rudder.io/issues/3967?journal_id=211112013-10-03T17:07:48ZNicolas PERRONnicolas.perron@normation.com
<ul><li><strong>Status</strong> changed from <i>Pending release</i> to <i>Released</i></li></ul> Rudder - Bug #3967: Change request cannot be accepted: multiline text cause merge incompatibilityhttps://issues.rudder.io/issues/3967?journal_id=742372017-03-30T09:29:55ZFrançois ARMANDfrancois.armand@rudder.io
<ul><li><strong>Related to</strong> <i><a class="issue tracker-4 status-1 priority-16 priority-default" href="/issues/3972">Architecture #3972</a>: Scala XML library accepts invalid XML characters</i> added</li></ul> Rudder - Bug #3967: Change request cannot be accepted: multiline text cause merge incompatibilityhttps://issues.rudder.io/issues/3967?journal_id=1636332022-12-22T14:25:32ZFrançois ARMANDfrancois.armand@rudder.io
<ul><li><strong>Related to</strong> <i><a class="issue tracker-1 status-16 priority-16 priority-default closed" href="/issues/22208">Bug #22208</a>: Directives cannot be deleted because change request say state diverged even when there are no change request activated</i> added</li></ul>