Skip to content
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.

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

Fix handling of URL in file field #7347

Merged
merged 8 commits into from
Jan 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
- We fixed an issue where the "Find unlinked files" dialog would freeze JabRef on importing. [#7205](https://github.com/JabRef/jabref/issues/7205)
- We fixed an issue where the "Find unlinked files" would stop importing when importing a single file failed. [#7206](https://github.com/JabRef/jabref/issues/7206)
- We fixed an issue where an exception would be displayed for previewing and preferences when a custom theme has been configured but is missing [#7177](https://github.com/JabRef/jabref/issues/7177)
- We fixed an issue where the regex based file search miss-interpreted specific symbols [#4342](https://github.com/JabRef/jabref/issues/4342)
- We fixed an issue where URLs in `file` fields could not be handled on Windows. [#7359](https://github.com/JabRef/jabref/issues/7359)
- We fixed an issue where the regex based file search miss-interpreted specific symbols. [#4342](https://github.com/JabRef/jabref/issues/4342)
- We fixed an issue where the Harvard RTF exporter used the wrong default file extension. [4508](https://github.com/JabRef/jabref/issues/4508)
- We fixed an issue where the Harvard RTF exporter did not use the new authors formatter and therefore did not export "organization" authors correctly. [4508](https://github.com/JabRef/jabref/issues/4508)
- We fixed an issue where the field `urldate` was not exported to the corresponding fields `YearAccessed`, `MonthAccessed`, `DayAccessed` in MS Office XML [#7354](https://github.com/JabRef/jabref/issues/7354)
Expand Down
53 changes: 38 additions & 15 deletions src/main/java/org/jabref/logic/importer/util/FileFieldParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import java.net.MalformedURLException;
import java.net.URL;
import java.nio.file.InvalidPathException;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;
Expand All @@ -17,7 +18,7 @@ public static List<LinkedFile> parse(String value) {
return files;
}

List<String> entry = new ArrayList<>();
List<String> linkedFileData = new ArrayList<>();
StringBuilder sb = new StringBuilder();
boolean inXmlChar = false;
boolean escaped = false;
Expand All @@ -39,30 +40,38 @@ public static List<LinkedFile> parse(String value) {
sb.append(c);
inXmlChar = false;
} else if (!escaped && (c == ':')) {
entry.add(sb.toString());
// We are in the next LinkedFile data element
linkedFileData.add(sb.toString());
sb = new StringBuilder();
} else if (!escaped && (c == ';') && !inXmlChar) {
entry.add(sb.toString());
sb = new StringBuilder();
linkedFileData.add(sb.toString());
files.add(convert(linkedFileData));

files.add(convert(entry));
// next iteration
sb = new StringBuilder();
} else {
sb.append(c);
}
escaped = false;
}
if (sb.length() > 0) {
entry.add(sb.toString());
linkedFileData.add(sb.toString());
}

if (!entry.isEmpty()) {
files.add(convert(entry));
if (!linkedFileData.isEmpty()) {
files.add(convert(linkedFileData));
}

return files;
}

private static LinkedFile convert(List<String> entry) {
/**
* Converts the given textual representation of a LinkedFile object
*
* SIDE EFFECT: The given entry list is cleared upon completion
*
* @param entry the list of elements in the linked file textual representation
* @return a LinkedFile object
*/
static LinkedFile convert(List<String> entry) {
// ensure list has at least 3 fields
while (entry.size() < 3) {
entry.add("");
Expand All @@ -71,17 +80,31 @@ private static LinkedFile convert(List<String> entry) {
LinkedFile field = null;
if (LinkedFile.isOnlineLink(entry.get(1))) {
try {
field = new LinkedFile(new URL(entry.get(1)), entry.get(2));
field = new LinkedFile(entry.get(0), new URL(entry.get(1)), entry.get(2));
} catch (MalformedURLException ignored) {
// ignored
// in case the URL is malformed, store it nevertheless
field = new LinkedFile(entry.get(0), entry.get(1), entry.get(2));
}
}

if (field == null) {
field = new LinkedFile(entry.get(0), Path.of(entry.get(1)), entry.get(2));
String pathStr = entry.get(1);
if (pathStr.contains("//")) {
// In case the path contains //, we assume it is a malformed URL, not a malformed path.
// On linux, the double slash would be converted to a single slash.
field = new LinkedFile(entry.get(0), pathStr, entry.get(2));
} else {
try {
// there is no Path.isValidPath(String) method
Path path = Path.of(pathStr);
field = new LinkedFile(entry.get(0), path, entry.get(2));
} catch (InvalidPathException e) {
field = new LinkedFile(entry.get(0), pathStr, entry.get(2));
}
}
}

// link is only mandatory field
// link is the only mandatory field
if (field.getDescription().isEmpty() && field.getLink().isEmpty() && !field.getFileType().isEmpty()) {
field = new LinkedFile("", Path.of(field.getFileType()), "");
} else if (!field.getDescription().isEmpty() && field.getLink().isEmpty() && field.getFileType().isEmpty()) {
Expand Down
17 changes: 13 additions & 4 deletions src/main/java/org/jabref/model/entry/LinkedFile.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,24 @@ public class LinkedFile implements Serializable {
private transient StringProperty fileType = new SimpleStringProperty();

public LinkedFile(String description, Path link, String fileType) {
this(Objects.requireNonNull(description), Objects.requireNonNull(link).toString(), Objects.requireNonNull(fileType));
}

/**
* Constructor for non-valid paths. We need to parse them, because the GUI needs to render it.
*/
public LinkedFile(String description, String link, String fileType) {
this.description.setValue(Objects.requireNonNull(description));
setLink(Objects.requireNonNull(link).toString());
setLink(link);
this.fileType.setValue(Objects.requireNonNull(fileType));
}

public LinkedFile(URL link, String fileType) {
this.description.setValue("");
setLink(Objects.requireNonNull(link).toString());
this.fileType.setValue(Objects.requireNonNull(fileType));
this("", Objects.requireNonNull(link).toString(), Objects.requireNonNull(fileType));
}

public LinkedFile(String description, URL link, String fileType) {
this(description, Objects.requireNonNull(link).toString(), Objects.requireNonNull(fileType));
}

public StringProperty descriptionProperty() {
Expand Down
129 changes: 4 additions & 125 deletions src/test/java/org/jabref/logic/bibtex/FileFieldWriterTest.java
Original file line number Diff line number Diff line change
@@ -1,13 +1,7 @@
package org.jabref.logic.bibtex;

import java.net.MalformedURLException;
import java.net.URL;
import java.nio.file.Path;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;

import org.jabref.logic.importer.util.FileFieldParser;
import org.jabref.model.entry.LinkedFile;

import org.junit.jupiter.api.Test;
Expand All @@ -17,121 +11,6 @@

public class FileFieldWriterTest {

@Test
public void emptyListForEmptyInput() {
String emptyInput = "";

assertEquals(Collections.emptyList(), FileFieldParser.parse(emptyInput));
assertEquals(Collections.emptyList(), FileFieldParser.parse(null));
}

@Test
public void parseCorrectInput() {
String input = "Desc:File.PDF:PDF";

assertEquals(
Collections.singletonList(new LinkedFile("Desc", Path.of("File.PDF"), "PDF")),
FileFieldParser.parse(input));
}

@Test
public void parseCorrectOnlineInput() throws MalformedURLException {
String input = ":http\\://arxiv.org/pdf/2010.08497v1:PDF";
String inputURL = "http://arxiv.org/pdf/2010.08497v1";
List<LinkedFile> expected = Collections.singletonList(new LinkedFile(new URL(inputURL), "PDF"));

assertEquals(expected, FileFieldParser.parse(input));
}

@Test
public void parseFaultyOnlineInput() {
String input = ":htt\\://arxiv.org/pdf/2010.08497v1:PDF";
String inputURL = "htt://arxiv.org/pdf/2010.08497v1";
List<LinkedFile> expected = Collections.singletonList(new LinkedFile("", Path.of(inputURL), "PDF"));

assertEquals(expected, FileFieldParser.parse(input));
}

@Test
public void ingoreMissingDescription() {
String input = ":wei2005ahp.pdf:PDF";

assertEquals(
Collections.singletonList(new LinkedFile("", Path.of("wei2005ahp.pdf"), "PDF")),
FileFieldParser.parse(input));
}

@Test
public void interpreteLinkAsOnlyMandatoryField() {
String single = "wei2005ahp.pdf";
String multiple = "wei2005ahp.pdf;other.pdf";

assertEquals(
Collections.singletonList(new LinkedFile("", Path.of("wei2005ahp.pdf"), "")),
FileFieldParser.parse(single));

assertEquals(
Arrays.asList(
new LinkedFile("", Path.of("wei2005ahp.pdf"), ""),
new LinkedFile("", Path.of("other.pdf"), "")),
FileFieldParser.parse(multiple));
}

@Test
public void escapedCharactersInDescription() {
String input = "test\\:\\;:wei2005ahp.pdf:PDF";

assertEquals(
Collections.singletonList(new LinkedFile("test:;", Path.of("wei2005ahp.pdf"), "PDF")),
FileFieldParser.parse(input));
}

@Test
public void handleXmlCharacters() {
String input = "test&#44\\;st\\:\\;:wei2005ahp.pdf:PDF";

assertEquals(
Collections.singletonList(new LinkedFile("test&#44;st:;", Path.of("wei2005ahp.pdf"), "PDF")),
FileFieldParser.parse(input));
}

@Test
public void handleEscapedFilePath() {
String input = "desc:C\\:\\\\test.pdf:PDF";

assertEquals(
Collections.singletonList(new LinkedFile("desc", Path.of("C:\\test.pdf"), "PDF")),
FileFieldParser.parse(input));
}

@Test
public void subsetOfFieldsResultsInFileLink() {
String descOnly = "file.pdf::";
String fileOnly = ":file.pdf";
String typeOnly = "::file.pdf";

assertEquals(
Collections.singletonList(new LinkedFile("", Path.of("file.pdf"), "")),
FileFieldParser.parse(descOnly));

assertEquals(
Collections.singletonList(new LinkedFile("", Path.of("file.pdf"), "")),
FileFieldParser.parse(fileOnly));

assertEquals(
Collections.singletonList(new LinkedFile("", Path.of("file.pdf"), "")),
FileFieldParser.parse(typeOnly));
}

@Test
public void tooManySeparators() {
String input = "desc:file.pdf:PDF:asdf";

assertEquals(
Collections.singletonList(new LinkedFile("desc", Path.of("file.pdf"), "PDF")),
FileFieldParser.parse(input));
}

@Test
public void testQuoteStandard() {
assertEquals("a", FileFieldWriter.quote("a"));
Expand All @@ -154,10 +33,10 @@ public void testQuoteNull() {

@Test
public void testEncodeStringArray() {
assertEquals("a:b;c:d", FileFieldWriter.encodeStringArray(new String[][]{{"a", "b"}, {"c", "d"}}));
assertEquals("a:;c:d", FileFieldWriter.encodeStringArray(new String[][]{{"a", ""}, {"c", "d"}}));
assertEquals("a:" + null + ";c:d", FileFieldWriter.encodeStringArray(new String[][]{{"a", null}, {"c", "d"}}));
assertEquals("a:\\:b;c\\;:d", FileFieldWriter.encodeStringArray(new String[][]{{"a", ":b"}, {"c;", "d"}}));
assertEquals("a:b;c:d", FileFieldWriter.encodeStringArray(new String[][] {{"a", "b"}, {"c", "d"}}));
assertEquals("a:;c:d", FileFieldWriter.encodeStringArray(new String[][] {{"a", ""}, {"c", "d"}}));
assertEquals("a:" + null + ";c:d", FileFieldWriter.encodeStringArray(new String[][] {{"a", null}, {"c", "d"}}));
assertEquals("a:\\:b;c\\;:d", FileFieldWriter.encodeStringArray(new String[][] {{"a", ":b"}, {"c;", "d"}}));
}

@Test
Expand Down
Loading