CGMES loading from zipped profiles inside a folder

[quinarygio]

Please check if the PR fulfills these requirements

• The commit message follows our guidelines
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

Does this PR already have an issue describing the problem?

#3259

Does this PR introduce a breaking change or deprecate an API?

• Yes
• No

Other information:

[olperr1]

Suggested change

	// copy and compress each of the profile to the file system
	Path workDir = fileSystem.getPath("/work");
	for (String profile : profiles) {
	try (var is = testDataSource.newInputStream(profile);
	var os = new ZipOutputStream(Files.newOutputStream(workDir.resolve(profile + ".zip")))) {
	os.closeEntry();
	}
	}
	Path workDir = fileSystem.getPath("/work");
	for (String profile : profiles) {
	try (var os = new ZipOutputStream(Files.newOutputStream(workDir.resolve(profile + ".zip")))) {
	os.closeEntry();
	}
	}

[olperr1]

In emptyZipErrorTest(), there's no need to create an input stream for each profile since they are not read.

[flo-dup]

There's a performance issue here. We're reading the whole unzipped file to detect if the zip extraction is safe but we only need the first tag (for namespaces definition or the base attribute). I think we need to do something smarter to get safely the first characters only, not unzipping the complete file.

[quinarygio]

I propose to check just first zip entry safety, checking only the compression ratio

[flo-dup]

I think a simpler way is to use a SafeZipInputStream class wrapping the ZipInputStream instead of the ZipInputStream itself:

public class SafeZipInputStream extends ForwardingInputStream<ZipInputStream> {
    @Override
    public int read() throws IOException {
        int byteRead = super.read();
        if (byteRead != -1 && this.bytesRead++ > this.maxBytesToRead) {
          throw new IOException();
        }
        return byteRead;
    }

    @Override
    public int read(byte[] b, int off, int len) throws IOException {
        ... // similar way
    }
}

That way you'll only read the first lines, not unzipping the full file, which might be more than 1GB uncompressed. Besides we could restrict maxBytesRead a lot in this PR (a few kB, or 1MB?), as we only read the first tag anyway (we stop at first START_ELEMENT in the 3 use cases).

[flo-dup]

It's not so clear here, I think we should enumerate the 3 ways (or more?) to import a CGMES file:

• a folder containing all uncompressed profile files
• a folder containing the zipped profile files, each one being in a separate zip file
• a zipped file containing all profile files

[quinarygio]

Using the new SafeZipInputStream class to read from the ZipInputStream, the ZipSecurityHelper class is not used anymore. Should we keep it or not ?

[flo-dup]

We might want to go further than 2GB, so an int is not enough.

Suggested change

	private int bytesRead;
	private int maxBytesToRead;
	private long bytesRead;
	private long maxBytesToRead;

[flo-dup]

We might want to reuse this for other usecases where several ZipInputStream entries have to be read. This is not really easy to use that way, I think you'd need to create a new SafeZipInputStream(zin, 1, max) for each entry?

Adding a SafeZipInputStream::getNextEntry method would do the trick, at the cost of adding a protected T getDelegate() in ForwardingInputStream.

[flo-dup]

to avoid doing twice the addition include this in an if (byteRead + -1) before comparing to the max

[flo-dup]

Indeed as it's not needed anymore (in this PR at least!) we should not keep it

[sonarqubecloud]

Quality Gate failed

Failed conditions
2 Security Hotspots

See analysis details on SonarQube Cloud