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

Added error handling if resumption token is not a valid base64 string #272

Merged
merged 3 commits into from
Oct 18, 2024

Conversation

bumann-sbb
Copy link

When sending an invalid resumption token (not a valid base64 encoded String) the library throws an unhandled exception (IllegalArgumentException).
This change handles this exception and throws a BadResumptionTokenException and thus produces a valid OAI PMH response.

@bumann-sbb
Copy link
Author

The build apparently does not work because I'm working on a fork.

@pdurbin
Copy link
Member

pdurbin commented Oct 1, 2024

Yes, apparently so. I opened an issue about this:

@bumann-sbb
Copy link
Author

Any updates on this? Is there a different way I can contribute to avoid this error?

@pdurbin
Copy link
Member

pdurbin commented Oct 15, 2024

@bumann-sbb no, you're doing everything right. We've just been busy! 😅 Thanks for your patience!

- Use a parameterized test with format.parse(), not the lower level API call, making it more aligned to the pre-existing tests.
- Add a parsing test with a string that contains UTF-8 chars to ensure encoding and decoding those works and don't provoke failures.
Copy link
Member

@poikilotherm poikilotherm left a comment

Choose a reason for hiding this comment

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

Hi @bumann-sbb !

Thanks for the PR, good catch! 🫶

I refactored the test a bit and will merge now. After I merged some of the open PRs by Renovate bot, I'm going to cut a 5.2.2 release, should be available soon. You can obviously already test using the 5.2.2-SNAPSHOT.

Cheers!

@poikilotherm poikilotherm merged commit 057f79b into gdcc:branch-5.0 Oct 18, 2024
1 of 2 checks passed
@poikilotherm
Copy link
Member

5.2.2 has been released, should be available soon from Central.

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.

3 participants