Skip to content
Open
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
2 changes: 1 addition & 1 deletion jablib/src/main/java/module-info.java
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@

// region: data mapping
requires jdk.xml.dom;
requires com.google.gson;
requires transitive com.google.gson;
requires tools.jackson.databind;
requires tools.jackson.dataformat.yaml;
requires tools.jackson.core;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package org.jabref.logic.cleanup;

import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.StringJoiner;

import org.jabref.model.metadata.SaveActionsDTO;

public class SaveActionsConverter {
public static FieldFormatterCleanupActions fromDTO(SaveActionsDTO saveActionsDTO) {
boolean enabled = saveActionsDTO.state;
StringBuilder actionsStringBuilder = new StringBuilder();
for (Map.Entry<String, List<String>> entry : saveActionsDTO.actions.entrySet()) {
StringJoiner joiner = new StringJoiner(",");
for (String formatter : entry.getValue()) {
joiner.add(formatter);
}
actionsStringBuilder.append(entry.getKey())
.append("[")
.append(joiner)
.append(']');
}
List<FieldFormatterCleanup> actions = FieldFormatterCleanupMapper.parseActions(actionsStringBuilder.toString());
return new FieldFormatterCleanupActions(enabled, actions);
}

public static SaveActionsDTO toDTO(FieldFormatterCleanupActions saveActions) {
SaveActionsDTO saveActionsDTO = new SaveActionsDTO();
saveActionsDTO.state = saveActions.isEnabled();
for (FieldFormatterCleanup action : saveActions.getConfiguredActions()) {
String field = action.getField().getName();
String formatter = action.getFormatter().getKey();
saveActionsDTO.actions
.computeIfAbsent(field, _ -> new ArrayList<>())
.add(formatter);
}
return saveActionsDTO;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,10 @@
import org.jabref.logic.cleanup.FieldFormatterCleanup;
import org.jabref.logic.cleanup.FieldFormatterCleanupActions;
import org.jabref.logic.cleanup.NormalizeWhitespacesCleanup;
import org.jabref.logic.cleanup.SaveActionsConverter;
import org.jabref.logic.formatter.bibtexfields.TrimWhitespaceFormatter;
import org.jabref.logic.importer.util.SaveActionsDTOConverter;
import org.jabref.logic.os.OS;
import org.jabref.logic.preferences.CliPreferences;
import org.jabref.logic.util.strings.StringUtil;
import org.jabref.model.FieldChange;
Expand All @@ -43,9 +46,13 @@
import org.jabref.model.entry.BibtexString;
import org.jabref.model.entry.field.InternalField;
import org.jabref.model.metadata.MetaData;
import org.jabref.model.metadata.SaveActionsDTO;
import org.jabref.model.metadata.SaveOrder;
import org.jabref.model.metadata.SelfContainedSaveOrder;

import com.google.gson.Gson;
import com.google.gson.GsonBuilder;
import com.google.gson.JsonObject;
import org.jooq.lambda.Unchecked;
import org.jspecify.annotations.NonNull;
import org.slf4j.Logger;
Expand Down Expand Up @@ -274,6 +281,29 @@ protected void writeMetaData(@NonNull MetaData metaData,
for (Map.Entry<String, String> metaItem : serializedMetaData.entrySet()) {
writeMetaDataItem(metaItem);
}

writeMetaDataJson(metaData);
}

protected void writeMetaDataJson(MetaData metaData) throws IOException {
JsonObject metaDataJson = new JsonObject();

if (metaData.getSaveActions().isPresent()) {
FieldFormatterCleanupActions saveActions = metaData.getSaveActions().get();
SaveActionsDTO saveActionsDTO = SaveActionsConverter.toDTO(saveActions);
JsonObject saveActionsJson = SaveActionsDTOConverter.toJson(saveActionsDTO);
metaDataJson.add(MetaData.SAVE_ACTIONS, saveActionsJson);
}

if (metaDataJson.isEmpty()) {
return;
}

Gson gson = new GsonBuilder().setPrettyPrinting().create();
bibWriter.write(COMMENT_PREFIX + "{" + MetaData.META_FLAG_V1 + OS.NEWLINE);
bibWriter.write(gson.toJson(metaDataJson));
bibWriter.writeLine(OS.NEWLINE + "}");
bibWriter.finishBlock();
}

protected void writeMetaDataItem(Map.Entry<String, String> metaItem) throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
import javax.xml.parsers.ParserConfigurationException;

import org.jabref.logic.bibtex.FieldWriter;
import org.jabref.logic.cleanup.FieldFormatterCleanupActions;
import org.jabref.logic.cleanup.SaveActionsConverter;
import org.jabref.logic.exporter.BibDatabaseWriter;
import org.jabref.logic.exporter.SaveConfiguration;
import org.jabref.logic.groups.GroupsFactory;
Expand All @@ -35,6 +37,7 @@
import org.jabref.logic.importer.Parser;
import org.jabref.logic.importer.ParserResult;
import org.jabref.logic.importer.util.MetaDataParser;
import org.jabref.logic.importer.util.SaveActionsDTOConverter;
import org.jabref.logic.l10n.Localization;
import org.jabref.logic.os.OS;
import org.jabref.model.database.BibDatabase;
Expand All @@ -53,13 +56,16 @@
import org.jabref.model.groups.GroupHierarchyType;
import org.jabref.model.groups.GroupTreeNode;
import org.jabref.model.metadata.MetaData;
import org.jabref.model.metadata.SaveActionsDTO;
import org.jabref.model.util.DummyFileUpdateMonitor;
import org.jabref.model.util.FileUpdateMonitor;

import com.dd.plist.BinaryPropertyListParser;
import com.dd.plist.NSArray;
import com.dd.plist.NSDictionary;
import com.dd.plist.NSString;
import com.google.gson.Gson;
import com.google.gson.JsonObject;
import io.github.adr.linked.ADR;
import org.jspecify.annotations.NonNull;
import org.slf4j.Logger;
Expand Down Expand Up @@ -113,6 +119,7 @@ public class BibtexParser implements Parser {

private ParserResult parserResult;
private final MetaDataParser metaDataParser;
private Optional<JsonObject> parsedJsonMetaData = Optional.empty();
private final Map<String, String> parsedBibDeskGroups;

private GroupTreeNode bibDeskGroupTreeNode;
Expand Down Expand Up @@ -295,6 +302,14 @@ private ParserResult parseFileContent() throws IOException {
);
}
parserResult.setMetaData(metaData);

parsedJsonMetaData.ifPresent(json -> {
if (json.has(MetaData.SAVE_ACTIONS)) {
SaveActionsDTO saveActionsDTO = SaveActionsDTOConverter.fromJson(json.getAsJsonObject(MetaData.SAVE_ACTIONS));
FieldFormatterCleanupActions saveActions = SaveActionsConverter.fromDTO(saveActionsDTO);
metaData.setSaveActions(saveActions);
}
});
} catch (ParseException exception) {
parserResult.addException(new ParserResult.Range(startLine, startColumn, line, column), exception);
}
Expand Down Expand Up @@ -401,9 +416,17 @@ private void parseJabRefComment(Map<String, String> meta) {
} catch (ParseException ex) {
parserResult.addException(new ParserResult.Range(startLine, startColumn, line, column), ex);
}
} else if (comment.startsWith(MetaData.META_FLAG_V1)) {
parsedJsonMetaData = parseCommentToJson(comment);
}
Comment on lines +419 to +421
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Action required

2. Json meta duplicated on save 🐞 Bug ✓ Correctness

META_FLAG_V1 (@Comment JSON) is parsed but not removed from the raw text buffer, so it can be stored
in the first entry’s parsed serialization and later written back verbatim. Since the writer also
always emits JSON metadata, saving can duplicate the JSON metadata block in the output file.
Agent Prompt
### Issue description
`META_FLAG_V1` JSON metadata comments are parsed but not removed from the parser’s preserved text buffer. This can cause the JSON `@Comment{jabref-meta-0.1.0 ...}` block to be stored as “user comments before the first entry” and later written back verbatim, while `BibDatabaseWriter` also writes a fresh JSON metadata block — resulting in duplicated JSON metadata after saving.

### Issue Context
Legacy JabRef metadata comments call `dumpTextReadSoFarToString()` to ensure metadata comments are rewritten by JabRef and not kept verbatim in the file. The JSON metadata branch should follow the same strategy.

### Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/importer/fileformat/BibtexParser.java[372-421]
- jablib/src/main/java/org/jabref/logic/importer/fileformat/BibtexParser.java[335-360]
- jablib/src/main/java/org/jabref/logic/bibtex/BibEntryWriter.java[68-74]

### Suggested approach
1. In the `comment.startsWith(MetaData.META_FLAG_V1)` branch, after parsing (and/or even on parse failure, consistent with how entrytype comments are treated), call `dumpTextReadSoFarToString()` so the comment is not retained as user text.
2. Consider adding a regression test that:
   - Parses a file containing only the JSON metadata comment + an entry.
   - Saves with `shouldReformatFile() == false` and the entry unchanged.
   - Asserts the output contains only one JSON metadata block (not one before the entry plus one in the metadata section).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

}

Optional<JsonObject> parseCommentToJson(String comment) {
Gson gson = new Gson();
String content = comment.substring(MetaData.META_FLAG_V1.length());
return Optional.ofNullable(gson.fromJson(content, JsonObject.class));
}
Comment on lines +424 to +428
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Action required

1. parsecommenttojson uncaught jsonsyntaxexception 📘 Rule violation ⛯ Reliability

JSON metadata parsing uses gson.fromJson(...) without handling malformed JSON, which can throw a
runtime exception and abort import. This violates the requirement for robust, contextual error
handling during parsing of external file content.
Agent Prompt
## Issue description
`parseCommentToJson` parses JSON from untrusted file content using `gson.fromJson` without handling malformed JSON, which can throw a runtime exception and crash parsing.

## Issue Context
This is part of BibTeX import logic; invalid/malformed metadata comments should not abort import and should instead be reported as a parser exception/warning with location/context.

## Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/importer/fileformat/BibtexParser.java[415-427]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


/// Adds BibDesk group entries to the JabRef database
private void addBibDeskGroupEntriesToJabRefGroups() {
for (String groupName : parsedBibDeskGroups.keySet()) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package org.jabref.logic.importer.util;

import java.util.ArrayList;
import java.util.List;
import java.util.Map;

import org.jabref.model.metadata.SaveActionsDTO;

import com.google.gson.JsonArray;
import com.google.gson.JsonElement;
import com.google.gson.JsonObject;

public class SaveActionsDTOConverter {
public static SaveActionsDTO fromJson(JsonObject saveActionsJson) {
SaveActionsDTO saveActionsDTO = new SaveActionsDTO();
saveActionsDTO.state = saveActionsJson.get("state").getAsBoolean();
for (Map.Entry<String, JsonElement> entry : saveActionsJson.entrySet()) {
// Already parsed before
if ("state".equals(entry.getKey())) {
continue;
}
List<String> actions = new ArrayList<>();
for (JsonElement action : entry.getValue().getAsJsonArray()) {
actions.add(action.getAsString());
}
saveActionsDTO.actions.put(entry.getKey(), actions);
}
return saveActionsDTO;
}

public static JsonObject toJson(SaveActionsDTO saveActionsDTO) {
JsonObject saveActionsJson = new JsonObject();
saveActionsJson.addProperty("state", saveActionsDTO.state);
for (Map.Entry<String, List<String>> entry : saveActionsDTO.actions.entrySet()) {
JsonArray formatters = new JsonArray();
for (String formatter : entry.getValue()) {
formatters.add(formatter);
}
saveActionsJson.add(entry.getKey(), formatters);
}
return saveActionsJson;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
public class MetaData {

public static final String META_FLAG = "jabref-meta: ";
public static final String META_FLAG_V1 = "jabref-meta-0.1.0";
public static final String ENTRYTYPE_FLAG = "jabref-entrytype: ";
public static final String SAVE_ORDER_CONFIG = "saveOrderConfig"; // ToDo: Rename in next major version to saveOrder, adapt testbibs
public static final String SAVE_ACTIONS = "saveActions";
Expand Down
10 changes: 10 additions & 0 deletions jablib/src/main/java/org/jabref/model/metadata/SaveActionsDTO.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package org.jabref.model.metadata;

import java.util.HashMap;
import java.util.List;
import java.util.Map;

public class SaveActionsDTO {
public boolean state = false;
public Map<String, List<String>> actions = new HashMap<>();
}
Original file line number Diff line number Diff line change
Expand Up @@ -752,13 +752,32 @@ void writeSaveActions() throws IOException {
databaseWriter.writePartOfDatabase(bibtexContext, List.of());

// The order should be kept (the cleanups are a list, not a set)
assertEquals("@Comment{jabref-meta: saveActions:enabled;"
+ OS.NEWLINE
+ "title[lower_case]" + OS.NEWLINE
+ "journal[title_case]" + OS.NEWLINE
+ "day[upper_case]" + OS.NEWLINE
+ ";}"
+ OS.NEWLINE, stringWriter.toString());
String expected = """
@Comment{jabref-meta: saveActions:enabled;
title[lower_case]
journal[title_case]
day[upper_case]
;}

@Comment{jabref-meta-0.1.0
{
"saveActions": {
"state": true,
"journal": [
"title_case"
],
"title": [
"lower_case"
],
"day": [
"upper_case"
]
}
}
}
""";

assertEquals(expected, stringWriter.toString());
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
import org.jabref.model.metadata.SaveOrder;
import org.jabref.model.metadata.UserHostInfo;

import com.google.gson.JsonObject;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -1310,6 +1311,28 @@ void integrationTestSaveActions() throws IOException {
saveActions.getConfiguredActions());
}

@Test
void integrationTestSaveActionsJson() throws IOException {
ParserResult parserResult = parser.parse(
Reader.of("""
@Comment{jabref-meta-0.1.0
{
"saveActions": {
"state": true,
"title": ["lower_case"]
}
}
}
"""));

FieldFormatterCleanupActions saveActions = parserResult.getMetaData().getSaveActions().get();

assertTrue(saveActions.isEnabled());
List<FieldFormatterCleanup> expected = List.of(new FieldFormatterCleanup(StandardField.TITLE, new LowerCaseFormatter()));
List<FieldFormatterCleanup> actual = saveActions.getConfiguredActions();
assertEquals(expected, actual);
}

@Test
void integrationTestBibEntryType() throws IOException {
ParserResult result = parser.parse(
Expand Down Expand Up @@ -2235,4 +2258,24 @@ void parseInvalidBibDeskFilesResultsInWarnings() throws IOException {

assertEquals(List.of(firstEntry, secondEntry), result.getDatabase().getEntries());
}

@Test
void parseCommentToJson() {
String comment = """
jabref-meta-0.1.0
{
"saveActions" :
{
"state": true
}
}
""";
BibtexParser parser = new BibtexParser(importFormatPreferences);
Optional<JsonObject> actualJson = parser.parseCommentToJson(comment);
JsonObject expectedSaveActions = new JsonObject();
expectedSaveActions.addProperty("state", true);
JsonObject expectedJson = new JsonObject();
expectedJson.add("saveActions", expectedSaveActions);
assertEquals(Optional.of(expectedJson), actualJson);
}
}
Loading