Skip to content

UTF-8 fixes #277

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.

Sign up for GitHub

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

Merged
dpharris merged 3 commits into from
Nov 19, 2024
Merged

UTF-8 fixes #277

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
15c1b30
properly create UTF-8 strings from input
bobjacobsen Nov 5, 2024
aafc4c0
force use of Dialog font for CDI tabs
bobjacobsen Nov 5, 2024
4978d9c
provide stand-in with error
bobjacobsen Nov 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 19 additions & 7 deletions src/org/openlcb/cdi/jdom/CdiMemConfigReader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

package org.openlcb.cdi.jdom;

import java.util.ArrayList;
import java.util.logging.Level;
import java.util.logging.Logger;
import static java.util.logging.Logger.getLogger;
Expand Down Expand Up @@ -42,19 +43,19 @@ public CdiMemConfigReader(NodeID node, OlcbInterface iface, int space) {


long nextAddress = 0;
StringBuffer buf;
ArrayList buf = new ArrayList();

ReaderAccess retval;
public void startLoadReader(ReaderAccess retval) {
this.retval = retval;
nextAddress = 0;
buf = new StringBuffer();
buf = new ArrayList();
nextRequest();
}

void nextRequest() {
if (retval != null) {
retval.progressNotify(buf.length(), -1);
retval.progressNotify(buf.size(), -1);
}
MemoryConfigurationService.McsReadHandler memo =
new MemoryConfigurationService.McsReadHandler() {
Expand Down Expand Up @@ -92,7 +93,7 @@ public void handleReadData(NodeID dest, int space, long address, byte[] data) {
done();
return; // don't do next request
}
buf.append((char)data[i]);
buf.add(data[i]);
}
// repeat if not done
nextAddress = nextAddress + LENGTH;
Expand All @@ -105,9 +106,20 @@ public void handleReadData(NodeID dest, int space, long address, byte[] data) {
private void done() {
// done, pass back a reader based on the current buffer contents
if (retval != null) {
retval.progressNotify(buf.length(), buf.length());
logger.log(Level.FINE, "Retrieved XML: \n{0}", buf);
retval.provideReader(new java.io.StringReader(new String(buf)));
retval.progressNotify(buf.size(), buf.size());
byte[] byteArray = new byte[buf.size()];
for (int i = 0 ; i<buf.size(); i++) {
byteArray[i] = (byte)buf.get(i);
}
try {
String xml = new String(byteArray, "UTF-8");
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

@balazsracz balazsracz Nov 5, 2024

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

@bobjacobsen bobjacobsen Nov 5, 2024

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

@balazsracz balazsracz Nov 5, 2024

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

@balazsracz balazsracz Nov 5, 2024

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.

// provide a stand-in
retval.provideReader(new java.io.StringReader(""));
}
}
}

Expand Down
17 changes: 15 additions & 2 deletions src/org/openlcb/cdi/swing/CdiPanel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,10 +149,22 @@ public class CdiPanel extends JPanel {
private List<util.CollapsiblePanel> navPanels = new ArrayList<>();
private final Color COLOR_BACKGROUND;
private CollapsiblePanel sensorHelperPanel;
/// Panel at the bottom of the window with command buttons.
//private JPanel bottomPanel;
/// To get focus to the bottom panel, this component needs to be activated.
private JComponent bottomPanelHead;

static private Font tabFont; // this will be referenced in getTabLabel below
static {
// ensure that JTabbedPane pane labels use
// the specific Dialog font
int size = 12;
Font defaultTabbedFont = UIManager.getFont("TabbedPane.font");
if (defaultTabbedFont != null) {
size = defaultTabbedFont.getSize();
} else {
logger.log(Level.WARNING, "Did not find default TabbedPane font, please report this.");
}
tabFont = new Font("Dialog", Font.PLAIN, size);
}

public CdiPanel () {
super();
Expand Down Expand Up @@ -1190,6 +1202,7 @@ public void propertyChange(PropertyChangeEvent event) {
*/
protected JComponent getTabLabel(JTabbedPane parentTabbedPane, int index, String name, ConfigRepresentation.GroupRep rep) {
JLabel tabLabel = new JLabel(name);
tabLabel.setFont(tabFont);

tabLabel.addMouseListener(new MouseAdapter() {

Expand Down