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

3.8.0 introduced a breaking change #507

Closed
ebickle opened this issue Aug 17, 2021 · 4 comments · Fixed by #513
Closed

3.8.0 introduced a breaking change #507

ebickle opened this issue Aug 17, 2021 · 4 comments · Fixed by #513
Labels
bug This points to a verified bug in the code

Comments

@ebickle
Copy link

ebickle commented Aug 17, 2021

Describe the problem

Version 3.8.0 of the library introduced a breaking change that broke binary compatibility. The method definition of com.auth0.jwt.interfaces.Verification.withAudience changed from (String) to (String[]) when the parameter was modified from being a regular String to using a varargs.

The breaking change is not documented.

What was the expected behavior?

Upgrading to a newer minor version of the library should not cause a breaking change in binary compatibility. Alternately, the breaking change should be documented.

The library should continue to have a proper withAudience(String) overload (or equivalent) to avoid the breaking change (assuming Java permits the overload alongside a varargs).

Reproduction

N/A. Breaking change can be seen in source for PR #246

Environment

  • Version of this library used: 3.18.1 (upgrading from 3.3.0)
  • Version of Java used: Java 8 (OpenJDK variant)
@jimmyjames
Copy link
Contributor

Thanks @ebickle for raising this. You are correct that no breaking changes should be included in a minor release; and if it needs to be done for some reason, it should be documented. I suspect this one got through because it may seem ok, but is indeed a binary incompatible change. We have added checks to our build pipeline to catch these sort of issues to prevent unintended breaking changes, but that was done after this change got through.

I'll see if there's a non-breaking way to restore the original method, though it may also be an issue as we'd need to add a new method to a public interface. If it's not possible, we'll retroactively update the changelog for 3.8.0 (obviously not ideal, we'll see what we can do).

@jimmyjames jimmyjames added the bug This points to a verified bug in the code label Aug 30, 2021
@jimmyjames
Copy link
Contributor

@ebickle I assume you mean the change to withIssuer, not withAudience, as made in #288?

@lbalmaceda lbalmaceda linked a pull request Sep 7, 2021 that will close this issue
@jimmyjames
Copy link
Contributor

@ebickle I have a PR to restore withIssuer(String issuer) in #513. I'll keep it open for a couple days if you would like to review/try it. Thanks!

@ebickle
Copy link
Author

ebickle commented Sep 7, 2021

@jimmyjames Good catch, looks like I made a mistake in the original issue report. withAudience took a String... parameter in 3.3.0 and is the same in 3.8.0. withIssuer is the method that changed between the two.

Our build issue showed up because of some sort of unclean build or caching issue where a class was being deployed with a new library instead of being recompiled as it normally is. We've resolved that and aren't blocked on this issue any longer - but the PR will fix the binary compatibility for others, so probably worth doing regardless 😀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This points to a verified bug in the code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants