Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Workaround for false negative junit test failures due to corrupted XML result files: 'Content is not allowed in trailing section.' #67

Closed
wants to merge 16 commits into from

Conversation

sgothel
Copy link

@sgothel sgothel commented Feb 28, 2011

Workaround for false negative junit test failures due to corrupted XML result files: 'Content is not allowed in trailing section.'

We have observed a lot of the following node failures:

https://jogamp.org/chuck/job/gluegen/294/label=linux-x86_32-nvidia/console
+++

Caused by: org.xml.sax.SAXParseException: Content is not allowed in trailing section.
at com.sun.org.apache.xerces.internal.util.ErrorHandlerWrapper.createSAXParseException(ErrorHandlerWrapper.java:195)
at com.sun.org.apache.xerces.internal.util.ErrorHandlerWrapper.fatalError(ErrorHandlerWrapper.java:174)
at com.sun.org.apache.xerces.internal.impl.XMLErrorReporter.reportError(XMLErrorReporter.java:388)

+++

The XML result was corrupted on the remote node (ant, junit, .. ?)
and has a trailing '>':

+++

+++

We were not yet able to identify the culprit and have observed that this bug
is visible for around 3 years ? (hudson mailinglist)

This patch introduces an XML ErrorHandler, which ignores the SAXException:

XERCES - FatalError - Content is not allowed in trailing section

A private build is alive at https://jogamp.org/chuck/

Olav Reinert and others added 16 commits February 22, 2011 20:05
…L result files: 'Content is not allowed in trailing section.'

We have observed a lot of the following node failures:

https://jogamp.org/chuck/job/gluegen/294/label=linux-x86_32-nvidia/console
+++

Caused by: org.xml.sax.SAXParseException: Content is not allowed in trailing section.
    at com.sun.org.apache.xerces.internal.util.ErrorHandlerWrapper.createSAXParseException(ErrorHandlerWrapper.java:195)
    at com.sun.org.apache.xerces.internal.util.ErrorHandlerWrapper.fatalError(ErrorHandlerWrapper.java:174)
    at com.sun.org.apache.xerces.internal.impl.XMLErrorReporter.reportError(XMLErrorReporter.java:388)

+++

The XML result was corrupted on the remote node (ant, junit, .. ?)
and has a trailing '>':

+++

</testsuite>
>

+++

We were not yet able to identify the culprit and have observed that this bug
is visible for around 3 years ? (hudson mailinglist)

This patch introduces an XML ErrorHandler, which ignores the SAXException:

    XERCES - FatalError - Content is not allowed in trailing section

A private build is alive at https://jogamp.org/chuck/
@kohsuke
Copy link
Member

kohsuke commented Mar 15, 2011

My apologies for the delay in getting back to you.

I'd like to discuss this a bit more --- it seems like a ticket is in order in http://issues.jenkins-ci.org/

I feel that this is a wrong way to fix the problem. If someone is producing a broken XML report file, a fix should be made there, not in Jenkins. It is not a false negative junit test failures --- the report file is indeed quite broken.

Or if you are suspecting that the report files are corrupted when they are copied to the master, again that's a serious problem that should be fixed on its own, instead of working it around like this.

If you feel strongly about this fix, what I can do is to make the code change so that you can do this in your plugin.

@kohsuke
Copy link
Member

kohsuke commented Mar 30, 2011

Since I'm not hearing back, I'm tentatively closing this.

If the error message wasn't pointing to the problem, that's definitely worth fixing on its own. I also wonder if this is a bug in the remoting code that we've fixed in 1.404.

@kohsuke kohsuke closed this Mar 30, 2011
@sgothel
Copy link
Author

sgothel commented Mar 30, 2011

On Tuesday, March 15, 2011 05:18:27 am kohsuke wrote:

My apologies for the delay in getting back to you.

I'd like to discuss this a bit more --- it seems like a ticket is in order in http://issues.jenkins-ci.org/

I feel that this is a wrong way to fix the problem. If someone is producing a broken XML report file, a fix should be made there, not in Jenkins. It is not a false negative junit test failures --- the report file is indeed quite broken.

Yes, the report files got corrupted on the slave nodes.
Hence it's not transport or Jenkins server at all.
(The trailing '>' at the end of the junit xml files)

Somehow this must have been produced by the combination of:

  • java
  • jenkins-slave
  • apache-ant (1.8.0 - 1.8.3)
  • junit 4.8.2
  • test cases ?

Sadly, we were not able to find this bug
and it seems we are not alone with this issue .. :(

The test itself run fine, all verified of course.
Hence my statement: false negative in respect to the actual tests, ignoring the toolchain.

Long story short, to enable us using Jenkins again
and not having all the many false negatives - I made this patch, obviously.

Or if you are suspecting that the report files are corrupted when they are copied to the master, again that's a serious problem that should be fixed on its own, instead of working it around like this.

If you feel strongly about this fix, what I can do is to make the code change so that you can do this in your plugin.

That would be awesome !

Maybe .. you can add an option for this behavior
which can be removed if it is fixed by the tool chain (see above).
Such option would also allow us to test the bug.
However, this change dumps a detection on the client side as well.

On Wednesday, March 30, 2011 06:54:37 am kohsuke wrote:

Since I'm not hearing back, I'm tentatively closing this.

If the error message wasn't pointing to the problem, that's definitely worth fixing on its own. I also wonder if this is a bug in the remoting code that we've fixed in 1.404.

I will update Jenkins soon on https://jogamp.org/chuck/.

In the end I would say, this patch won't hurt.
On the other end .. I understand the notion of not having corner cases included, sure.

How about I run your latest code version w/o the patch,
and if it breaks again - you use the patch ?

Cheers, Sven

@sgothel sgothel reopened this Mar 30, 2011
@kohsuke
Copy link
Member

kohsuke commented Jun 5, 2011

Implemented a new extension point in commit bdf13dc to address this.

@kohsuke kohsuke closed this Jun 5, 2011
kohsuke pushed a commit that referenced this pull request May 11, 2016
[FIX SECURITY-276] Don't allow open redirect using scheme-rel. URL
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants