Skip to content

Commit 1b1306b

Browse files
AaronAsAChimpAaron Spaulding
andauthored
Fix language server crash when it encounters a file that is not UTF-8 (#258)
* Fix language server crash when it encounters a file that is not UTF-8 This PR fixes the crash by ignoring any file that throws an encoding exception while its being read. * Add test for Windows-1252 encoded source files. * Reset the FileStore before running tests so that there aren't any files that are carried over from previous tests. --------- Co-authored-by: Aaron Spaulding <aspaulding@idashboards.com>
1 parent a2a216f commit 1b1306b

5 files changed

Lines changed: 64 additions & 2 deletions

File tree

src/main/java/org/javacs/FileStore.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import java.io.*;
44
import java.net.URI;
5+
import java.nio.charset.CharacterCodingException;
56
import java.nio.file.*;
67
import java.nio.file.attribute.*;
78
import java.time.Instant;
@@ -88,6 +89,12 @@ static Collection<Path> all() {
8889
return javaSources.keySet();
8990
}
9091

92+
static void reset() {
93+
activeDocuments.clear();
94+
workspaceRoots.clear();
95+
javaSources.clear();
96+
}
97+
9198
static List<Path> list(String packageName) {
9299
var list = new ArrayList<Path>();
93100
for (var file : javaSources.keySet()) {
@@ -204,7 +211,7 @@ private static void readInfoFromDisk(Path file) {
204211
var time = Files.getLastModifiedTime(file).toInstant();
205212
var packageName = StringSearch.packageName(file);
206213
javaSources.put(file, new Info(time, packageName));
207-
} catch (NoSuchFileException e) {
214+
} catch (NoSuchFileException | CharacterCodingException e) {
208215
LOG.warning(e.getMessage());
209216
javaSources.remove(file);
210217
} catch (IOException e) {

src/main/java/org/javacs/StringSearch.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import java.nio.ByteBuffer;
66
import java.nio.channels.FileChannel;
77
import java.nio.charset.StandardCharsets;
8+
import java.nio.charset.CharacterCodingException;
89
import java.nio.file.*;
910
import java.util.*;
1011
import java.util.logging.Logger;
@@ -369,7 +370,7 @@ static String fileName(URI uri) {
369370
return parts[parts.length - 1];
370371
}
371372

372-
static String packageName(Path file) {
373+
static String packageName(Path file) throws CharacterCodingException {
373374
var packagePattern = Pattern.compile("^package +(.*);");
374375
var startOfClass = Pattern.compile("^[\\w ]*class +\\w+");
375376
try (var lines = FileStore.lines(file)) {
@@ -381,6 +382,8 @@ static String packageName(Path file) {
381382
return id;
382383
}
383384
}
385+
} catch (CharacterCodingException e) {
386+
throw e;
384387
} catch (IOException e) {
385388
throw new RuntimeException(e);
386389
}
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
package org.javacs.example;
2+
3+
// This file is intentionally not Unicode. It exists to test handling
4+
// non-unicode files.
5+
//
6+
// This file should be encoded using the Windows-1252 encoding
7+
8+
class EncodingWindows1252 {
9+
/**
10+
* This property tests for the support of non-unicode strings such as "ü".
11+
* Also this comment tests for support in doc comments.
12+
*/
13+
String uWithUmlaut = "ü";
14+
}
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
package org.javacs;
2+
3+
import static org.hamcrest.Matchers.equalTo;
4+
import static org.junit.Assert.assertThat;
5+
6+
import java.nio.file.Paths;
7+
import java.util.Set;
8+
import org.junit.Test;
9+
import org.junit.Before;
10+
11+
/**
12+
* These tests are isolated because bugs caused by encoding issues could cause
13+
* phantom failures in other tests.
14+
*/
15+
public class FileEncodingTest {
16+
17+
@Before
18+
public void resetSourcesBefore() {
19+
FileStore.reset();
20+
}
21+
22+
@Test
23+
public void packageNameForNonUnicodeSource() {
24+
var encodingTestRoot = Paths.get("src/test/examples/encoding").normalize();
25+
26+
// If an exception is thrown due to an unknown encoding it would
27+
// be here.
28+
FileStore.setWorkspaceRoots(Set.of(encodingTestRoot));
29+
30+
var file = FindResource.path("/org/javacs/example/EncodingWindows1252.java");
31+
32+
// Currently, non-unicode files are ignored. This test will change if
33+
// support is added in the future.
34+
assertThat(FileStore.suggestedPackageName(file), equalTo(""));
35+
}
36+
}

src/test/java/org/javacs/LanguageServerFixture.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ public void customNotification(String method, JsonElement params) {}
4949
}
5050

5151
static JavaLanguageServer getJavaLanguageServer(Path workspaceRoot, LanguageClient client) {
52+
FileStore.reset();
53+
5254
var server = new JavaLanguageServer(client);
5355
var init = new InitializeParams();
5456

0 commit comments

Comments
 (0)