Java bug digest: parsing untrusted numeric input

Overview

This digest outlines some of the most interesting or impactful bugs encountered by our AI assistant over the past period. Our goal is to highlight issues that can inform and improve future development by learning from past challenges.

Findings

Bug 1

Description

In a recipe rating system, there’s a bug in the method that processes user ratings. The code assumes the AI-powered rating generator always returns a valid numeric string, but it might return non-numeric data, potentially causing a crash.

Solution

To fix this issue in the processRating method, add error handling to catch potential NumberFormatException:

private Recipe processRating(Recipe recipe) {
    try {
        float ratingValue = Float.parseFloat(aiRatingGenerator.generateRating(recipe.getIngredients()));
        recipe.setRating(ratingValue == 0 ? 0 : ratingValue / 5);
    } catch (NumberFormatException e) {
        // Log the error and set a default rating, or handle it as appropriate for your use case
        recipe.setRating(0);
    }
    return recipe;
}

Key Takeaway

Always validate and handle potential errors when parsing string inputs to numeric types, especially when dealing with external data sources or AI-generated content.

Bug 2

Description

In a library management system, there’s a bug in the method that retrieves book categories. The current implementation might inadvertently lose some categories due to an inconsistent mapping process. Here’s a problematic piece of code demonstrating this issue:

public List<String> getBookCategories(Library library) {
    return libraryClient.getCategories(library.getId())
        .stream().map(c -> categoryToDisplayName.getOrDefault(c, c))
        .collect(toList());
}

Solution

To fix this bug and ensure all categories are properly handled, modify the getBookCategories method as follows:

public List<String> getBookCategories(Library library) {
    return libraryClient.getCategories(library.getId())
        .stream().map(c -> {
            String displayName = categoryToDisplayName.get(c);
            if (displayName == null) {
                logger.warn("Unrecognized book category: " + c);
                return "Miscellaneous";
            }
            return displayName;
        })
        .collect(toList());
}

Key Takeaway

When mapping between different representations of data, always handle cases where the mapping is undefined to prevent data loss and maintain consistency throughout the application.

Bug 3

Description

In a weather data processing system, there’s a bug in the extractTemperatureReading() method. This method attempts to extract valid temperature readings from a list of sensor data. The bug can cause an infinite loop when processing invalid or incomplete data.

Solution

To fix the bug in the extractTemperatureReading() method, modify the while loop condition to check if there’s enough data remaining for a complete temperature reading:

while (sensorData.size() >= TEMPERATURE_READING_LENGTH) {
    List<Integer> chunk = sensorData.subList(0, TEMPERATURE_READING_LENGTH);
    if (isValidTemperatureReading(chunk)) {
        return List.copyOf(chunk);
    }
    LOG.warn("Invalid temperature reading {}, skipping to next", chunk);
    sensorData.remove(0);
    alignSensorDataToNextValidStart();
    if (sensorData.size() < TEMPERATURE_READING_LENGTH) {
        break;
    }
}

This ensures the loop exits when there’s insufficient data left to form a complete temperature reading, preventing an infinite loop.

Key Takeaway

When processing data in chunks, always ensure your loop conditions account for the remaining data size to prevent infinite loops, especially when removing or skipping data.

Bug 4

Description

In a weather monitoring system, there’s a bug in the WeatherStationConfig class which implements the SensorDataConfig interface. The getConfigPrefix() and getConfigValue(String key) methods are incorrectly implemented, always returning null.

Solution

To fix this issue, implement these methods properly based on your weather station’s requirements. For example:

@Override
public String getConfigPrefix() {
    return "weather-station";
}

@Override
public String getConfigValue(String key) {
    if ("measurement-interval".equals(key)) {
        return "5m";
    }
    if ("temperature-unit".equals(key)) {
        return "celsius";
    }
    return null; // or throw an exception for unknown keys
}

Adjust the implementation to match your specific weather station configuration needs.

Key Takeaway

When implementing an interface, ensure all methods are properly implemented with meaningful return values to avoid unexpected behavior or errors in the system.

Bug 5

Description

In a weather data processing system, a WeatherReport class contains methods to retrieve various weather attributes. However, these getter methods handle null values inconsistently, potentially leading to unexpected behavior or null pointer exceptions.

Solution

To address the inconsistent null handling, we can use Java’s Optional class to process potentially null values uniformly across all getter methods. Here’s an example of how to refactor the getTemperature() method:

public String getTemperature() {
    return Optional.ofNullable(temperature)
        .map(WeatherUtil::formatTemperature)
        .orElse("");
}

Apply this same pattern to getHumidity(), getWindSpeed(), and getPrecipitation() methods for consistent null handling throughout the class.

Key Takeaway

When dealing with potentially null values in getter methods, consider using Optional to handle nulls consistently and safely, reducing the risk of null pointer exceptions and improving code readability.

Bug 6

Description

In a file storage system for a recipe sharing application, there’s a bug in the method responsible for saving new recipes. The code doesn’t properly handle potential exceptions when writing to the storage, which could lead to resource leaks.

Solution

To fix the bug in the saveNewRecipe method, wrap the storage operation in a try-catch block. This ensures that any exceptions during the write process are caught and handled appropriately. Here’s how you can modify the code:

public void saveNewRecipe(String userId, List<Recipe> recipes) {
    for (Recipe recipe : recipes) {
        String recipeId = generateUniqueId();
        String filePath = userId + "/" + recipeId + ".json";

        StorageRequest storageRequest = StorageRequest.builder()
                .container(RECIPE_CONTAINER)
                .metadata(Map.of("category", recipe.getCategory()))
                .path(filePath)
                .build();

        try {
            storageClient.writeFile(storageRequest, convertToJson(recipe));
        } catch (Exception e) {
            throw new StorageException("Failed to save recipe: " + filePath + "\n" + e.getMessage());
        }
    }
}

Key Takeaway

Always handle potential exceptions in I/O operations, especially when dealing with external storage systems. This practice prevents resource leaks and allows for proper error handling and reporting.

Bug 7

Description

The following code snippet demonstrates a common misuse of Lombok annotations in a product inventory system. The ProductDTO class and its nested DetailsDTO class both use the @RequiredArgsConstructor annotation incorrectly.

import lombok.Getter;
import lombok.RequiredArgsConstructor;

@Getter
@RequiredArgsConstructor
public class ProductDTO {
    private String sku;
    private DetailsDTO details;

    @Getter
    @RequiredArgsConstructor
    public static class DetailsDTO {
        private String description;
        private double price;
    }
}

Solution

Remove the @RequiredArgsConstructor annotation from both ProductDTO and DetailsDTO classes, as it’s not effective without any final fields. If you intend to have a no-args constructor, use @NoArgsConstructor instead. Alternatively, if the fields are meant to be immutable, mark them as final. For example:

import lombok.Getter;
import lombok.NoArgsConstructor;

@Getter
@NoArgsConstructor
public class ProductDTO {
    private String sku;
    private DetailsDTO details;

    @Getter
    @NoArgsConstructor
    public static class DetailsDTO {
        private String description;
        private double price;
    }
}

Key Takeaway

When using Lombok’s @RequiredArgsConstructor, ensure that the class has at least one final field; otherwise, the annotation won’t generate the intended constructor. Always review your Lombok annotations to confirm they align with your class design and requirements.

Bug 8

Description

In a weather forecasting application, a bug exists in the method responsible for closing weather alert pop-ups. The code attempts to hide the alert window twice, which is unnecessary and potentially problematic.

Solution

To fix this issue in the dismissWeatherAlert method, remove the redundant call to hide the alert window. The corrected code should look like this:

if (alertWindow != null && alertWindow.isVisible()) {
  try {
    alertWindow.hide();
  } catch (Exception e) {
    LOGGER.error("Failed to close weather alert: {}, Error: {}", alertType, e.getMessage());
  }
  // Remove this line:
  // alertWindow.hide();
} else {
  LOGGER.info("No visible alert window for alert type: {}", alertType);
}

This change ensures that the hide() method is called only once, preventing any potential issues that could arise from attempting to hide an already hidden window.

Key Takeaway

Always review your code for redundant operations, especially when dealing with UI elements. Unnecessary method calls can lead to unexpected behavior and reduced performance.

Bug 9

Description

In a weather forecasting application, there’s a bug in the extractTemperature method. The method creates an unnecessary WeatherData object that is never used, leading to potential performance issues and code clutter.

Solution

Remove the following line from the extractTemperature method:

WeatherData data = new WeatherData();

This object is not used in the method and can be safely removed without affecting the functionality. The method should focus solely on extracting and returning the temperature value from the JSON response.

Key Takeaway

Always review your code for unused variables or objects. Removing unnecessary instantiations can improve code readability and performance, especially in methods that are called frequently.

Conclusions

Thank you for reading this Mavka Digest! Stay tuned for more insights and updates on code reviews, top comments, and useful development tips. Subscribe to keep up with our weekly, monthly, and yearly digests, and make sure not to miss out on the latest from Mavka. Follow us for continuous learning and improvement in your coding journey!