[JENKINS-71182] Correct Unicode behavior of XML_1_1
#7924
Conversation
|
(Oh, and I tried to use |
|
I filed x-stream/xstream#337 to fix XStream. Do you think it is worth temporarily inlining that patch into Jenkins while waiting for upstream to merge/release it, or do you think it is preferable to revert back to quirks mode? |
|
I'm not sure if this is of any help here, but I have seen filtered streams/readers in other real world applications just stripping offending characters from the input stream before passing it to the XML handling (e.g. the NUL character that started these changes). Examples of software I have actually been using: |
NotMyFault
added
the
regression-fix
Seems simplest to me to just use quirks mode until a fix makes it into a release. The fail-fast behavior is nice to have but hardly essential, assuming the
Yes, and/or conversely rejecting |
|
(Whatever the approach, let us get some fix into the next weekly.) |
|
My preference would be to inline the XStream fix, since that would provide additional real-world testing which would increase the likelihood that the fix would be accepted upstream. |
|
OK, I should be able to do that today. |
| } | ||
|
|
||
| private void writeText(final String text, final boolean isAttribute) { | ||
| text.codePoints().forEach(c -> { |
There was a problem hiding this comment.
This and matching lines are the fix from the upstream PR.
| public static int XML_1_1 = 1; | ||
|
|
||
| private final QuickWriter writer; | ||
| private final FastStack elementStack = new FastStack(16); |
There was a problem hiding this comment.
had to drop type parameter
| } | ||
|
|
||
| @Override | ||
| public void startNode(final String name, final Class clazz) { |
| * Created on 07. March 2004 by Joe Walnes | ||
| */ | ||
|
|
||
| package hudson.util; |
There was a problem hiding this comment.
edited from original, obviously
| @@ -0,0 +1,340 @@ | |||
| // TODO adapted from https://github.com/x-stream/xstream/blob/32e52a6519a25366bbb5774bb536b5e290b64a42/xstream/src/java/com/thoughtworks/xstream/io/xml/PrettyPrintWriter.java pending release of https://github.com/jenkinsci/jenkins/pull/7924 | |||
There was a problem hiding this comment.
Reformatting for Checkstyle, plus a couple minor changes noted inline.
jglick
changed the title
XML_QUIRKSXML_1_1
| * @author Joe Walnes | ||
| * @author Jörg Schaible | ||
| */ | ||
| class PrettyPrintWriter extends AbstractXmlWriter { |
There was a problem hiding this comment.
dropped public on class and constructors
|
This PR is now ready for merge. We will merge it after approximately 24 hours if there is no negative feedback. Please see the merge process documentation for more information about the merge process. Thanks! |
basil
added
the
ready-for-merge
(cherry picked from commit 8d9e8b9)
See JENKINS-71182.
There does not seem to be any good solution, until#7875 (comment)PrettyPrintWritersupports Unicode properly.Testing done
See test cases. (Writing emojis was apparently not tested before, only reading.)
Proposed changelog entries
Proposed upgrade guidelines
N/A
Maintainer checklist
Before the changes are marked as
ready-for-merge:upgrade-guide-neededlabel is set and there is a Proposed upgrade guidelines section in the pull request title (see example).lts-candidateto be considered (see query).