Bug 1139667
| Summary: | Special characters in remote operation parameters causes various issues | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Retired] JBoss BPMS Platform 6 | Reporter: | Ivo Bek <ibek> | ||||
| Component: | Business Central | Assignee: | Shelly McGowan <smcgowan> | ||||
| Status: | CLOSED EOL | QA Contact: | Ivo Bek <ibek> | ||||
| Severity: | medium | Docs Contact: | |||||
| Priority: | high | ||||||
| Version: | 6.1.0 | CC: | ibek, kverlaen | ||||
| Target Milestone: | ER6 | ||||||
| Target Release: | 6.1.0 | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Whiteboard: | |||||||
| Fixed In Version: | Doc Type: | Bug Fix | |||||
| Doc Text: | Story Points: | --- | |||||
| Clone Of: | Environment: | ||||||
| Last Closed: | 2020-03-27 19:43:48 UTC | Type: | Bug | ||||
| Regression: | --- | Mount Type: | --- | ||||
| Documentation: | --- | CRM: | |||||
| Verified Versions: | Category: | --- | |||||
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||
| Cloudforms Team: | --- | Target Upstream Version: | |||||
| Embargoed: | |||||||
| Attachments: |
|
||||||
|
Description
Ivo Bek
2014-09-09 11:53:52 UTC
Created attachment 935651 [details]
server log
This note is here for others to read: This is a relatively expensive feature to support. In short, this is because we can not predict what inputs a user will use. For example, we could try to simply encode *only* the characters, such as '&' for JAXB, that cause problems. However, how do we encode that? We can not simply substitute "XAMPX" for '&' in the input because "XAMPX" might also be in the input! When we decode the string on the server side, how do we know which "XAMPX" should be changed back into '&' and which into "XAMPX"? The only solution here is thus to uniformly encode all input, regardless of whether or not it contains special characters: however, encoded strings are at least 2 times and sometimes up to 4 times as long, not to mention that there's a performance penalty for encoding and decoding the string. As a result, the fix here is optional and by default turned off. See other comments below for more informatoin. Fixed. Commits: 6.2.x: https://github.com/droolsjbpm/drools/commit/a7b0cdd7 https://github.com/droolsjbpm/droolsjbpm-integration/commit/c32a1c34 https://github.com/droolsjbpm/droolsjbpm-integration/commit/4a1a27a8 https://github.com/droolsjbpm/droolsjbpm-integration/commit/7f7f604d master: https://github.com/droolsjbpm/drools/commit/b1faf912 https://github.com/droolsjbpm/droolsjbpm-integration/commit/c5a2bb56 https://github.com/droolsjbpm/droolsjbpm-integration/commit/9500f2a3 https://github.com/droolsjbpm/droolsjbpm-integration/commit/b4f6ac36 The "org.kie.xml.encode" property must be set to "true" on *both* the client and server side for the changes in the code to be active. (See previous comment for explanation as to why the fix is not active by default). See: https://github.com/droolsjbpm/droolsjbpm-integration/blob/6.2.x/kie-remote/kie-remote-jaxb-gen/src/main/java/org/kie/remote/jaxb/gen/util/JaxbUnknownAdapter.java#L32 https://github.com/droolsjbpm/drools/blob/master/drools-core/src/main/java/org/drools/core/xml/jaxb/util/JaxbUnknownAdapter.java#L39 Lowering severity, since there is a workaround to use already encoded strings like: "String with ampersand &." ""quoted string"" from the examples in the BZ description. Ivo, In addition to &, >, <, ' and ", there are also all of the characters outside of the set that the XML standard mandates: http://www.w3.org/TR/xml/#charsets My main concern is that I want a solution that I don't ever have to worry about again, as opposed to a whitelist solution. Adding a "whitelist" of characters that should be encoded is, in my opinion, a piece of code that will come back to poke me a couple times. Maybe there is (or will be) an additional character out there that a client will use -- or maybe there are or will be characters in languages like Chinese or Arabic that fall outside of the XML specification: if that happens, I don't want to have to reengineeer this code. In other words, this is a small problem, I don't want to write a solution for it that I or other engineers might have to care for later, especially if there's a solution that takes care of it that requires less maintenance. Does that make sense to you? Are there points that I've gotten wrong or have otherwise missed? Sure there might be also other characters and I did not mean creating a whitelist and replacing the characters manually. I was trying Arabic and Chineese alphabet and these are working well. I propose to use https://commons.apache.org/proper/commons-lang/javadocs/api-3.1/org/apache/commons/lang3/StringEscapeUtils.html#escapeXml%28java.lang.String%29 for escaping the xml content. I believe that it should cover all the most common characters. I share your fear but I would say that the Appache Commons library will solve it good enough. WDYT? Fixed. Commits: 6.2.x: https://github.com/droolsjbpm/drools/commit/4a387aa2 (revert of previous commit) https://github.com/droolsjbpm/droolsjbpm-integration/commit/2d6c65eb master: https://github.com/droolsjbpm/drools/commit/e14aa6e1 (revert of previous commit) https://github.com/droolsjbpm/droolsjbpm-integration/commit/72e9ecc1 Verified in BPMS 6.1.0.ER6 though I mentioned some concerns in Comment 11. Ivo, Sorry for the late answer. No problem and good point: my original thoughts were that using the library would be too much of a performance cost. Also, the changes I added already fit into the existing code framework where using the StringEscapeUtils would mean that I would have to refactor some code. Later, I realized that the network would always cost more than this. If you want, feel free to open a bug concerning this for 6.2? What do you think? |