Why is this simple bit of code outputting a corrupted class file?

Question

ClassReader classReader = new ClassReader(new FileInputStream(new File("input.class")));
ClassWriter classWriter = new ClassWriter(classReader, ClassWriter.COMPUTE_MAXS | ClassWriter.COMPUTE_FRAMES);
Files.write(Paths.get("output.class"), classWriter.toByteArray());

When decompiling output.class I get

package corrupted_class_files;

The input.class is good and I can use the ClassReader to read the instructions just fine, I just cannot save the class

Answer 1

Your code lacks the step of actually copying the class features from the source to the destination:

try(FileInputStream in = new FileInputStream(new File("input.class")) {
    ClassReader classReader = new ClassReader(in);
    ClassWriter classWriter = new ClassWriter(classReader, 0);
    classReader.accept(classWriter, 0);
    Files.write(Paths.get("output.class"), classWriter.toByteArray());
}

Passing the ClassReader to the ClassWriter’s constructor does not copy the features, it rather enables optimizations, if your transformation keeps most of the original class files. Or, as the documentation of ClassWriter(ClassReader classReader, int flags) puts it:

Constructs a new ClassWriter object and enables optimizations for "mostly add" bytecode transformations. These optimizations are the following:

• The constant pool and bootstrap methods from the original class are copied as is in the new class, which saves time. New constant pool entries and new bootstrap methods will be added at the end if necessary, but unused constant pool entries or bootstrap methods won't be removed.
• Methods that are not transformed are copied as is in the new class, directly from the original class bytecode (i.e. without emitting visit events for all the method instructions), which saves a lot of time. Untransformed methods are detected by the fact that the ClassReader receives MethodVisitor objects that come from a ClassWriter (and not from any other ClassVisitor instance).

So, when you chain the ClassWriter directly to the ClassReader in the accept method, all method visitors will stem from the writer, hence, all of them are copied directly.

When you are going to change the class significantly or constructing a new class, you would use the constructor ClassWriter(int flags) instead.

Note that COMPUTE_FRAMES already implies COMPUTE_MAXS. In the example above, I specified neither, as the methods are copied anyway. When you are going to actually change or add code and need COMPUTE_FRAMES, it’s worth specifying SKIP_FRAMES to the reader, as there is no point in decoding the original frames when they get recalculated from scratch anyway.

So a typical transformation setup looks like this:

public class MyClassVisitor extends ClassVisitor {

    public MyClassVisitor(ClassVisitor cv) {
        super(Opcodes.ASM5, cv);
    }

    @Override
    public MethodVisitor visitMethod(int access, String name, String desc,
                                     String signature, String[] exceptions) {
        MethodVisitor visitor = super.visitMethod(
            access, name, desc, signature, exceptions);
        if(method matches criteria) {
            visitor = new MyMethodVisitorAdapter(visitor);
        }
        return visitor;
    }
}

try(FileInputStream in = new FileInputStream(new File("input.class"))) {
    ClassReader classReader = new ClassReader(in);
    ClassWriter classWriter = new ClassWriter(classReader, ClassWriter.COMPUTE_FRAMES);
    classReader.accept(new MyClassVisitor(classWriter), ClassReader.SKIP_FRAMES);
    Files.write(Paths.get("output.class"), classWriter.toByteArray());
}

When chaining the visitors through the constructors, every method you don’t override will delegate to the chained visitor, replicating the original construct when the final target is a ClassWriter, resp. the MethodVisitor provided by the ClassWriter. If the method does not fulfill your transformation condition, so you return the original MethodVisitor, the optimization described above still applies. The method visitor follows the same pattern as the class visitor, overriding those method you want to intercept.

By the way, you should avoid mixing old I/O and NIO. A simplified variant of your code looks like

ClassReader classReader = new ClassReader(Files.readAllBytes(Paths.get("input.class")));
ClassWriter classWriter = new ClassWriter(classReader, ClassWriter.COMPUTE_FRAMES);
classReader.accept(new MyClassVisitor(classWriter), ClassReader.SKIP_FRAMES);
Files.write(Paths.get("output.class"), classWriter.toByteArray());

^{Note the symmetry between reading and writing}

Though, when you use getResource et al, you might be forced to deal with InputStream. But for classes reachable through the system class loader, you can also just pass the class name to the ClassReader(String) constructor.