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

Strip invalid prolog when loading CitationStyles #3404

Merged
merged 5 commits into from
Nov 5, 2017
Merged

Conversation

lenhard
Copy link
Member

@lenhard lenhard commented Nov 4, 2017

Fixes #3389 by skipping invalid parts of the prolog of citation styles.

  • Change in CHANGELOG.md described
  • Tests created for changes
  • Screenshots added (for bigger UI changes)
  • Manually tested changed features in running JabRef
  • Check documentation status (Issue created for outdated help page at help.jabref.org?)
  • If you changed the localization: Did you run gradle localizationUpdate?

@lenhard lenhard added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Nov 4, 2017
@@ -71,7 +71,9 @@ private static CitationStyle createCitationStyleFromSource(final String source,
String title = ((CharacterData) titleNode.item(0).getFirstChild()).getData();

return new CitationStyle(filename, title, source);
} catch (ParserConfigurationException | SAXException | IOException e) {
} catch (SAXException ignore) {
// is triggered when a use has not configured CitationStyles -> ignore
Copy link
Member

Choose a reason for hiding this comment

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

Is there no easy way to prevent the exception from being triggered when the user actually does not use citation styles (I suspect these null checks in the beginning should actually serve this job, although they fail obviously). Reason: I think these SAX errors also appears if a user uses a citation style but the parsing fails for some reason. In this case the logged error message is actually helpful.

If there is no easy way to prevent the exception, then I'm fine with merging this PR.

Copy link
Member Author

@lenhard lenhard Nov 5, 2017

Choose a reason for hiding this comment

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

You are right of course, I should rather track down the origin. The problem lies in the citationstyles. The first line of some of the styles is broken. For example, the style university-of-york-vancouver.csl looks like this ?<?xml version="1.0" encoding="utf-8"?> and is invalid because it starts with a question mark.

That should be fixed at the end of the citation styles. For now, I have pushed a fix on our side that removes all invalid content in the prolog, i.e. everything before the first <. That way all styles can be processed and the error is gone.

@lenhard lenhard changed the title Ignore SAXException when not using CitationStyles Strip invalid prolog when loading CitationStyles Nov 5, 2017
@LinusDietz LinusDietz merged commit ab517f4 into master Nov 5, 2017
@LinusDietz LinusDietz deleted the fix-saxexception branch November 5, 2017 19:20
Siedlerchr added a commit that referenced this pull request Nov 10, 2017
* upstream/master: (26 commits)
  Fix test for quoted lang messages (#3424)
  Update gradle from 4.3 to 4.3.1
  Fix #3411: ordering of fields in customized entry types works again (#3422)
  Backport of syncLang to python2 (#3420)
  Remove Versioneye badge
  Fix some error prone warnings
  Fix for issue #2721 append to a field (#3395)
  Fix travis - hopefully
  Remove 3.x changelog (#3250)
  Try to use hint of https://github.com/TheBoegl/shadow-log4j-transformer#usage-as-library
  Try to enable LGTM
  Update guava from 23.2 -> 23.3 (#3409)
  Update wiremock from 2.8.0 -> 2.10.1
  Move groups field from others to general (#3407)
  Fix checkstyle issues to repair build
  Fix #3046: No longer allow duplicate fields in customized entry types (#3405)
  Strip invalid prolog when loading CitationStyles (#3404)
  Integrity check "Abbreviation Detection" detects abbreviated names for journals and booktitles based on the internal list instead of only looking for "." signs. (#3362)
  Update gradle from 4.2.1 to 4.3
  Update gradle from 4.2.1 to 4.3
  ...
Siedlerchr added a commit that referenced this pull request Nov 18, 2017
* upstream/master: (23 commits)
  Feature java version check again (#3428)
  Fix test for quoted lang messages (#3424)
  Update gradle from 4.3 to 4.3.1
  Fix #3411: ordering of fields in customized entry types works again (#3422)
  Backport of syncLang to python2 (#3420)
  Remove Versioneye badge
  Fix some error prone warnings
  Fix for issue #2721 append to a field (#3395)
  Fix travis - hopefully
  Remove 3.x changelog (#3250)
  Try to use hint of https://github.com/TheBoegl/shadow-log4j-transformer#usage-as-library
  Try to enable LGTM
  Update guava from 23.2 -> 23.3 (#3409)
  Update wiremock from 2.8.0 -> 2.10.1
  Move groups field from others to general (#3407)
  Fix checkstyle issues to repair build
  Fix #3046: No longer allow duplicate fields in customized entry types (#3405)
  Strip invalid prolog when loading CitationStyles (#3404)
  Integrity check "Abbreviation Detection" detects abbreviated names for journals and booktitles based on the internal list instead of only looking for "." signs. (#3362)
  Update gradle from 4.2.1 to 4.3
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants