Skip to content

Conversation

timazhum
Copy link
Contributor

Fixing the issue raised in #62

Problem: auth key with leading / trailing whitespaces is throwing exceptions. For example, assigning the secret via echo leaves a trailing \n.

Solution: strip input auth key as an input sanitization procedure

Notice: Java 8 does not support String.strip(), while String.trim() does not support Unicode whitespaces. Therefore, using regex expressions for stripping.

@JanEbbing
Copy link
Member

Oh, this looks very nice, thanks for that! I will review on monday

Copy link
Member

@JanEbbing JanEbbing left a comment

Choose a reason for hiding this comment

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

Fundamentally looks good to me, thanks :)

I'll just get a second opinion from a colleague by tomorrow.

@timazhum
Copy link
Contributor Author

Amended the commit, using trim() instead. Also updated a null/empty string checker to match with C# implementation (https://github.com/DeepLcom/deepl-dotnet/blob/main/DeepL/Translator.cs#L438)

@JanEbbing
Copy link
Member

Happy to merge this once Leon's comment is addressed :) thanks for your work!

Fixing the issue raised in DeepLcom#62

Problem: auth key with leading / trailing whitespaces is throwing exceptions. For example, assigning the secret via `echo` leaves a trailing `\n`.

Solution: strip input auth key as an input sanitization procedure

Notice: Java 8 does not support `String.strip()`, while `String.trim()` does not support Unicode whitespaces. We anticipate ASCII only input and utilizing `trim()` for simplicity.
@timazhum
Copy link
Contributor Author

Happy to merge this once Leon's comment is addressed :) thanks for your work!

✅ Addressed, thanks for all the feedback provided

@daniel-jones-dev daniel-jones-dev self-requested a review June 27, 2025 09:49
@daniel-jones-dev daniel-jones-dev merged commit b48baec into DeepLcom:main Jun 27, 2025
4 checks passed
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.

4 participants