Hi Gerd Attached patch
- uses StandardCharsets.* where possible. - notes some usage of the java local DefaultCharset. - changed a couple of these to force utf-8 instead. - if --read-config file gives decoding errors, names the charset used to read the file (ie DefaultCharset) instead of 'utf-8' in the error message. - accepts/ignores unicode BOM in more files - uses try (open...) {} where possible in files changed for the above reasons. There is some code in mkgmap/srt/SrtTextReader.java:sortForCodepage() that I don't understand; it would appear to get into a recursive loop on IOException. Ticker On Tue, 2020-01-14 at 09:55 +0000, Gerd Petermann wrote: > Hi Ticker, > > yes, and every missing close() is a brain teaser ;) > We have a few places where files are opened and closed in a different > method. This is likely to cause trouble in unit tests, esp. on > Windows. > Whereever possible we should use try-with-ressources instead of > Utils.closeFile() and add a comment > like in SeaGenerator line > in zipFile = new ZipFile(precompSeaDir); // don't close here! > when a file is intentionally kept open. > > Gerd > > ________________________________________ > Von: mkgmap-dev <mkgmap-dev-boun...@lists.mkgmap.org.uk> im Auftrag > von Ticker Berkin <rwb-mkg...@jagit.co.uk> > Gesendet: Dienstag, 14. Januar 2020 10:43 > An: Development list for mkgmap > Betreff: Re: [mkgmap-dev] TYP files and character encoding > > Hi Gerd > > Here is updated patch that closes the file, although I find many > files > in mkgmap that don't have explicit close(), but I presume .finalize() > will close them eventually. > > I'll do another patch for other text file handling, using > StandardCharset where possible and fixing TokenScanner message for > bad > characters if not utf-8 and, if reasonable, allowing a BOM even if > the > file is opened as utf-8 anyway. > > Ticker > > On Tue, 2020-01-14 at 08:21 +0000, Gerd Petermann wrote: > > Hi Ticker, > > > > thanks for the patch. > > > > Please review TypCompiler.CharsetProbe. BufferedReader br is not > > closed. Is that intended? > > > > I see that we have a mix of "utf-8" and "UTF-8" in the mkgmap > > sources. I think it would be good to use StandardCharsets.UTF_8 > > where > > possible > > and unify the rest.
Index: src/uk/me/parabola/imgfmt/app/labelenc/TableTransliterator.java =================================================================== --- src/uk/me/parabola/imgfmt/app/labelenc/TableTransliterator.java (revision 4413) +++ src/uk/me/parabola/imgfmt/app/labelenc/TableTransliterator.java (working copy) @@ -17,6 +17,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; +import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.Locale; @@ -115,13 +116,11 @@ } private void readCharFile(String name, String[] newRow) { - InputStream is = getClass().getResourceAsStream(name); - if (is == null) - return; + try (InputStream is = getClass().getResourceAsStream(name)) { + if (is == null) + return; + BufferedReader br = new BufferedReader(new InputStreamReader(is, StandardCharsets.UTF_8)); - try { - BufferedReader br = new BufferedReader(new InputStreamReader(is, "utf-8")); - String line; while ((line = br.readLine()) != null) { line = line.trim(); Index: src/uk/me/parabola/imgfmt/app/labelenc/Utf8Decoder.java =================================================================== --- src/uk/me/parabola/imgfmt/app/labelenc/Utf8Decoder.java (revision 4413) +++ src/uk/me/parabola/imgfmt/app/labelenc/Utf8Decoder.java (working copy) @@ -18,6 +18,7 @@ import java.io.ByteArrayOutputStream; import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; /** * Decoder for labels in utf-8, note that I am not actually sure that any @@ -29,7 +30,7 @@ private final ByteArrayOutputStream out = new ByteArrayOutputStream(); private boolean needreset; - private final Charset charset = Charset.forName("utf-8"); + private final Charset charset = StandardCharsets.UTF_8; /** * Add a byte to this decoder. This will be saved until a complete Index: src/uk/me/parabola/imgfmt/app/labelenc/Utf8Encoder.java =================================================================== --- src/uk/me/parabola/imgfmt/app/labelenc/Utf8Encoder.java (revision 4413) +++ src/uk/me/parabola/imgfmt/app/labelenc/Utf8Encoder.java (working copy) @@ -18,6 +18,7 @@ import java.io.UnsupportedEncodingException; import java.util.Locale; +import java.nio.charset.StandardCharsets; /** * Encoder for labels in utf-8. @@ -37,17 +38,11 @@ uctext = text; EncodedText et; - try { - byte[] buf = uctext.getBytes("utf-8"); - byte[] res = new byte[buf.length + 1]; - System.arraycopy(buf, 0, res, 0, buf.length); - res[buf.length] = 0; - et = new EncodedText(res, res.length, uctext.toCharArray()); - } catch (UnsupportedEncodingException e) { - // As utf-8 must be supported, this can't happen - byte[] buf = uctext.getBytes(); - et = new EncodedText(buf, buf.length, uctext.toCharArray()); - } + byte[] buf = uctext.getBytes(StandardCharsets.UTF_8); + byte[] res = new byte[buf.length + 1]; + System.arraycopy(buf, 0, res, 0, buf.length); + res[buf.length] = 0; + et = new EncodedText(res, res.length, uctext.toCharArray()); return et; } } Index: src/uk/me/parabola/imgfmt/app/srt/Sort.java =================================================================== --- src/uk/me/parabola/imgfmt/app/srt/Sort.java (revision 4413) +++ src/uk/me/parabola/imgfmt/app/srt/Sort.java (working copy) @@ -19,6 +19,7 @@ import java.nio.charset.Charset; import java.nio.charset.CharsetEncoder; import java.nio.charset.CodingErrorAction; +import java.nio.charset.StandardCharsets; import java.text.CollationKey; import java.text.Collator; import java.util.ArrayList; @@ -546,10 +547,10 @@ Charset charset; switch (codepage) { case 0: - charset = Charset.forName("ascii"); + charset = StandardCharsets.US_ASCII; break; case 65001: - charset = Charset.forName("UTF-8"); + charset = StandardCharsets.UTF_8; break; case 932: // Java uses "ms932" for code page 932 Index: src/uk/me/parabola/mkgmap/Options.java =================================================================== --- src/uk/me/parabola/mkgmap/Options.java (revision 4413) +++ src/uk/me/parabola/mkgmap/Options.java (working copy) @@ -18,7 +18,8 @@ import java.io.BufferedReader; import java.io.File; -import java.io.FileReader; +import java.io.FileInputStream; +import java.io.InputStreamReader; import java.io.IOException; import java.io.Reader; import java.util.Collection; @@ -76,7 +77,7 @@ return; } - try (Reader r = new FileReader(filename)) { + try (Reader r = new InputStreamReader(new FileInputStream(filename)/*NB: DefaultCharset*/)) { readOptionFile(r, filename); } @@ -83,9 +84,15 @@ } public void readOptionFile(Reader r, String filename) { - BufferedReader br = new BufferedReader(r); + BufferedReader br; + if (r instanceof BufferedReader) + br = (BufferedReader)r; + else + br = new BufferedReader(r); TokenScanner ts = new TokenScanner(filename, br); ts.setExtraWordChars("-"); + if (r instanceof InputStreamReader) + ts.setCharset(((InputStreamReader)r).getEncoding()); File file = new File(filename); String parent = file.getParent(); Index: src/uk/me/parabola/mkgmap/main/Main.java =================================================================== --- src/uk/me/parabola/mkgmap/main/Main.java (revision 4413) +++ src/uk/me/parabola/mkgmap/main/Main.java (working copy) @@ -19,6 +19,7 @@ import java.io.InputStream; import java.io.InputStreamReader; import java.io.PrintStream; +import java.nio.charset.StandardCharsets; import java.lang.management.ManagementFactory; import java.lang.management.MemoryPoolMXBean; import java.lang.management.MemoryType; @@ -201,19 +202,18 @@ * Grab the options help file and print it. * @param err The output print stream to write to. * @param lang A language hint. The help will be displayed in this - * language if it has been translated. + * language if it has been translated. * @param file The help file to display. */ private static void printHelp(PrintStream err, String lang, String file) { String path = "/help/" + lang + '/' + file; - InputStream stream = Main.class.getResourceAsStream(path); - if (stream == null) { - err.println("Could not find the help topic: " + file + ", sorry"); - return; - } + try (InputStream stream = Main.class.getResourceAsStream(path)) { + if (stream == null) { + err.println("Could not find the help topic: " + file + ", sorry"); + return; + } - BufferedReader r = new BufferedReader(new InputStreamReader(stream)); - try { + BufferedReader r = new BufferedReader(new InputStreamReader(stream, StandardCharsets.UTF_8)); String line; while ((line = r.readLine()) != null) err.println(line); @@ -224,13 +224,12 @@ private static Set<String> getValidOptions(PrintStream err) { String path = "/help/en/options"; - InputStream stream = Main.class.getResourceAsStream(path); - if (stream == null) - return null; + try (InputStream stream = Main.class.getResourceAsStream(path)) { + if (stream == null) + return null; - Set<String> result = new HashSet<>(); - try { - BufferedReader r = new BufferedReader(new InputStreamReader(stream, "utf-8")); + Set<String> result = new HashSet<>(); + BufferedReader r = new BufferedReader(new InputStreamReader(stream, StandardCharsets.UTF_8)); Pattern p = Pattern.compile("^--?([a-zA-Z0-9-]*).*$"); String line; @@ -241,12 +240,12 @@ result.add(opt); } } + return result; + } catch (IOException e) { - err.println("Could not read valid optoins"); + err.println("Could not read valid options"); return null; } - - return result; } public void startOptions() { Index: src/uk/me/parabola/mkgmap/main/StyleTester.java =================================================================== --- src/uk/me/parabola/mkgmap/main/StyleTester.java (revision 4413) +++ src/uk/me/parabola/mkgmap/main/StyleTester.java (working copy) @@ -15,7 +15,9 @@ import java.io.BufferedReader; import java.io.FileNotFoundException; -import java.io.FileReader; +import java.io.FileInputStream; +import java.io.InputStreamReader; +import java.nio.charset.StandardCharsets; import java.io.FileWriter; import java.io.IOException; import java.io.InputStream; @@ -221,9 +223,8 @@ * file all in one. */ public static void runSimpleTest(String filename) { - try { - FileReader reader = new FileReader(filename); - BufferedReader br = new BufferedReader(reader); + // 14Jan20 Changed from using DefaultCharset to UTF-8 + try (BufferedReader br = new BufferedReader(new InputStreamReader(new FileInputStream(filename), StandardCharsets.UTF_8))) { List<Way> ways = readSimpleTestFile(br); List<String> givenList = readGivenResults(); @@ -346,7 +347,6 @@ // ignore blank lines. }*/ } - br.close(); return ways; } Index: src/uk/me/parabola/mkgmap/osmstyle/DirectoryFileLoader.java =================================================================== --- src/uk/me/parabola/mkgmap/osmstyle/DirectoryFileLoader.java (revision 4413) +++ src/uk/me/parabola/mkgmap/osmstyle/DirectoryFileLoader.java (working copy) @@ -23,6 +23,7 @@ import java.io.InputStreamReader; import java.io.Reader; import java.io.UnsupportedEncodingException; +import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.List; @@ -54,17 +55,7 @@ * @return An open file reader for the file. */ public Reader open(String filename) throws FileNotFoundException { - File file = new File(dir, filename); - - Reader r = null; - try { - r = new InputStreamReader(new FileInputStream(file), "UTF-8"); - } catch (UnsupportedEncodingException uee) { - System.out.println("DirectoryFileLoader: Encoding UTF-8 not supported"); - r = new InputStreamReader(new FileInputStream(file)); - } - - return new BufferedReader(r); + return new BufferedReader(new InputStreamReader(new FileInputStream(new File(dir, filename)), StandardCharsets.UTF_8)); } /** Index: src/uk/me/parabola/mkgmap/osmstyle/JarFileLoader.java =================================================================== --- src/uk/me/parabola/mkgmap/osmstyle/JarFileLoader.java (revision 4413) +++ src/uk/me/parabola/mkgmap/osmstyle/JarFileLoader.java (working copy) @@ -23,6 +23,7 @@ import java.io.InputStreamReader; import java.io.Reader; import java.io.UnsupportedEncodingException; +import java.nio.charset.StandardCharsets; import java.net.JarURLConnection; import java.net.MalformedURLException; import java.net.URL; @@ -131,13 +132,7 @@ } catch (IOException e) { throw new FileNotFoundException("Could not open " + filename); } - Reader reader = null; - try { - reader = new InputStreamReader(stream, "UTF-8"); - } catch (UnsupportedEncodingException e) { - System.out.println("JarFileLoader: Encoding UTF-8 not supported"); - reader = new InputStreamReader(stream); - } + Reader reader = new InputStreamReader(stream, StandardCharsets.UTF_8); return new BufferedReader(reader); } Index: src/uk/me/parabola/mkgmap/osmstyle/PrefixSuffixFilter.java =================================================================== --- src/uk/me/parabola/mkgmap/osmstyle/PrefixSuffixFilter.java (revision 4413) +++ src/uk/me/parabola/mkgmap/osmstyle/PrefixSuffixFilter.java (working copy) @@ -16,6 +16,7 @@ import java.io.FileInputStream; import java.io.InputStreamReader; import java.io.Reader; +import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -76,7 +77,7 @@ private boolean readConfig(String cfgFile) { if (cfgFile == null) return false; - try (InputStreamReader reader = new InputStreamReader(new FileInputStream(cfgFile), "utf-8")) { + try (InputStreamReader reader = new InputStreamReader(new FileInputStream(cfgFile), StandardCharsets.UTF_8)) { readOptionFile(reader, cfgFile); return true; } catch (Exception e) { Index: src/uk/me/parabola/mkgmap/osmstyle/StyleImpl.java =================================================================== --- src/uk/me/parabola/mkgmap/osmstyle/StyleImpl.java (revision 4413) +++ src/uk/me/parabola/mkgmap/osmstyle/StyleImpl.java (working copy) @@ -24,6 +24,7 @@ import java.io.OutputStreamWriter; import java.io.Reader; import java.io.Writer; +import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -223,7 +224,7 @@ // There are a lot of tags that are used within mkgmap that try (InputStream is = this.getClass().getResourceAsStream("/styles/builtin-tag-list");) { if (is != null) { - BufferedReader br = new BufferedReader(new InputStreamReader(is)); + BufferedReader br = new BufferedReader(new InputStreamReader(is, StandardCharsets.UTF_8)); // System.out.println("Got built in list"); String line; while ((line = br.readLine()) != null) { @@ -511,16 +512,12 @@ name = "default"; if (name == null){ - StyleFileLoader loader = null; - try { - loader = StyleFileLoader.createStyleLoader(loc, null); + try (StyleFileLoader loader = StyleFileLoader.createStyleLoader(loc, null)) { int numEntries = loader.list().length; if (numEntries > 1) throw new ExitException("Style file " + loc + " contains multiple styles, use option --style to select one."); } catch (FileNotFoundException e) { throw new ExitException("Could not open style file " + loc); - } finally { - Utils.closeFile(loader); } } Index: src/uk/me/parabola/mkgmap/reader/hgt/HGTList.java =================================================================== --- src/uk/me/parabola/mkgmap/reader/hgt/HGTList.java (revision 4413) +++ src/uk/me/parabola/mkgmap/reader/hgt/HGTList.java (working copy) @@ -13,7 +13,8 @@ package uk.me.parabola.mkgmap.reader.hgt; import java.io.BufferedReader; -import java.io.FileReader; +import java.io.FileInputStream; +import java.io.InputStreamReader; import java.io.IOException; import java.io.InputStream; import java.io.RandomAccessFile; @@ -88,7 +89,8 @@ */ private static BitSet compileHGTList(String filename) throws IOException { final Pattern hgtPattern = Pattern.compile("([sSnN])(\\d{2})([eEwW])(\\d{3}).*"); - try (BufferedReader br = new BufferedReader(new FileReader(filename))) { + // maybe should be changed from using DefaultCharset to UTF-8 + try (BufferedReader br = new BufferedReader(new InputStreamReader(new FileInputStream(filename)/*NB: DefaultCharset*/))) { BitSet bs = new BitSet(180*360); String strLine; while ((strLine = br.readLine()) != null) { Index: src/uk/me/parabola/mkgmap/reader/osm/OsmMapDataSource.java =================================================================== --- src/uk/me/parabola/mkgmap/reader/osm/OsmMapDataSource.java (revision 4413) +++ src/uk/me/parabola/mkgmap/reader/osm/OsmMapDataSource.java (working copy) @@ -19,7 +19,8 @@ import java.io.BufferedReader; import java.io.File; import java.io.FileNotFoundException; -import java.io.FileReader; +import java.io.InputStreamReader; +import java.io.FileInputStream; import java.io.IOException; import java.io.InputStream; import java.nio.charset.StandardCharsets; @@ -145,8 +146,12 @@ @Override public void load(String name, boolean addBackground) throws FileNotFoundException { - InputStream is = Utils.openFile(name); - parse(is, name); + try (InputStream is = Utils.openFile(name)) { + parse(is, name); + } catch (IOException e) { + // exception thrown from implicit call to close() on resource variable 'is' + } + elementSaver.finishLoading(); osmReadingHooks.end(); @@ -289,11 +294,19 @@ private static Map<String, Set<String>> readDeleteTagsFile(String fileName) { Map<String, Set<String>> deletedTags = new HashMap<>(); - try (BufferedReader br = new BufferedReader(new FileReader(fileName))) { + + // 14Jan20 Changed from using DefaultCharset to UTF-8 + try (BufferedReader br = new BufferedReader(new InputStreamReader(new FileInputStream(fileName), StandardCharsets.UTF_8))) { String line; - while((line = br.readLine()) != null) { - line = line.trim(); - if(line.length() > 0 && !line.startsWith("#") && !line.startsWith(";")) { + while ((line = br.readLine()) != null) { + if (line.isEmpty()) + continue; + if (line.charAt(0) == '\uFEFF') { // BOM + line = line.substring(1); + if (line.isEmpty()) + continue; + } + if (!line.startsWith("#") && !line.startsWith(";")) { String[] parts = line.split("="); if (parts.length == 2) { parts[0] = parts[0].trim(); Index: src/uk/me/parabola/mkgmap/reader/polish/PolishMapDataSource.java =================================================================== --- src/uk/me/parabola/mkgmap/reader/polish/PolishMapDataSource.java (revision 4413) +++ src/uk/me/parabola/mkgmap/reader/polish/PolishMapDataSource.java (working copy) @@ -28,6 +28,7 @@ import java.nio.charset.Charset; import java.nio.charset.CharsetDecoder; import java.nio.charset.CodingErrorAction; +import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.HashMap; import java.util.LinkedHashMap; @@ -76,7 +77,7 @@ public class PolishMapDataSource extends MapperBasedMapDataSource implements LoadableMapDataSource { private static final Logger log = Logger.getLogger(PolishMapDataSource.class); - private static final String READING_CHARSET = "iso-8859-1"; + private final Charset READING_CHARSET = StandardCharsets.ISO_8859_1; private static final int S_IMG_ID = 1; private static final int S_POINT = 2; @@ -127,20 +128,11 @@ @Override public void load(String name, boolean addBackground) throws FileNotFoundException { - Reader reader; - try { - reader = new InputStreamReader(Utils.openFile(name), READING_CHARSET); - } catch (UnsupportedEncodingException e) { - // Java is required to support iso-8859-1 so this is unlikely - throw new FormatException("Unrecognised charset " + READING_CHARSET); - } - // If no code page is given then we read labels in utf-8 - dec = Charset.forName("utf-8").newDecoder(); + dec = StandardCharsets.UTF_8.newDecoder(); dec.onUnmappableCharacter(CodingErrorAction.REPLACE); - - try (BufferedReader in = new BufferedReader(reader)){ + try (BufferedReader in = new BufferedReader(new InputStreamReader(Utils.openFile(name), READING_CHARSET))) { String line; while ((line = in.readLine()) != null) { ++lineNo; @@ -154,16 +146,16 @@ else processLine(line); } - - // Add all restrictions to the map after reading the full map. - // The reason being, the restrictions section appear in the beginning of the map. - // All the nodes will only be read later on. - // Required to pass the road helper instance as it contains all node data. - restrictionHelper.processAndAddRestrictions(roadHelper, mapper); } catch (IOException e) { throw new FormatException("Reading file failed", e); } + // Add all restrictions to the map after reading the full map. + // The reason being, the restrictions section appear in the beginning of the map. + // All the nodes will only be read later on. + // Required to pass the road helper instance as it contains all node data. + restrictionHelper.processAndAddRestrictions(roadHelper, mapper); + if (addBackground && !havePolygon4B) addBackground(); } @@ -712,17 +704,14 @@ */ private String recode(String value) { if (dec != null) { + // Get the bytes that were actually in the file. + byte[] bytes = value.getBytes(READING_CHARSET); + ByteBuffer buf = ByteBuffer.wrap(bytes); + try { - // Get the bytes that were actually in the file. - byte[] bytes = value.getBytes(READING_CHARSET); - ByteBuffer buf = ByteBuffer.wrap(bytes); - // Decode from bytes with the correct code page. CharBuffer out = dec.decode(buf); return out.toString(); - } catch (UnsupportedEncodingException e) { - // Java requires this support, so unlikely to happen - log.warn("no support for " + READING_CHARSET); } catch (CharacterCodingException e) { log.error("error decoding label", e); } Index: src/uk/me/parabola/mkgmap/srt/SrtTextReader.java =================================================================== --- src/uk/me/parabola/mkgmap/srt/SrtTextReader.java (revision 4413) +++ src/uk/me/parabola/mkgmap/srt/SrtTextReader.java (working copy) @@ -22,6 +22,7 @@ import java.nio.CharBuffer; import java.nio.charset.CharacterCodingException; import java.nio.charset.CharsetEncoder; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Paths; import java.util.ArrayList; @@ -107,7 +108,7 @@ } private SrtTextReader(String filename) throws IOException { - this(filename, new InputStreamReader(new FileInputStream(filename), "utf-8")); + this(filename, new InputStreamReader(new FileInputStream(filename), StandardCharsets.UTF_8)); } private SrtTextReader(String filename, Reader r) throws IOException { @@ -125,23 +126,22 @@ */ public static Sort sortForCodepage(int codepage) { String name = "sort/cp" + codepage + ".txt"; - InputStream is = Sort.class.getClassLoader().getResourceAsStream(name); - if (is == null) { - if (codepage == 1252) - throw new ExitException("No sort description for code-page 1252 available"); + try (InputStream is = Sort.class.getClassLoader().getResourceAsStream(name)) { + if (is == null) { + if (codepage == 1252) + throw new ExitException("No sort description for code-page 1252 available"); - Sort defaultSort = SrtTextReader.sortForCodepage(1252); - defaultSort.setCodepage(codepage); - defaultSort.setDescription("Default sort"); - return defaultSort; - } + Sort defaultSort = SrtTextReader.sortForCodepage(1252); + defaultSort.setCodepage(codepage); + defaultSort.setDescription("Default sort"); + return defaultSort; + } - try { - InputStreamReader r = new InputStreamReader(is, "utf-8"); + InputStreamReader r = new InputStreamReader(is, StandardCharsets.UTF_8); SrtTextReader sr = new SrtTextReader(r); return sr.getSort(); } catch (IOException e) { - return SrtTextReader.sortForCodepage(codepage); + return SrtTextReader.sortForCodepage(codepage); // ??? I don't understand this } }
_______________________________________________ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev