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

Using Base64 instead of DatatypeConverter #318

Conversation

azagniotov
Copy link

@azagniotov azagniotov commented Mar 29, 2018

This PR addresses issues #317 & #297 and #143 (points out a significant difference between the DatatypeConverter & Base64)

Replacing usages of javax/xml/bind/DatatypeConverter with java.util.Base64

Please note

This PR removes support for building the library against Java 7 as it leverages java.util.Base64 which was added in Java 8. I do not know what the maintainers of jjwt have in mind in terms of sunsetting support for Java 7, so I am not sure if my PR is the right direction. If support for for Java 7 will be dropped, then the issue #308 will be addressed too

As an alternative, the following PR #283 tries to solve this issue by adding a dependency on jaxb-api, which brings in DatatypeConverter without dropping the Java 7 build support

Replacing usages of `javax/xml/bind/DatatypeConverter` with ` java.util.Base64`
@azagniotov azagniotov force-pushed the issue_317_replace_datatype_converter branch from e555bff to 64f258a Compare March 29, 2018 18:06
@coveralls
Copy link

coveralls commented Mar 29, 2018

Coverage Status

Coverage remained the same at 100.0% when pulling 801ff37 on azagniotov:issue_317_replace_datatype_converter into 44faaca on jwtk:master.

@@ -4,7 +4,6 @@ dist: trusty
sudo: required
language: java
jdk:
- openjdk7
- oraclejdk8
- oraclejdk9

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add oraclejdk10 and oraclejdk11 here would be good. We can allow failures for oraclejdk11 by adding the following configuration:

matrix:
  allow_failures:
    - jdk: oraclejdk11

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried adding oraclejdk10 and oraclejdk11, but build failed on Travis when running tests against these JDKs with NPE (https://travis-ci.org/jwtk/jjwt/builds/398495426).

For now, I removed the above changes. If/when the current PR gets merged, we can issue a separate PR to add the oraclejdk10 and oraclejdk11 to Travis config

@azagniotov azagniotov force-pushed the issue_317_replace_datatype_converter branch from 801ff37 to 64f258a Compare June 30, 2018 03:17
@lhazlewood
Copy link
Contributor

lhazlewood commented Jul 5, 2018

JDK 7 may be dropped in the 1.0 final release, but until then, I think we need to continue support for JDK 7.

Based on this, are you willing to re-issue the PR and pull in the jaxb lib as a dependency for JDK 7 environments?

@azagniotov
Copy link
Author

@lhazlewood I can do that. Thanks

@lhazlewood
Copy link
Contributor

@azagniotov please disregard my previous comment - check out #333 for the latest on how we're trying to solve this.

@lhazlewood
Copy link
Contributor

Closing in favor of #341. Thanks for your continued help and review on this @azagniotov !

@lhazlewood lhazlewood closed this Jul 9, 2018
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 this pull request may close these issues.

None yet

4 participants