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

False positive SE_PREVENT_EXT_OBJ_OVERWRITE #2750

Closed
rovarga opened this issue Dec 7, 2023 · 1 comment · Fixed by #2762
Closed

False positive SE_PREVENT_EXT_OBJ_OVERWRITE #2750

rovarga opened this issue Dec 7, 2023 · 1 comment · Fixed by #2762

Comments

@rovarga
Copy link
Contributor

rovarga commented Dec 7, 2023

The logic to detect readExternal() guards introduced in #2426 requires a very strict shape of the guard.

The first example is:

class Foo implements Externalizable {
  private Object result;
  private boolean initialized;

  @Override
  public final void readExternal(ObjectInput in) throws IOException, ClassNotFoundException {
    if (initialized) {
      throw new IllegalStateException("already initialized");
    }

    // stuff and then ...
    result = new Object();
    initialized = true;
  }

  @java.io.Serial
  final Object readResolve() {
    return requireNonNull(result);
  }
}

This differs from GoodExternalizableTest only in the bytecode shape, code reachability remains exactly the same.

A second example, common when using Externalizable Serialization Proxies, is:

class Bar implements Externalizable {
  private Object result = null;

  @Override
  public final void readExternal(ObjectInput in) throws IOException, ClassNotFoundException {
    if (result == null) {
      // stuff and then ...
      result = new Object();
    } else {
      throw new IllegalStateException("already initialized");
    }
  }

  @java.io.Serial
  final Object readResolve() {
    return requireNonNull(result);
  }
}

Here the difference is the use of IFNULL/IFNONNULL instead of IFEQ/IFNE to check state.

@PatrikScully
Copy link
Contributor

Hello! I'll check it early next week.

PatrikScully added a commit to PatrikScully/spotbugs that referenced this issue Dec 11, 2023
hazendaz added a commit to PatrikScully/spotbugs that referenced this issue Dec 12, 2023
hazendaz added a commit that referenced this issue Dec 12, 2023
* #2750 Enhance readExternal detector

* Add changelog entry

* Add more test cases

* Add supporting for complex if statements

---------

Co-authored-by: Jeremy Landis <jeremylandis@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants