fix: potential serialization error due to non-transient field inside a serializable class

[rng70-or]

In file: MimeType.java, the serializable class MimeType contains a non-serializable field. In this case, serialization of the class can lead to a NotSerializableException. To learn more check CWE 594 and also more explanation and its side effects are briefly explained here: What Happens When A Serializable Object Contains a Non-Serializable Field?

Sponsorship and Support:

This work is done by the security researchers from OpenRefactory and is supported by the Open Source Security Foundation (OpenSSF): Project Alpha-Omega. Alpha-Omega is a project partnering with open source software project maintainers to systematically find new, as-yet-undiscovered vulnerabilities in open source code - and get them fixed – to improve global software supply chain security.

The bug is found by running the Intelligent Code Repair (iCR) tool by OpenRefactory and then manually triaging the results.

[jmehrens]

@rng70-or There isn't a failure here because the JAF-TCK has a runtime test which test the writeExternal method directly and this test I wrote to indirectly invoke that method via serialization:

@Test
public void testExternalizable() throws Exception {
    String primary = "text";
    String sub = "html";
    ByteArrayOutputStream baos = new ByteArrayOutputStream();
    try (ObjectOutputStream oos = new ObjectOutputStream(baos)) {
        oos.writeObject(new MimeType(primary, sub));
    }

    InputStream bais = new ByteArrayInputStream(baos.toByteArray());
    try (ObjectInputStream ois = new ObjectInputStream(bais)) {
        MimeType mt = (MimeType) ois.readObject();
        assertEquals(primary, mt.getPrimaryType());
        assertEquals(sub, mt.getSubType());
    }
}

Both tests pass and do not demonstrate CWE 594 as described above. I would be interested in seeing a test that does fail if you can provide one.

If you look at the code for MimeType.java it implements java.io.Externalizable and just saves a string representation of the mime types and completely overrides the behavior of serializable.

The java compiler prints the following warning: jakarta/activation/MimeType.java:[27,35] non-transient instance field of a serializable class declared with a non-serializable type

The compiler and "Intelligent Code Repair (iCR) tool by OpenRefactory" have not been improved to reason about the behavior of Externalizable::writeExternal.

At this time, this PR is simply fixing a compiler warning which means we should consider just suppressing this warning and adding a comment vs. marking as transient. Marking as transient might please some of these analyzer tools but we have to be sure we have enough test coverage before we mark as transient as that opens the door for just dropping data.

[rng70-or]

@jmehrens it seems that, I somehow missed this part during triaging,

public String toString() {
        return getBaseType() + parameters.toString();
 }

public void writeExternal(ObjectOutput out) throws IOException {
        out.writeUTF(toString());
        out.flush();
}

where the parameters are converted to String by invoking MimeTypeParameterList.toString(). So, as the parameters are converted to String which is Serializable thus it does not break the Serialization process.

So, in that case, marking this as transient can have opposite impact which I have initially thought. As, it is turned out to be negative and my initial findings was incorrect thus closing the PR. Thanks for your investigation and it was my bad, I somehow missed that part.