Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve tree-API namespace setters' robustness against invalid input #117

Merged
merged 2 commits into from
Dec 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 21 additions & 2 deletions src/main/java/net/fabricmc/mappingio/tree/MappingTree.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,22 +21,41 @@

import org.jetbrains.annotations.Nullable;

import net.fabricmc.mappingio.adapter.MappingDstNsReorder;
import net.fabricmc.mappingio.adapter.MappingSourceNsSwitch;

/**
* Mutable mapping tree.
*
* <p>All returned collections are to be assumed unmodifiable, unless explicitly stated otherwise.
*/
public interface MappingTree extends MappingTreeView {
/**
* Sets the tree's and all of its contained elements' source namespace.
* Sets the tree's and all of its contained elements' source namespace name.
*
* <p>If the passed namespace name equals an existing destination namespace's name,
* implementors may choose to switch the two namespaces around, analogous to {@link MappingSourceNsSwitch}.
* This has to be made clear in the implementation's documentation.
*
* @return The previous source namespace, if present.
* @implSpec If switching with an existing destination namespace is requested, but not supported, an {@link UnsupportedOperationException} must be thrown.
*
* @return The previous source namespace name, if present.
*/
@Nullable
String setSrcNamespace(String namespace);

/**
* Sets the tree's and all of its contained elements' destination namespace names.
*
* <p>Can be used to reorder and/or drop destination namespaces, analogous to {@link MappingDstNsReorder}.
*
* <p>Implementors may allow switching with the source namespace as well, analogous to {@link MappingSourceNsSwitch}.
* This has to be made clear in the implementation's documentation.
*
* @implSpec If switching with the source namespace is requested, but not supported, an {@link UnsupportedOperationException} must be thrown.
*
* @return The previous destination namespaces.
* @throws IllegalArgumentException If the passed namespace names contain duplicates.
*/
List<String> setDstNamespaces(List<String> namespaces);

Expand Down
41 changes: 39 additions & 2 deletions src/main/java/net/fabricmc/mappingio/tree/MemoryMappingTree.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,13 @@
import net.fabricmc.mappingio.MappedElementKind;
import net.fabricmc.mappingio.MappingFlag;
import net.fabricmc.mappingio.MappingVisitor;
import net.fabricmc.mappingio.adapter.MappingSourceNsSwitch;

/**
* {@link VisitableMappingTree} implementation that stores all data in memory.
*
* <p>Switching the source namespace with an existing destination namespace via
* {@link #setSrcNamespace(String)} or {@link #setDstNamespaces(List)} is not supported yet.
*/
public final class MemoryMappingTree implements VisitableMappingTree {
public MemoryMappingTree() {
Expand Down Expand Up @@ -118,13 +122,25 @@ public String getSrcNamespace() {
return srcNamespace;
}

/**
* {@inheritDoc}
*
* @throws UnsupportedOperationException If the passed namespace name is already in use by one of the destination namespaces. This may change in a future release.
*/
@Override
@Nullable
public String setSrcNamespace(String namespace) {
assertNotInVisitPass();

if (dstNamespaces.contains(namespace)) {
throw new UnsupportedOperationException(String.format(
"Can't use name \"%s\" for the source namespace, as it's already in use by one of the destination namespaces %s."
+ " If a source namespace shuffle was the desired outcome, please resort to a %s instead; %s doesn't support this operation natively yet.",
namespace, dstNamespaces, MappingSourceNsSwitch.class.getSimpleName(), getClass().getSimpleName()));
}

String ret = srcNamespace;
srcNamespace = namespace;

return ret;
}

Expand All @@ -133,23 +149,44 @@ public List<String> getDstNamespaces() {
return dstNamespaces;
}

/**
* {@inheritDoc}
*
* @throws IllegalArgumentException If the passed namespace names contain duplicates.
* @throws UnsupportedOperationException If the passed namespace names contain the source namespace's name. This may change in a future release.
*/
@Override
public List<String> setDstNamespaces(List<String> namespaces) {
assertNotInVisitPass();

if (!classesBySrcName.isEmpty()) { // classes present, update existing dstNames
int newSize = namespaces.size();
int[] nameMap = new int[newSize];
Set<String> processedNamespaces = new HashSet<>(newSize);
Set<String> duplicateNamespaces = new HashSet<>(newSize);

for (int i = 0; i < newSize; i++) {
String newNs = namespaces.get(i);

if (newNs.equals(srcNamespace)) {
throw new IllegalArgumentException("can't use the same namespace for src and dst");
throw new UnsupportedOperationException(String.format(
"Can't use name \"%s\" for destination namespace %s, as it's already in use by the source namespace."
+ " If a source namespace shuffle was the desired outcome, please resort to a %s instead; %s doesn't support this operation natively yet.",
newNs, i, MappingSourceNsSwitch.class.getSimpleName(), getClass().getSimpleName()));
} else {
int oldNsIdx = dstNamespaces.indexOf(newNs);
nameMap[i] = oldNsIdx;
}

if (processedNamespaces.contains(newNs)) {
duplicateNamespaces.add(newNs);
}

processedNamespaces.add(newNs);
}

if (!duplicateNamespaces.isEmpty()) {
throw new IllegalArgumentException("Duplicate destination namespace names: " + duplicateNamespaces);
}

boolean useResize = true;
Expand Down
108 changes: 108 additions & 0 deletions src/test/java/net/fabricmc/mappingio/tree/SetNamespacesTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
/*
* Copyright (c) 2024 FabricMC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package net.fabricmc.mappingio.tree;

import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.io.IOException;
import java.util.Arrays;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import net.fabricmc.mappingio.TestHelper;

public class SetNamespacesTest {
private MemoryMappingTree tree;
private String srcNs;
private String dstNs0;
private String dstNs1;

@BeforeEach
public void setup() throws IOException {
tree = new MemoryMappingTree();
TestHelper.acceptTestMappings(tree);

srcNs = tree.getSrcNamespace();
dstNs0 = tree.getDstNamespaces().get(0);
dstNs1 = tree.getDstNamespaces().get(1);
}

@Test
public void srcToNewSrc() {
String srcNsNew = srcNs + "New";

tree.setSrcNamespace(srcNsNew);
assertSrc(srcNsNew);
assertDst(dstNs0, dstNs1);

tree.setSrcNamespace(srcNs);
assertSrc(srcNs);
assertDst(dstNs0, dstNs1);
}

@Test
public void dstToNewDst() {
String dstNs0New = dstNs0 + "New";
String dstNs1New = dstNs1 + "New";

tree.setDstNamespaces(Arrays.asList(dstNs0New, dstNs1New));
assertSrc(srcNs);
assertDst(dstNs0New, dstNs1New);

tree.setDstNamespaces(Arrays.asList(dstNs0, dstNs1));
assertSrc(srcNs);
assertDst(dstNs0, dstNs1);
}

@Test
public void srcToDst() {
assertThrows(UnsupportedOperationException.class, () -> tree.setSrcNamespace(dstNs0));
}

@Test
public void dstToSrc() {
assertThrows(UnsupportedOperationException.class, () -> tree.setDstNamespaces(Arrays.asList(srcNs, dstNs1)));
}

@Test
public void dstToDst() {
tree.setDstNamespaces(Arrays.asList(dstNs1, dstNs0));
assertSrc(srcNs);
assertDst(dstNs1, dstNs0);

tree.setDstNamespaces(Arrays.asList(dstNs0, dstNs1));
assertSrc(srcNs);
assertDst(dstNs0, dstNs1);
}

@Test
public void duplicateDst() {
assertThrows(IllegalArgumentException.class, () -> tree.setDstNamespaces(Arrays.asList(dstNs0, dstNs0)));
assertThrows(IllegalArgumentException.class, () -> tree.setDstNamespaces(Arrays.asList(dstNs1, dstNs1)));
assertThrows(IllegalArgumentException.class, () -> tree.setDstNamespaces(Arrays.asList(dstNs1 + "New", dstNs1 + "New")));
}

private void assertSrc(String srcNs) {
assertTrue(tree.getSrcNamespace().equals(srcNs));
}

private void assertDst(String... dstNs) {
assertTrue(tree.getDstNamespaces().equals(Arrays.asList(dstNs)));
}
}