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

HHH-18455 Strict XML validation compliance #9558

Open
wants to merge 1 commit into
base: main
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 @@ -21,6 +21,7 @@
import org.hibernate.resource.jdbc.spi.PhysicalConnectionHandlingMode;
import org.hibernate.resource.jdbc.spi.StatementInspector;
import org.hibernate.type.format.FormatMapper;
import org.hibernate.boot.xsd.XmlValidationMode;

import jakarta.persistence.criteria.Nulls;

Expand Down Expand Up @@ -746,6 +747,18 @@ public interface SessionFactoryBuilder {
@Incubating
SessionFactoryBuilder applyXmlFormatMapper(FormatMapper xmlFormatMapper);

/**
* Specifies a {@link XmlValidationMode validation mode} to use for validation of XML files.
*
* @param xmlValidationMode The {@link XmlValidationMode} to use.
*
* @return {@code this}, for method chaining
*
* @see org.hibernate.cfg.AvailableSettings#XML_VALIDATION_MODE
*/
@Incubating
SessionFactoryBuilder applyXmlValidationMode(XmlValidationMode xmlValidationMode);

/**
* After all options have been set, build the SessionFactory.
*
Expand Down
Copy link
Member Author

Choose a reason for hiding this comment

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

Perhaps InputstreamRepeatableAccess would be a better name for this ...
I also wonder about what to do (if anything) with that chunk of memory the byte[] occupies, once this repeatable access is no longer needed; should there be some mechanism to release this?

Copy link
Member

Choose a reason for hiding this comment

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

Hey @jrenaat 👋🏻 😃

I remembered that we have something for XML validation in Validator code here: https://github.com/hibernate/hibernate-validator/blob/b14c620a3f7d93a322fa122b52e1b9e83c376c35/engine/src/main/java/org/hibernate/validator/internal/xml/mapping/MappingXmlParser.java#L98-L143

you'll see that that code relies on InputStream#mark()/InputStream#reset() instead of keeping all the file. And the way we make sure that these methods are "supported" by the stream is here:
https://github.com/hibernate/hibernate-validator/blob/b14c620a3f7d93a322fa122b52e1b9e83c376c35/engine/src/main/java/org/hibernate/validator/internal/engine/AbstractConfigurationImpl.java#L287

IDK if that would be applicable here or not, but I thought I'd let you know about it, and you can then decide 😃.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the pointers. I did consider the mark/reset approach but i'm not 100% it would work in our case, i think we actually need 2 separate inputstreams.

Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*
* SPDX-License-Identifier: LGPL-2.1-or-later
* Copyright Red Hat Inc. and Hibernate Authors
*/
package org.hibernate.boot.archive.internal;


import org.hibernate.HibernateException;
import org.hibernate.boot.archive.spi.InputStreamAccess;

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

/**
* @author Jan Schatteman
*/
public class RepeatableInputStreamAccess implements InputStreamAccess {

private final String resourceName;
private byte[] bytes = new byte[0];

public RepeatableInputStreamAccess(String resourceName, InputStream inputStream) {
this.resourceName = resourceName;
try {
bytes = inputStream.readAllBytes();
}
catch (IOException | OutOfMemoryError e) {
throw new HibernateException( "Could not read resource " + resourceName, e );
}
}

@Override
public String getStreamName() {
return resourceName;
}

@Override
public InputStream accessInputStream() {
return new ByteArrayInputStream( bytes );
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.hibernate.boot.spi.MetadataImplementor;
import org.hibernate.boot.spi.SessionFactoryBuilderImplementor;
import org.hibernate.boot.spi.SessionFactoryOptions;
import org.hibernate.boot.xsd.XmlValidationMode;
import org.hibernate.bytecode.internal.SessionFactoryObserverForBytecodeEnhancer;
import org.hibernate.bytecode.spi.BytecodeProvider;
import org.hibernate.cache.spi.TimestampsCacheFactory;
Expand Down Expand Up @@ -431,6 +432,12 @@ public void disableJtaTransactionAccess() {
this.optionsBuilder.disableJtaTransactionAccess();
}

@Override
public SessionFactoryBuilder applyXmlValidationMode(XmlValidationMode xmlValidationMode) {
this.optionsBuilder.applyXmlValidationMode( xmlValidationMode );
return this;
}

@Override
public SessionFactory build() {
return new SessionFactoryImpl( metadata, buildSessionFactoryOptions(), bootstrapContext );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import org.hibernate.boot.spi.BootstrapContext;
import org.hibernate.boot.spi.MetadataBuildingContext;
import org.hibernate.boot.spi.SessionFactoryOptions;
import org.hibernate.boot.xsd.XmlValidationMode;
import org.hibernate.cache.internal.NoCachingRegionFactory;
import org.hibernate.cache.internal.StandardTimestampsCacheFactory;
import org.hibernate.cache.spi.RegionFactory;
Expand Down Expand Up @@ -253,6 +254,7 @@ public class SessionFactoryOptionsBuilder implements SessionFactoryOptions {

private final int queryStatisticsMaxSize;

private XmlValidationMode xmlValidationMode;
private final Map<String, Object> defaultSessionProperties;
private final CacheStoreMode defaultCacheStoreMode;
private final CacheRetrieveMode defaultCacheRetrieveMode;
Expand Down Expand Up @@ -534,6 +536,8 @@ public SessionFactoryOptionsBuilder(StandardServiceRegistry serviceRegistry, Boo

defaultLockOptions = defaultLockOptions( defaultSessionProperties );
initialSessionFlushMode = defaultFlushMode( defaultSessionProperties );

xmlValidationMode = ConfigurationHelper.resolveXmlValidationMode( settings );
}

private TimeZone getJdbcTimeZone(Object jdbcTimeZoneValue) {
Expand Down Expand Up @@ -1279,6 +1283,9 @@ public boolean isPreferJdbcDatetimeTypesInNativeQueriesEnabled() {
return preferJdbcDatetimeTypes;
}

@Override
public XmlValidationMode getXmlValidationMode() { return xmlValidationMode; }

// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
// In-flight mutation access

Expand Down Expand Up @@ -1553,7 +1560,6 @@ public void enableGeneratorNameScopeCompliance(boolean enabled) {
mutableJpaCompliance().setGeneratorNameScopeCompliance( enabled );
}


public void enableCollectionInDefaultFetchGroup(boolean enabled) {
this.collectionsInDefaultFetchGroupEnabled = enabled;
}
Expand All @@ -1562,6 +1568,10 @@ public void disableJtaTransactionAccess() {
this.jtaTransactionAccessEnabled = false;
}

public void applyXmlValidationMode(XmlValidationMode xmlValidationMode) {
this.xmlValidationMode = xmlValidationMode;
}

public SessionFactoryOptions buildOptions() {
if ( jpaCompliance instanceof MutableJpaCompliance ) {
jpaCompliance = mutableJpaCompliance().immutableCopy();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,23 @@
package org.hibernate.boot.jaxb.internal;

import java.io.InputStream;
import java.util.function.Consumer;
import javax.xml.stream.XMLEventReader;
import javax.xml.stream.XMLInputFactory;
import javax.xml.stream.XMLStreamException;
import javax.xml.stream.events.StartElement;
import javax.xml.stream.events.XMLEvent;
import javax.xml.transform.Source;
import javax.xml.validation.Schema;

import org.hibernate.boot.MappingException;
import org.hibernate.boot.ResourceStreamLocator;
import org.hibernate.boot.archive.spi.InputStreamAccess;
import org.hibernate.boot.jaxb.Origin;
import org.hibernate.boot.jaxb.internal.stax.BufferedXMLEventReader;
import org.hibernate.boot.jaxb.internal.stax.LocalXmlResourceResolver;
import org.hibernate.boot.jaxb.spi.Binder;
import org.hibernate.boot.jaxb.spi.Binding;
import org.hibernate.boot.xsd.XmlValidationMode;
import org.hibernate.internal.util.StringHelper;

import org.jboss.logging.Logger;
Expand All @@ -36,15 +38,18 @@ public abstract class AbstractBinder<T> implements Binder<T> {

private final LocalXmlResourceResolver xmlResourceResolver;

protected InputStreamAccess streamAccess;

protected AbstractBinder(ResourceStreamLocator resourceStreamLocator) {
this.xmlResourceResolver = new LocalXmlResourceResolver( resourceStreamLocator );
}

public abstract boolean isValidationEnabled();
public abstract XmlValidationMode getXmlValidationMode();

@Override
public <X extends T> Binding<X> bind(InputStream stream, Origin origin) {
final XMLEventReader eventReader = createReader( stream, origin );
public <X extends T> Binding<X> bind(InputStreamAccess streamAccess, Origin origin) {
this.streamAccess = streamAccess;
final XMLEventReader eventReader = createReader( streamAccess.accessInputStream(), origin );
try {
return doBind( eventReader, origin );
}
Expand Down Expand Up @@ -146,17 +151,19 @@ protected static boolean hasNamespace(StartElement startElement) {
return StringHelper.isNotEmpty( startElement.getName().getNamespaceURI() );
}

protected <X extends T> X jaxb(XMLEventReader reader, Schema xsd, JAXBContext jaxbContext, Origin origin) {
protected void validateXml(Unmarshaller unmarshaller, Consumer<Unmarshaller> validationAction) {
// handled by subclasses if needed/applicable
validationAction.accept(unmarshaller);
}

protected <X extends T> X jaxb(XMLEventReader reader, JAXBContext jaxbContext, Origin origin, Consumer<Unmarshaller> validationAction) {
final ContextProvidingValidationEventHandler handler = new ContextProvidingValidationEventHandler();

try {
final Unmarshaller unmarshaller = jaxbContext.createUnmarshaller();
if ( isValidationEnabled() ) {
unmarshaller.setSchema( xsd );
}
else {
unmarshaller.setSchema( null );
}

validateXml( unmarshaller, validationAction );

unmarshaller.setEventHandler( handler );

//noinspection unchecked
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,22 @@
import javax.xml.stream.XMLEventReader;
import javax.xml.stream.events.StartElement;

import jakarta.xml.bind.Unmarshaller;
import org.hibernate.Internal;
import org.hibernate.boot.ResourceStreamLocator;
import org.hibernate.boot.jaxb.Origin;
import org.hibernate.boot.jaxb.configuration.spi.JaxbPersistenceImpl;
import org.hibernate.boot.jaxb.internal.stax.ConfigurationEventReader;
import org.hibernate.boot.jaxb.spi.Binding;
import org.hibernate.boot.xsd.ConfigXsdSupport;
import org.hibernate.boot.xsd.XmlValidationMode;
import org.hibernate.internal.util.config.ConfigurationException;

import jakarta.xml.bind.JAXBContext;
import jakarta.xml.bind.JAXBException;

import java.util.function.Consumer;

/**
* @author Steve Ebersole
*/
Expand All @@ -32,8 +36,8 @@ public ConfigurationBinder(ResourceStreamLocator resourceStreamLocator) {
}

@Override
public boolean isValidationEnabled() {
return false;
public XmlValidationMode getXmlValidationMode() {
return XmlValidationMode.DISABLED;
}

@Override
Expand All @@ -42,11 +46,22 @@ protected <X extends JaxbPersistenceImpl> Binding<X> doBind(
StartElement rootElementStartEvent,
Origin origin) {
final XMLEventReader reader = new ConfigurationEventReader( staxEventReader, xmlEventFactory );

final Consumer<Unmarshaller> validationAction;
// evaluate extended (the former validate_xml 'true') in case anyone should override getXmlValidationMode() to switch it on
if ( getXmlValidationMode() == XmlValidationMode.EXTENDED ) {
validationAction = unmarshaller -> unmarshaller.setSchema(
ConfigXsdSupport.configurationXsd().getSchema() );
}
else {
validationAction = unmarshaller -> unmarshaller.setSchema( null );
}

final JaxbPersistenceImpl bindingRoot = jaxb(
reader,
ConfigXsdSupport.configurationXsd().getSchema(),
jaxbContext(),
origin
origin,
validationAction
);
//noinspection unchecked
return new Binding<>( (X) bindingRoot, origin );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ public Binding doBind(Binder binder) {
}

public static Binding doBind(Binder binder, InputStreamAccess inputStreamAccess, Origin origin) {
return inputStreamAccess.fromStream(
inputStream -> binder.bind( inputStream, origin )
);
return binder.bind( inputStreamAccess, origin ) ;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import java.io.InputStream;

import org.hibernate.boot.InvalidMappingException;
import org.hibernate.boot.archive.internal.RepeatableInputStreamAccess;
import org.hibernate.boot.jaxb.Origin;
import org.hibernate.boot.jaxb.spi.Binder;
import org.hibernate.boot.jaxb.spi.Binding;
Expand Down Expand Up @@ -38,7 +39,7 @@ public Binding doBind(Binder binder) {

public static Binding doBind(Binder binder, InputStream inputStream, Origin origin, boolean autoClose) {
try {
return binder.bind( inputStream, origin );
return binder.bind( new RepeatableInputStreamAccess( origin.getName(), inputStream), origin );
}
catch ( Exception e ) {
throw new InvalidMappingException( origin, e );
Expand Down
Loading
Loading