Skip to content

Conversation

@bobjacobsen
Copy link
Contributor

@bobjacobsen bobjacobsen commented Nov 5, 2024

  • Properly handle the byte array to UTF-8 String transformation
  • Force use of Dialog font for CDI tab labels

Note: this should be merged after #275

logger.log(Level.FINE, "Retrieved XML: \n{0}", xml);
retval.provideReader(new java.io.StringReader(xml));
} catch (java.io.UnsupportedEncodingException e) {
logger.warning("UnsupportedEncodingException while preparing XML data");
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should provide some string back even in the error case. It could be an empty string. Otherwise the UI will get stuck.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is one of those "can't realistically happen, but has to be caught" exceptions. It'll only fire if the new String(byteArray, "UTF-8") expression decides that "UTF-8" is not a valid encoding. If that happens, there's something broken deep within Java.

That said, we could still provide something in retval. I'm just not sure what that content should be. An empty string? A minimal CDI with no content inside the <cdi> element?

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok i misunderstood and thought that broken input utf8 tokens will also cause this error path.
Maybe we should just have a finally { provide(new StringREader("")) }

Copy link
Collaborator

Choose a reason for hiding this comment

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

actually finally {} is a bad idea. I take that back.

But providing an empty string in the error path is reasonable.

@dpharris dpharris merged commit 0f3c3e8 into openlcb:master Nov 19, 2024
@bobjacobsen bobjacobsen deleted the utf-8-fixes branch January 27, 2025 11:43
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