coheigea commented on code in PR #101:
URL:
https://github.com/apache/santuario-xml-security-java/pull/101#discussion_r1241595720
##########
src/main/java/org/apache/jcp/xml/dsig/internal/dom/ApacheNodeSetData.java:
##########
@@ -35,9 +35,9 @@
import org.apache.xml.security.utils.XMLUtils;
import org.w3c.dom.Node;
-public class ApacheNodeSetData implements ApacheData, NodeSetData {
+public class ApacheNodeSetData implements ApacheData, NodeSetData<Node> {
- private XMLSignatureInput xi;
+ private final XMLSignatureInput xi;
Review Comment:
Could all changes in this class be backported in a separate PR?
##########
src/main/java/org/apache/jcp/xml/dsig/internal/dom/ApacheOctetStreamData.java:
##########
@@ -27,15 +27,12 @@
import org.apache.xml.security.signature.XMLSignatureInput;
-public class ApacheOctetStreamData extends OctetStreamData
- implements ApacheData {
+public class ApacheOctetStreamData extends OctetStreamData implements
ApacheData {
- private XMLSignatureInput xi;
Review Comment:
Could all changes in this class be backported in a separate PR?
##########
src/main/java/org/apache/xml/security/c14n/implementations/Canonicalizer20010315.java:
##########
@@ -303,10 +303,10 @@ protected void circumventBugIfNeeded(XMLSignatureInput
input)
return;
}
Document doc = null;
Review Comment:
All changes in class could be backported?
##########
src/main/java/org/apache/xml/security/parser/XMLParser.java:
##########
@@ -27,6 +27,15 @@
*/
public interface XMLParser {
+ /**
+ * Parses a document from the input stream.
+ * Caller is responsible for closing the stream.
+ *
+ * @param inputStream
+ * @param disallowDocTypeDeclarations
+ * @return {@link Document}
+ * @throws XMLParserException
+ */
Review Comment:
Backport
##########
src/main/java/org/apache/xml/security/signature/XMLSignature.java:
##########
@@ -225,7 +226,7 @@ public final class XMLSignature extends
SignatureElementProxy {
*/
private boolean followManifestsDuringValidation = false;
- private Element signatureValueElement;
+ private final Element signatureValueElement;
Review Comment:
Backport
##########
src/main/java/org/apache/xml/security/stax/impl/OutputProcessorChainImpl.java:
##########
@@ -31,18 +33,16 @@
import org.apache.xml.security.stax.ext.OutputProcessorChain;
import org.apache.xml.security.stax.ext.stax.XMLSecEvent;
import org.apache.xml.security.stax.ext.stax.XMLSecStartElement;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
/**
* Implementation of a OutputProcessorChain
*
*/
public class OutputProcessorChainImpl implements OutputProcessorChain {
- protected static final transient Logger LOG =
LoggerFactory.getLogger(OutputProcessorChainImpl.class);
+ private static final Logger LOG =
System.getLogger(OutputProcessorChainImpl.class.getName());
- private List<OutputProcessor> outputProcessors;
+ private final List<OutputProcessor> outputProcessors;
Review Comment:
Backport
##########
src/main/java/org/apache/xml/security/utils/resolver/implementations/ResolverFragment.java:
##########
@@ -83,20 +86,19 @@ public XMLSignatureInput
engineResolveURI(ResourceResolverContext context)
);
}
}
- LOG.debug(
- "Try to catch an Element with ID {} and Element was {}", id,
selectedElem
+ LOG.log(Level.DEBUG,
+ "Try to catch an Element with ID {0} and Element was {1}", id,
selectedElem
);
}
- XMLSignatureInput result = new XMLSignatureInput(selectedElem);
+ XMLSignatureInput result = new XMLSignatureNodeInput(selectedElem);
result.setSecureValidation(context.secureValidation);
result.setExcludeComments(true);
-
result.setMIMEType("text/xml");
- if (context.baseUri != null && context.baseUri.length() > 0) {
- result.setSourceURI(context.baseUri.concat(context.uriToResolve));
- } else {
+ if (context.baseUri == null || context.baseUri.isEmpty()) {
result.setSourceURI(context.uriToResolve);
+ } else {
+ result.setSourceURI(context.baseUri.concat(context.uriToResolve));
Review Comment:
backport
##########
src/main/java/org/apache/xml/security/utils/resolver/implementations/ResolverLocalFilesystem.java:
##########
@@ -74,24 +70,24 @@ public boolean engineCanResolveURI(ResourceResolverContext
context) {
}
try {
- LOG.debug("I was asked whether I can resolve {}",
context.uriToResolve);
+ LOG.log(Level.DEBUG, "I was asked whether I can resolve {0}",
context.uriToResolve);
if (context.uriToResolve.startsWith("file:") ||
context.baseUri.startsWith("file:")) {
- LOG.debug("I state that I can resolve {}",
context.uriToResolve);
+ LOG.log(Level.DEBUG, "I state that I can resolve {0}",
context.uriToResolve);
return true;
}
} catch (Exception e) {
- LOG.debug(e.getMessage(), e);
+ LOG.log(Level.DEBUG, e.getMessage(), e);
}
- LOG.debug("But I can't");
+ LOG.log(Level.DEBUG, "But I can't");
return false;
}
private static URI getNewURI(String uri, String baseURI) throws
URISyntaxException {
- URI newUri = null;
- if (baseURI == null || baseURI.length() == 0) {
+ final URI newUri;
+ if (baseURI == null || baseURI.isEmpty()) {
Review Comment:
backport
##########
src/main/java/org/apache/jcp/xml/dsig/internal/dom/DOMReference.java:
##########
@@ -118,7 +123,7 @@ public final class DOMReference extends DOMStructure
private Data derefData;
private InputStream dis;
private MessageDigest md;
- private Provider provider;
+ private final Provider provider;
Review Comment:
Could be backported?
##########
.github/codeql/santuario.qls:
##########
@@ -1,5 +1,3 @@
- import: codeql-suites/java-security-and-quality.qls
from: codeql-java
-- exclude:
- id: java/missing-override-annotation
Review Comment:
Could be backported in a separate PR?
##########
src/main/java/org/apache/jcp/xml/dsig/internal/dom/Utils.java:
##########
@@ -35,24 +35,23 @@
/**
* Miscellaneous static utility methods for use in JSR 105 RI.
- *
*/
public final class Utils {
- private Utils() {}
+ private Utils() {
Review Comment:
Could be backported?
##########
src/main/java/org/apache/xml/security/encryption/XMLCipher.java:
##########
@@ -3462,7 +3464,7 @@ public String getBaseNamespace() {
private class ReferenceListImpl implements ReferenceList {
private Class<?> sentry;
- private List<Reference> references;
+ private final List<Reference> references;
Review Comment:
The final changes in this class could be backported
##########
src/main/java/org/apache/xml/security/c14n/implementations/Canonicalizer20010315Excl.java:
##########
@@ -342,11 +342,11 @@ protected void circumventBugIfNeeded(XMLSignatureInput
input)
if (!input.isNeedsToBeExpanded() || inclusiveNSSet.isEmpty()) {
return;
}
- Document doc = null;
- if (input.getSubNode() != null) {
- doc = XMLUtils.getOwnerDocument(input.getSubNode());
- } else {
+ Document doc;
Review Comment:
All changes in class could be backported?
##########
src/main/java/org/apache/xml/security/keys/KeyInfo.java:
##########
@@ -113,7 +114,7 @@ public class KeyInfo extends SignatureElementProxy {
/**
* Stores the individual (per-KeyInfo) {@link KeyResolverSpi}s
*/
- private List<KeyResolverSpi> internalKeyResolvers = new ArrayList<>();
+ private final List<KeyResolverSpi> internalKeyResolvers = new
ArrayList<>();
Review Comment:
Backport
##########
src/main/java/org/apache/xml/security/signature/Manifest.java:
##########
@@ -55,16 +57,15 @@ public class Manifest extends SignatureElementProxy {
*/
public static final int MAXIMUM_REFERENCE_COUNT = 30;
- private static final org.slf4j.Logger LOG =
- org.slf4j.LoggerFactory.getLogger(Manifest.class);
+ private static final Logger LOG =
System.getLogger(Manifest.class.getName());
private static Integer referenceCount =
AccessController.doPrivileged(
(PrivilegedAction<Integer>) () ->
Integer.parseInt(System.getProperty("org.apache.xml.security.maxReferences",
Integer.toString(MAXIMUM_REFERENCE_COUNT))));
/** Field references */
- private List<Reference> references;
+ private final List<Reference> references;
Review Comment:
Backport
##########
src/main/java/org/apache/xml/security/stax/ext/ComparableType.java:
##########
@@ -21,9 +21,9 @@
/**
*/
-public abstract class ComparableType<T extends ComparableType> implements
Comparable<T> {
+public abstract class ComparableType<T extends ComparableType<?>> implements
Comparable<T> {
Review Comment:
Backport all class changes
##########
src/main/java/org/apache/xml/security/encryption/XMLCipherInput.java:
##########
@@ -44,11 +46,10 @@
*/
public class XMLCipherInput {
- private static final org.slf4j.Logger LOG =
- org.slf4j.LoggerFactory.getLogger(XMLCipherInput.class);
+ private static final Logger LOG =
System.getLogger(XMLCipherInput.class.getName());
/** The data we are working with */
- private CipherData cipherData;
+ private final CipherData cipherData;
Review Comment:
Backport
##########
src/main/java/org/apache/xml/security/c14n/CanonicalizerSpi.java:
##########
@@ -42,13 +43,13 @@ public abstract class CanonicalizerSpi {
*
* @throws XMLParserException
* @throws java.io.IOException
- * @throws javax.xml.parsers.ParserConfigurationException
+ * @throws CanonicalizationException
Review Comment:
All changes in class could be backported?
##########
src/main/java/org/apache/xml/security/stax/impl/InputProcessorChainImpl.java:
##########
@@ -30,18 +32,16 @@
import org.apache.xml.security.stax.ext.InputProcessorChain;
import org.apache.xml.security.stax.ext.XMLSecurityConstants;
import org.apache.xml.security.stax.ext.stax.XMLSecEvent;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
/**
* Implementation of a InputProcessorChain
*
*/
public class InputProcessorChainImpl implements InputProcessorChain {
- protected static final transient Logger LOG =
LoggerFactory.getLogger(InputProcessorChainImpl.class);
+ private static final Logger LOG =
System.getLogger(InputProcessorChainImpl.class.getName());
- private List<InputProcessor> inputProcessors;
+ private final List<InputProcessor> inputProcessors;
Review Comment:
Backport
##########
src/main/java/org/apache/xml/security/stax/impl/processor/input/AbstractDecryptInputProcessor.java:
##########
@@ -620,7 +620,7 @@ public abstract class
AbstractDecryptedEventReaderInputProcessor
private boolean encryptedHeader = false;
private final InboundSecurityToken inboundSecurityToken;
private boolean rootElementProcessed;
- private EncryptedDataType encryptedDataType;
+ private final EncryptedDataType encryptedDataType;
Review Comment:
Backport
##########
src/main/java/org/apache/xml/security/transforms/implementations/TransformXPath.java:
##########
@@ -88,9 +88,7 @@ protected XMLSignatureInput enginePerformTransform(
}
Node xpathnode = xpathElement.getFirstChild();
if (xpathnode == null) {
- throw new DOMException(
- DOMException.HIERARCHY_REQUEST_ERR, "Text must be in
ds:Xpath"
- );
+ throw new DOMException(DOMException.HIERARCHY_REQUEST_ERR,
"Text must be in ds:Xpath");
Review Comment:
backport
##########
src/main/java/org/apache/xml/security/stax/impl/processor/output/AbstractSignatureEndingOutputProcessor.java:
##########
@@ -56,18 +58,16 @@
import org.apache.xml.security.stax.securityToken.SecurityTokenProvider;
import org.apache.xml.security.utils.UnsyncBufferedOutputStream;
import org.apache.xml.security.utils.XMLUtils;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
import static
org.apache.xml.security.algorithms.implementations.SignatureBaseRSA.SignatureRSASSAPSS.DigestAlgorithm.fromDigestAlgorithm;
/**
*/
public abstract class AbstractSignatureEndingOutputProcessor extends
AbstractBufferingOutputProcessor {
- private static final transient Logger LOG =
LoggerFactory.getLogger(AbstractSignatureEndingOutputProcessor.class);
+ private static final transient Logger LOG =
System.getLogger(AbstractSignatureEndingOutputProcessor.class.getName());
- private List<SignaturePartDef> signaturePartDefList;
+ private final List<SignaturePartDef> signaturePartDefList;
Review Comment:
backport
##########
src/main/java/org/apache/xml/security/stax/impl/processor/output/AbstractSignatureEndingOutputProcessor.java:
##########
@@ -287,9 +287,9 @@ protected static class SignedInfoProcessor extends
AbstractOutputProcessor {
private Transformer transformer;
private byte[] signatureValue;
private String inclusiveNamespacePrefixes;
- private SignatureAlgorithm signatureAlgorithm;
- private XMLSecStartElement xmlSecStartElement;
- private String signatureId;
+ private final SignatureAlgorithm signatureAlgorithm;
+ private final XMLSecStartElement xmlSecStartElement;
+ private final String signatureId;
Review Comment:
backport
##########
src/main/java/org/apache/xml/security/transforms/implementations/TransformXPath2Filter.java:
##########
@@ -85,10 +85,10 @@ protected XMLSignatureInput enginePerformTransform(
}
Document inputDoc = null;
Review Comment:
Backport
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
