Separating codecs and canonicalization functionality into a separate maven artifact

[forgedhallpass]

It would be nice if:

• some of the functionalities (e.g. canonicalization) would be extracted into separate maven modules so that they could be used independently and without the need of initializing the whole library.
• the canonicalization logic would be extracted into a utility class to which e.g. the DefaultEncoder could delegate to.
• the method signature would enable passing in a java 8 Function<String, RuntimeException> so that the invoker could decide what kind of exception should be thrown. The same can be implemented using a custom interface or the canonicalization could throw a new exception declared within the new module, that can be wrapped and re-thrown in the DefaultEncoder.
• the type of the logger to be org.slf4j.Logger so it wouldn't require initializing the whole library

example:

public class DefaultEncoder implements Encoder {
	private final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(DefaultEncoder.class);
	...
	public String canonicalize(String input, boolean restrictMultiple, boolean restrictMixed ) {
		return CononicalizationUtils.canonicalize(input, this.codecs, restrictMultiple, restrictMixed, IntrusionException::new);
	}
        ...
}

public final class CononicalizationUtils {
	private final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(CononicalizationUtils .class);

	private CononicalizationUtils() {}

	public String canonicalize(String input, Iterable<Codec<?>> codecs, boolean restrictMultiple, boolean restrictMixed, Function<String, RuntimeException> exceptionFunction ) {
		...
		throw exceptionFunction.apply("Input validation failure...");
                ...
	}
}

[xeno6696]

@jeremiahjstacey and @kwwall have both independently solved the problem of singletons BUT I don't think we've gone so far as to piecemeal the library.

IIRC the last time I talked with Kevin about that the agreement was that would be a strictly ESAPI 2.0 -> 3.0 bridge project, as in, stage 1 of ESAPI 3.0 would be to break up the monolithic kernel structure of the library into exactly the componentized pieces you discuss here.

[kwwall]

The only group of people that I think who would benefit from that in ESAPI 2.x are those who are only using canonicalization. That's too little benefit to too little people for the amount of work that is required. The good news is, if we ever get ESAPI 2.x wrapped up, the intent is in ESAPI 3.x to make it much more componentized than even you suggest. That would probably consist of multiple jars. What we are envisioning there would be something like this (note that this may change as we experiment, but this is the general idea):

• a main esapi-.jar that would only contain interfaces

• an esapi-core-.jar that would contain some of the core components that are broadly used everywhere else. (Might make this part of the interface jar)

• an esapi-stubs-.jar that would have minimal stubbed out, do nothing implementations of all the interfaces

• for each component (validator, encoder, encryptor, etc.) there would be an esapi--.jar that would implement the actual reference component. These probably would be separate Maven components as well (unless, by that time we decide to switch to Gradle). Then your application would include esapi, esapi-core, and whatever esapi- jars that your application intends to use but wouldn't be burdened by all of the current ESAPI dependencies.

But all this is still quite a long way off. Until then, hey, it's open source under one of the most liberal FOSS licenses available so grab the code and build your own.

But just realize that it is not as easy as you may think. (And not only because of all the blasted singletons that @xeno6696 alluded to in the previous comment.) The bigger reason is that in ESAPI, the canonicalization is built around the various codecs and the codecs are most definitely a major important part of the Encoder component. So you'd still be dragging in way more than you might think. @jeremiahjstacey, @xeno6696, and I have discussed this tight-coupling of the current monolithich ESAPI 2.x structure at great length and we all pretty much agree that it is going to take more than minor tweaks to deal with this issue. So we intend to more or less burn it to the ground and re-architect much of it from scratch.

Given that, I'm fairly certain this will be a "wontfix" for ESAPI 2.x, but this sort of thing will be a major design point in ESAPI 3.x.

[forgedhallpass]

@kwwall, @xeno6696 First of all thank you for your quick and detailed responses. It seems this project is more active than the 3.x :)

I have already created a POC for myself and extracted the canonicalization logic I need into a separate module:

• copied most of the code underneath the codecs package
• created two exceptions of my own
• created a CanonicalizationUtil class (currently left it as a singleton)

\---org
	\---owasp
		\---esapi
		|   CanonicalizationUtil.java
		|
		+---codecs
		|   |   AbstractCharacterCodec.java
		|   |   AbstractCodec.java
		|   |   AbstractIntegerCodec.java
		|   |   AbstractPushbackSequence.java
		|   |   Codec.java
		|   |   HashTrie.java
		|   |   HTMLEntityCodec.java
		|   |   JavaScriptCodec.java
		|   |   PercentCodec.java
		|   |   PushbackSequence.java
		|   |   PushBackSequenceImpl.java
		|   |   PushbackString.java
		|   |   Trie.java
		|   |
		|   \---error
		|           EncodingException.java
		|           IntrusionException.java
		|
		\---util
			NullSafe.java

The only dependency I have is slf4j-api, and it works, although I've thrown out the encoding logic as I am using the OWASP Java Encoder project for that.

The question is, would you consider accepting a pull request and doing a release for this module so I could use it in my projects? It could even be committed to the ESAPI 3.x project and use it in here as well. I assume it would bring you a little step towards your end goal.

[kwwall]

@forgedhallpass - The one thing that I'm not sure you are considering is ESAPI is a library so unlike an application, we can't just arbitrarily refactor the code. Backward compatibility is very important. It's not as much as a concern for major releases (e.g., changing from 2.x to 3.x), but it is in only updating a minor release (2.x to 2.y). And because of the way ESAPI 2.x is written, we have to assume that any public class might be directly being used even though all those classes were not intended 5o be used in that manner. So when making ESAPI changes, our motto has been to "break no code using ESAPI". The only time we might make an exception to that and intentionally break code using ESAPI is if a change is required to fix a serious security vulnerability and even then we try to minimize the impact to client applications using ESAPI.

Given that, I'm not sure your proposed changes are going to allow that. The only way that I can see that you can accomplish what you desire is to split out the canonicalization into a separate jar file (e.g., something like esapi-canonicalization-version.jar or esapi-c14n-version.jar or whatever you intend to call it). If the only place that canonicalization were used was via the DefaultEncoder, perhaps we could figure out a way to at list minimize the impact to ESAPI users. But canonicalization is used in other places as well, so it's not going to be something simple like "if you are using any of the Encoder.canonicalize() methods, then you also include the new esapi-canonicalization-verison.jar as a dependency" and as a result only having it affect a small subset of the ESAPI community.

Instead, it's pretty much going to require almost everyone using ESAPI in any significant way to include the new esapi-canonicalization-version.jar would have to do that. Okay, maybe that's not that big of a deal; I mean, as long as the package names stay the same, in theory we're only talking about a compile-time dependency for the ESAPI users. But there are other things as well. To be used separately, you are going to have to place all the those classes that you've identified in your new ESAPI jar and we would want to remove them from the normal esapi-version.jar. So that means that the Javadoc will appear elsewhere as well. So now if a user wants to find a class in org.owasp.esapi.codecs package, that user will have to potentially look in both ESAPI jars. That makes ESAPI harder to use. It's also going to make splitting out JUnit tests somewhat of a pain (although that's mostly only a one time thing, but it will make it a bit more of a PITA adding new test cases). Lastly, I'm concerned that it will complicate the ESAPI release steps as documented in ESAPI-release-steps.odt.

So I am not going to reject a PR outright--we would at least look at it, but I think this is going to be a uphill battle to convince the 3 of us major contributors, much less the rest of the ESAPI community. Like I said in previous comments, this seems like a small benefit to relatively few in the ESAPI community. (My personal observations over the past 10 years or so is that 90% of ESAPI users are using its Encoders, maybe 5% are using the Validators, and 5% are using all the rest of the components.)

That said, I'm not quite sure for your reasons for doing this. One can use ESAPI with the Java Encoder project (without using their ESAPI shim), so I can only guess is your concern is all the dependencies that ESAPI pulls in. (I'm just guessing that because you mention "The only dependency I have is slf4j-ap".)

If dependencies are indeed the the reason you are attempting all this then you could just explicitly exclude the transitive dependencies that are not actually being used by your application (jdepend is your friend) and then replace DefaultEncoder with a minimal stubbed out (say) MyCustomEsapiEncoder (and referencing it in ESAPI.Encoder property in your ESAPI.properties file). To me, that seems like a much more reasonable approach and doesn't have all the potential problems that your approach has.

[forgedhallpass]

I am still not sure what do you see as a big of a problem, maybe I am missing something. There could be a maven multi-module project, with the parent with pom packaging, two modules for now, the esapi-java-legacy and the esapi-c14, or some other name as you mentioned above. The esapi-java-legacy module with jar packaging type could have the esapi-c14 module as a compile dependency. Both modules could inherit the version from the parent, so the releasing should not be too complicated and users wouldn't need to add the new dependency manually either.

As for my motivation to get rid of ESAPI:

• question like Should I use ESAPI? I'd prefer keeping the operational risk of my projects low and legacy for me means that at some point it will be deprecated.

  "The second question you should ask, if I’m using it, why am I not contributing to it in some manner?"

  You can't say I haven't tried ;-)

• it also seems that the 3.x version is DOA. Last commits around 7 years old. Maybe the investment should rather go there...

• I need to get rid of LGPL 2.1 licenses (com.io7m.xom:xom)

• my OSS scanning tool reported that there were vulnerable transitive dependencies (I believe that has been solved with v2.2.2.0)

• I appreciate your effort of wanting to be backwards compatible, but since this is blocking you moving further it will only drive people away. E.g. the EOL date for Java 7 was ~5 years ago. The market dictates if you decide not to upgrade, you'll have to pay for the support...in case of FOS, that could mean that you'll have to backport the changes to a legacy version that you are using.

With that being said, I do not intend to spend time on a PR, if you are not convinced, but it's funny that:

1. you have talked about backward compatibility, because I've tried upgrading from v2.2.0.0 to v2.2.2.0 and my build failed. Of course this could be because I might have initialized the solution in an unintended way (a few years back), with the intention to get it up and running with as less initialization code as possible:

private static void initESAPI() {
        final Properties esapiProperties = new Properties();
        esapiProperties.setProperty(DefaultSecurityConfiguration.ENCODER_IMPLEMENTATION, CustomEncoder.class.getName());
        esapiProperties.setProperty(DefaultSecurityConfiguration.ALLOW_MULTIPLE_ENCODING, String.valueOf(false));
        esapiProperties.setProperty(DefaultSecurityConfiguration.ALLOW_MIXED_ENCODING, String.valueOf(false));
        esapiProperties.setProperty(DefaultSecurityConfiguration.CIPHER_TRANSFORMATION_IMPLEMENTATION, "AES/CBC/PKCS5Padding");

        ESAPI.override(new DefaultSecurityConfiguration(esapiProperties));
    }

    
public static class CustomEncoder extends DefaultEncoder {

	private static volatile Encoder singletonInstance;

	public CustomEncoder(final List<String> codecNames) {
		super(codecNames);
	}

	public static Encoder getInstance() {
		if (singletonInstance == null) {
			synchronized (DefaultEncoder.class) {
				if (singletonInstance == null) {
					singletonInstance = new DefaultEncoder(Arrays.asList(HTMLEntityCodec.class.getSimpleName(),
											     PercentCodec.class.getSimpleName()));
				}
			}
		}
		return singletonInstance;
	}
}

2. After I've extracted only the canonicalization part for my projects, I had to modify the documentation because my build failed again because of the javadocs from ESAPI (-Xdoclint:-html).

[kwwall]

@forgedhallpass - I would like to respond to your last comment, point by point, but unfortunately, I find this forum of GitHub comments to something this complex to be somewhat of a PITA to do that with.(Also, that is the forum where we really should be discussing ESAPI development concerns.) So I am going to respond to this, but on the ESAPI-Project-Dev Google group. Anyone can read that, but in order to post to it, you need to first subscribe.

• First, subscribe to ESAPI Developers list
• Then, send email to esapi-project-dev to post or ideally, follow up to this post https://groups.google.com/a/owasp.org/g/esapi-project-dev/c/opx8sSqW4Yg

UNTIL FURTHER NOTICE, please follow up on the ESAPI Developers list on Google Groups, as mentioned above. Thanks.