PVS-Studio 개발자 키릴 에피파노프(Kirill Epifanov)가 Create 모드의 특정 커밋을 정적 분석하고 Minecraft에서 일부 동작을 확인했다. 좌표 붙여넣기 실패와 로그 기록 중 예외 발생 가능성 등 여러 문제를 소개했으며, 기고문에 담은 수정 사항은 프로젝트에 풀 리퀘스트로 제출했다.
Create는 축과 기어 같은 기계 부품을 추가하는 Minecraft 모드다. CurseForge에서만 다운로드 수가 2억 600만 회를 넘었으며, Modrinth 다운로드 수는 이 집계에 포함되지 않는다. 에피파노프는 PVS-Studio를 사용해 당시 최신 리비전인 87b3c6a를 분석했다. 글에는 발견한 문제 가운데 흥미롭다고 판단한 사례만 실렸다.
좌표 입력 화면에서는 replaceAll이 반환한 문자열을 저장하지 않아 공백이 제거되지 않았다. 따라서 -156, 67, 204처럼 쉼표 뒤에 공백이 있는 좌표는 정수 변환에 실패해 입력란이 비었고, 공백이 없는 -156,67,204는 작동했다. 에피파노프는 반환값을 coords에 다시 대입하는 수정안을 제시했다.
기계식 제작기에서는 선행 제작기가 없는 경우를 null로 검사했지만, 해당 메서드는 빈 목록을 반환할 뿐 null을 반환하지 않았다. 이 때문에 ejectWholeGrid() 분기는 실행될 수 없었다. 글에 따르면 빈 목록 검사로 바꿔 시험했을 때는 제작기 순환이 생기지 않았지만, 제작기 두 개를 더 연결하자 수정이 제대로 작동하지 않았다. 에피파노프는 순환을 다른 방식으로 감지해야 하는지, 순환 동작 자체가 버그가 아닌지 판단이 필요하다고 설명했다.
열차 이동 패킷 처리에서는 열차를 찾지 못한 경우에도 로그 메시지를 만들면서 train.id에 접근해 잠재적 널 참조가 발생했다. 클라이언트가 존재하지 않는 열차 ID를 보낼 수 있어 서버 로그에 스택 추적이 쌓일 수 있지만, Create는 패킷을 안전하게 실행하도록 등록하므로 서버가 중단되지는 않는다고 글은 설명한다. 제안된 수정은 train.id 대신 패킷에 담긴 trainId를 로그에 사용하는 것이다.
그 밖에도 믹서 코드에 항상 거짓인 조건이 있어 실행되지 않는 보호 분기가 있었고, 존재하지 않는 타입의 구조물을 불러올 때 null 확인 없이 사용해 NullPointerException이 발생할 수 있었다. 후자의 경우 잘못된 타입을 가진 광산 수레를 레일에 올리면 오류가 나고, 게임은 해당 구조물 대신 일반 광산 수레를 표시한다고 설명한다. 디버그 빌드에서 운영자만 쓸 수 있는 서버 지연 명령어에는 InterruptedException을 처리하면서 스레드의 인터럽트 상태를 복원하지 않는 문제도 있었다.
에피파노프는 글에서 다룬 문제를 프로젝트 개발자에게 풀 리퀘스트와 풀 리퀘스트로 제출했다고 밝혔다. 각 코드와 분석 경고는 원문에서 확인할 수 있다.
부록: 원문 예시
public boolean keyPressed(int code, ....) {
if (isPaste(code)) {
String coords = minecraft.keyboardHandler.getClipboard();
if (coords != null && !coords.isEmpty()) {
coords.replaceAll(" ", ""); // <=
String[] split = coords.split(",");
if (split.length == 3) {
boolean valid = true;
for (String s : split) {
try {
Integer.parseInt(s);
} catch (NumberFormatException e) {
valid = false;
}
}
if (valid) {
xInput.setValue(split[0]);
yInput.setValue(split[1]);
zInput.setValue(split[2]);
return true;
}
}
}
}
....
}
for (List<ScheduleWaitCondition> list : entry.conditions) {
int maxWidth = getConditionColumnWidth(list);
for (int i = 0; i < list.size(); i++) {
ScheduleWaitCondition scheduleWaitCondition = list.get(i);
Math.max(maxWidth, renderInput(....)); // <=
scheduleWaitCondition.renderSpecialIcon(....);
}
AllGuiTextures.SCHEDULE_CONDITION_APPEND.render(
graphics,
xOffset + (maxWidth - 10) / 2,
29 + list.size() * 18
);
xOffset += maxWidth + 10;
}
protected void continueIfAllPrecedingFinished() {
List<MechanicalCrafterBlockEntity> preceding =
RecipeGridHandler.getPrecedingCrafters(this);
if (preceding == null) { // <=
ejectWholeGrid();
return;
}
for (MechanicalCrafterBlockEntity blockEntity : preceding)
if (blockEntity.phase != Phase.WAITING)
return;
phase = Phase.ASSEMBLING;
countDown = 1;
}
public static List<MechanicalCrafterBlockEntity> getPrecedingCrafters(
MechanicalCrafterBlockEntity crafter
) {
BlockPos pos = crafter.getBlockPos();
Level world = crafter.getLevel();
List<MechanicalCrafterBlockEntity> crafters = new ArrayList<>();
BlockState blockState = crafter.getBlockState();
if (!isCrafter(blockState))
return crafters;
....
return crafters;
}
public void handle(ServerPlayer sender) {
Train train = Create.RAILWAYS.trains.get(trainId);
Entity entity = sender.level().getEntity(entityId);
String messagePrefix = sender.getName()
.getString() + " could not relocate Train ";
if (
train == null || // <=
!(entity instanceof CarriageContraptionEntity cce)
) {
Create.LOGGER.warn(messagePrefix + train.id.toString()
.substring(0, 5) + ": not present on server");
return;
}
if (!train.id.equals(cce.trainId))
return;
....
}
messagePrefix + trainId.toString().substring(0, 5)
protected List<Recipe<?>> getMatchingRecipes() {
List<Recipe<?>> matchingRecipes = super.getMatchingRecipes();
if (!AllConfigs.server().recipes.allowBrewingInMixer.get())
return matchingRecipes;
Optional<BasinBlockEntity> basin = getBasin();
if (!basin.isPresent())
return matchingRecipes;
BasinBlockEntity basinBlockEntity = basin.get();
if (basin.isEmpty()) // <=
return matchingRecipes;
IItemHandler availableItems = level.getCapability(
Capabilities.ItemHandler.BLOCK,
basinBlockEntity.getBlockPos(), null
);
if (availableItems == null)
return matchingRecipes;
for (int i = 0; i < availableItems.getSlots(); i++) {
....
}
return matchingRecipes;
}
public static Contraption fromNBT(Level world, CompoundTag nbt,
boolean spawnData) {
String type = nbt.getString("Type");
Contraption contraption = ContraptionType.fromType(type);
contraption.readNBT(world, nbt, spawnData);
....
}
/**
* Lookup the ContraptionType with the given ID,
* and create a new Contraption from it if present.
* If it doesn't exist, returns null.
*/
@Nullable
public static Contraption fromType(String typeId) {
ContraptionType legacy = AllContraptionTypes.BY_LEGACY_NAME
.get(typeId);
if (legacy != null) {
return legacy.factory.get();
}
ResourceLocation id = ResourceLocation.tryParse(typeId);
ContraptionType type = CreateBuiltInRegistries.CONTRAPTION_TYPE.get(id);
return type == null ? null : type.factory.get();
}
/give @p
create:minecart_contraption[create:minecart_contraption_data=
{Type:"create:does_not_exist",InitialOrientation:"north"}]
Contraption contraption = ContraptionType.fromType(type);
if (contraption == null) {
Create.LOGGER.warn("Unknown contraption type: {}", type);
return null;
}
public void tick() {
if (!isLagging || tickTime <= 0)
return;
try {
Thread.sleep(tickTime);
} catch (InterruptedException e) {
e.printStackTrace();
}
}
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
}
댓글 (0)
로그인하면 이 기사에 내 생각을 남길 수 있어요