Bug 42061 - Method to disable XMLUtils.addReturnToElement
Method to disable XMLUtils.addReturnToElement
Status: RESOLVED FIXED
Product: Security - Now in JIRA
Classification: Unclassified
Component: Signature
unspecified
All All
: P2 enhancement
: ---
Assigned To: XML Security Developers Mailing List
:
Depends on:
Blocks:
  Show dependency tree
 
Reported: 2007-04-05 09:55 UTC by Michael McIntosh
Modified: 2009-05-22 05:24 UTC (History)
0 users



Attachments
A patch to allow programatic disabling of line breaks (1.88 KB, patch)
2009-01-23 06:03 UTC, coheigea
Details | Diff
A new patch for this issue (517 bytes, patch)
2009-05-19 08:50 UTC, coheigea
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael McIntosh 2007-04-05 09:55:20 UTC
Some Signature consumers are not able to accept Signatures containing 
whitespace in certain places even though the standard allows for it.

An example of a solution would be:

public static void addReturnToElement(Element e) {
  if (null == System.getProperty
("org.apache.xml.security.util.XMLUtils.ignoreAddReturnToElement")) {
    Document doc = e.getOwnerDocument();
    e.appendChild(doc.createTextNode("\n"));
  }
}
Comment 1 sean.mullan 2007-10-04 07:11:26 UTC
Fixed. If you set the system property named
"org.apache.xml.security.ignoreLineBreaks" to true, line breaks will not be
inserted in the Signature. This includes line breaks after elements and in
base64 encoded values.
Comment 2 coheigea 2009-01-23 06:03:39 UTC
Created attachment 23167 [details]
A patch to allow programatic disabling of line breaks


This is a patch to add a programatic means of disabling line breaks, rather than just polling a system property statically. I also disabled some printlns that were introduced in 1.
Comment 3 sean.mullan 2009-01-28 07:42:09 UTC
Thanks for the patch, but I was thinking that you were going to add a method that would enable this feature on a per-signature basis. Since it is a static method, it affects all applications which is not ideal. I would suggest adding a non-static method to the XMLSignature class.
Comment 4 coheigea 2009-04-10 04:27:34 UTC
Hi Sean,

Sorry for the delay in replying to this.

"Thanks for the patch, but I was thinking that you were going to add a method
that would enable this feature on a per-signature basis. Since it is a static
method, it affects all applications which is not ideal. I would suggest adding
a non-static method to the XMLSignature class."

The problem is that the existing method of disabling line breaks is static, as it polls the system property statically. The existing method *already* affects all applications, my patch was just a means of providing programmatic access to changing this value. In my particular case I'm fine with disabling it for all applications as I don't want any new lines inserted in any of the DOM created by XML-Security, e.g. not just ds:Signature but also ds:KeyInfo elements.

My point is that enforcing whether to insert new-lines or not by polling a system property once is unnecessarily restrictive, as it doesn't give the programmer any control over this. 

I guess ideally there should be no static variable which controls this for all applications, and it should be done by non-static methods on each of the classes. 

Thanks,

Colm.
Comment 5 sean.mullan 2009-04-16 12:30:11 UTC
(In reply to comment #4)

> My point is that enforcing whether to insert new-lines or not by polling a
> system property once is unnecessarily restrictive, as it doesn't give the
> programmer any control over this. 

Agree. Personally, I'm not a big fan of system properties. They have their place of course, and are good for workarounds where the API can't be changed, or you can't or don't want to change your application. In hindsight I think I should have initially added an API to control this behavior on a per signature basis. I think adding a static method to toggle this behavior could cause more harm than good. 

> I guess ideally there should be no static variable which controls this for all
> applications, and it should be done by non-static methods on each of the
> classes. 

So what I'll do is leave this open to add a method on a per Signature basis, as that seems easy enough to add for the next release.
Comment 6 coheigea 2009-05-19 08:50:24 UTC
Created attachment 23692 [details]
A new patch for this issue

Hi Sean,

I had a look at disabling line breaks on a per-signature basis. It's more tricky than it appears, as there are lots of calls to insert line breaks in various constructors, and you quickly end up with a lot of work. 

Please see attached for a revised (and simple!) patch. It just fixes the case in Base64 for automatically inserting line breaks when the encoded length was greater than 76. So even when line breaks were disabled via the System property, xml-sec was still inserting line-breaks e.g. in <ds:Modulus>...</ds:Modulus>.

I'm happy just to leave things as they are, and to close this JIRA once the new patch is applied.

Colm.
Comment 7 sean.mullan 2009-05-22 05:24:12 UTC
Patch looks good. I have integrated it. Thanks!