Skip to content
This repository has been archived by the owner on Feb 5, 2019. It is now read-only.

In unmarshaller, make errorLimitCounter customizable. #1154

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@

package com.sun.xml.bind.v2.runtime.unmarshaller;

import static com.sun.xml.bind.v2.runtime.unmarshaller.UnmarshallerProperties.ENABLE_ERROR_REPORT_LIMIT;
import static com.sun.xml.bind.v2.runtime.unmarshaller.UnmarshallerProperties.ERROR_REPORT_LIMIT;

import java.io.IOException;
import java.io.InputStream;

Expand Down Expand Up @@ -511,6 +514,14 @@ public void setProperty(String name, Object value) throws PropertyException {
coordinator.classLoader = (ClassLoader)value;
return;
}
if(name.equals(ENABLE_ERROR_REPORT_LIMIT)) {
coordinator.setLimitErrorReporting((boolean) value);
return ;
}
if(name.equals(ERROR_REPORT_LIMIT)) {
coordinator.setErrorsCounter((int)value);
return ;
}
super.setProperty(name, value);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package com.sun.xml.bind.v2.runtime.unmarshaller;

public class UnmarshallerProperties {
public static final String ENABLE_ERROR_REPORT_LIMIT = "unmarshaller.enableErrorReportLimit";
public static final String ERROR_REPORT_LIMIT = "unmarshaller.errorReportLimit";
}
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@
import com.sun.istack.Nullable;
import com.sun.istack.SAXParseException2;
import com.sun.xml.bind.IDResolver;
import com.sun.xml.bind.Util;
import com.sun.xml.bind.api.AccessorException;
import com.sun.xml.bind.api.ClassResolver;
import com.sun.xml.bind.unmarshaller.InfosetScanner;
Expand All @@ -77,8 +76,6 @@
import com.sun.xml.bind.v2.runtime.Coordinator;
import com.sun.xml.bind.v2.runtime.JAXBContextImpl;
import com.sun.xml.bind.v2.runtime.JaxBeanInfo;
import java.util.logging.Level;
import java.util.logging.Logger;

import org.xml.sax.ErrorHandler;
import org.xml.sax.Locator;
Expand Down Expand Up @@ -199,13 +196,24 @@ public final class UnmarshallingContext extends Coordinator
*/
public @Nullable ClassLoader classLoader;

public static volatile int DEFAULT_ERROR_COUNTER = 10;

/**
* The variable introduced to avoid reporting n^10 similar errors.
* After error is reported counter is decremented. When it became 0 - errors should not be reported any more.
* The variable introduced to avoid reporting more than {@value DEFAULT_ERROR_COUNTER} errors.
* The default value can be overrode setting the {@link UnmarshallerProperties#ERROR_REPORT_LIMIT} property in the {@link Unmarshaller}.
* After error is reported counter is decremented. When it became lower than 0, errors should not be reported any more.
* If the {@link UnmarshallingContext#limitErrorReporting} is set to false, this field will be totally ignored.
*
* volatile is required to ensure that concurrent threads will see changed value
*/
private static volatile int errorsCounter = 10;
private volatile int errorsCounter = DEFAULT_ERROR_COUNTER;

/**
* Indicates if it should exist an error reporting limit. Default value is true.
* This property can be overrode setting the setting the {@link UnmarshallerProperties#ENABLE_ERROR_REPORT_LIMIT} property in the {@link Unmarshaller}.
*
*/
private volatile boolean limitErrorReporting = true;

/**
* State information for each element.
Expand Down Expand Up @@ -1336,28 +1344,36 @@ public StructureLoader getStructureLoader() {
return null;
}

public void setErrorsCounter(int errorsCounter) {
this.errorsCounter = errorsCounter;
}

public void setLimitErrorReporting(boolean limitErrorReporting) {
this.limitErrorReporting = limitErrorReporting;
}

/**
* Based on current {@link Logger} {@link Level} and errorCounter value determines if error should be reported.
* Based on current {@link UnmarshallingContext#limitErrorReporting} and errorCounter value determines if error should be reported.
*
* If the method called and return true it is expected that error will be reported. And that's why
* errorCounter is automatically decremented during the check.
*
* NOT THREAD SAFE!!! In case of heave concurrency access several additional errors could be reported. It's not expected to be the
* problem. Otherwise add synchronization here.
*
* @return true in case if {@link Level#FINEST} is set OR we haven't exceed errors reporting limit.
* @return true in case if {@link UnmarshallingContext#limitErrorReporting} OR we haven't exceed errors reporting limit.
*/
public boolean shouldErrorBeReported() throws SAXException {
if (logger.isLoggable(Level.FINEST))
if (!limitErrorReporting)
return true;

if (errorsCounter >= 0) {
--errorsCounter;
if (errorsCounter == 0) // it's possible to miss this because of concurrency. If required add synchronization here
if (errorsCounter == -1) // it's possible to miss this because of concurrency. If required add synchronization here
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Due to comment on concurrency should be errorsCounter < 0 instead of ==

handleEvent(new ValidationEventImpl(ValidationEvent.WARNING, Messages.ERRORS_LIMIT_EXCEEDED.format(),
getLocator().getLocation(), null), true);
}
return errorsCounter >= 0;
return false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't work, if we always return false on shouldErrorBeReported(), error will never appear in the log.
Should be errorsCounter >= 0

}
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package com.sun.xml.bind.utils;

import javax.xml.bind.annotation.XmlRootElement;

@XmlRootElement
public class Person {

private String value;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
package com.sun.xml.bind.v2.runtime.unmarshaller;

import static com.sun.xml.bind.v2.runtime.unmarshaller.Messages.ERRORS_LIMIT_EXCEEDED;
import static com.sun.xml.bind.v2.runtime.unmarshaller.UnmarshallerProperties.ENABLE_ERROR_REPORT_LIMIT;
import static com.sun.xml.bind.v2.runtime.unmarshaller.UnmarshallingContext.DEFAULT_ERROR_COUNTER;
import static org.hamcrest.CoreMatchers.is;
import static org.junit.Assert.assertThat;

import com.sun.xml.bind.utils.Person;

import java.io.ByteArrayInputStream;

import javax.xml.bind.JAXBContext;
import javax.xml.bind.JAXBException;
import javax.xml.bind.Unmarshaller;
import javax.xml.bind.ValidationEvent;
import javax.xml.bind.util.ValidationEventCollector;

import junit.framework.TestCase;


public class LimitErrorTest extends TestCase {

private static final String EXPECTED_ERROR_MESSAGE = "unexpected element (uri:\"\", local:\"unexpectedChild\"). Expected elements are (none)";
private static final String GENERIC_ERROR_MESSAGE = ERRORS_LIMIT_EXCEEDED.format();
private final ValidationEventCollector validationCollector = new JAXB2ValidationEventCollector();
private static final String xml = "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"yes\"?>" +
"<person>" +
"<unexpectedChild></unexpectedChild>" +
"</person>";

private Unmarshaller unmarshaller = null;

protected void setUp() throws Exception {
unmarshaller = JAXBContext.newInstance(Person.class).createUnmarshaller();
unmarshaller.setEventHandler(validationCollector);
}

public void testDisableErrorReportLimit() throws Exception
{
unmarshaller.setProperty(ENABLE_ERROR_REPORT_LIMIT, false);

for (int i = 0; i < DEFAULT_ERROR_COUNTER + 1; i ++) {
try {
unmarshaller.unmarshal(new ByteArrayInputStream(xml.getBytes()));
}
catch (JAXBException e) {
assertThat(e.getMessage(), is(EXPECTED_ERROR_MESSAGE));
}
}
}

public void testEnableErrorReportLimit() {

for (int i = 0; i < DEFAULT_ERROR_COUNTER; i ++) {
try {
unmarshaller.unmarshal(new ByteArrayInputStream(xml.getBytes()));
}
catch (JAXBException e) {
assertThat(e.getMessage(), is(EXPECTED_ERROR_MESSAGE));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Due to return false; error in shouldBeErrorReported() this assertion is never called in any of test methods.

}
}

try {
unmarshaller.unmarshal(new ByteArrayInputStream(xml.getBytes()));
}
catch (JAXBException e) {
assertThat(e.getMessage(), is(GENERIC_ERROR_MESSAGE));
}
}

public void testCustomErrorReportLimit() {
int customErrorReportLimit = 5;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

custom error limit is never passed to context here, so test will fail after fixing shouldBeErrorReported()
please add unmarshaller.setProperty(UnmarshallerProperties.ERROR_REPORT_LIMIT, customErrorReportLimit); below int customErrorReportLimit = 5;

for (int i = 0; i < customErrorReportLimit; i ++) {
try {
unmarshaller.unmarshal(new ByteArrayInputStream(xml.getBytes()));
}
catch (JAXBException e) {
assertThat(e.getMessage(), is(EXPECTED_ERROR_MESSAGE));
}
}

try {
unmarshaller.unmarshal(new ByteArrayInputStream(xml.getBytes()));
}
catch (JAXBException e) {
assertThat(e.getMessage(), is(GENERIC_ERROR_MESSAGE));
}
}


private class JAXB2ValidationEventCollector extends ValidationEventCollector {
@Override
public boolean handleEvent(ValidationEvent event) {
super.handleEvent(event);
return false;
}
}

}