Skip to content

Fix NullPointerException in ConnectToSharedDatabaseDialog#2297

Merged
Siedlerchr merged 4 commits into
JabRef:masterfrom
obraliar:fixNPEinConnectToSharedDatabaseDialog
Nov 20, 2016
Merged

Fix NullPointerException in ConnectToSharedDatabaseDialog#2297
Siedlerchr merged 4 commits into
JabRef:masterfrom
obraliar:fixNPEinConnectToSharedDatabaseDialog

Conversation

@obraliar

@obraliar obraliar commented Nov 20, 2016

Copy link
Copy Markdown
Contributor

A NullPointerException occurred when leaving the file path field empty and connecting to a shared database.

Exception:

[AWT-EventQueue-0] ERROR net.sf.jabref.gui.shared.ConnectToSharedDatabaseDialog - Error while saving the database
java.lang.NullPointerException: null
        at java.util.prefs.AbstractPreferences.put(AbstractPreferences.java:241) ~[?:1.8.0_111]
        at net.sf.jabref.preferences.JabRefPreferences.put(JabRefPreferences.java:922) ~[main/:?]
        at net.sf.jabref.gui.exporter.SaveDatabaseAction.saveAs(SaveDatabaseAction.java:337) ~[main/:?]
...
  • Manually tested changed features in running JabRef

tobiasdiez
tobiasdiez requested changes Nov 20, 2016
View reviewed changes

@tobiasdiez tobiasdiez left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The changes look good, but please also add a file.getParent() != null check in net.sf.jabref.gui.exporter.SaveDatabaseAction.saveAs(SaveDatabaseAction.java:337

Is this NPE present in the released 3.7 version? If yes, please add a changelog entry.

@obraliar

obraliar commented Nov 20, 2016 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

There is already an isPresent check in init() and saveAs() which are calling saveAs(File file).

Is this NPE present in the released 3.7 version? If yes, please add a changelog entry.

Done.

@tobiasdiez

tobiasdiez commented Nov 20, 2016 •
edited
Loading

Copy link
Copy Markdown
Member

Yes, there is a check for the file itself, but not for its parent. Moreove, please try to write a changelog entry which is understandable for the average user (they don't know what a NPE is or care about a class called ConnectToSharedDatabaseDialog). Thanks

@tobiasdiez tobiasdiez added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Nov 20, 2016
@Siedlerchr

Siedlerchr commented Nov 20, 2016

Copy link
Copy Markdown
Member

LGTM! I merge it in

@Siedlerchr Siedlerchr merged commit fc70d01 into JabRef:master Nov 20, 2016
Siedlerchr added a commit that referenced this pull request Nov 27, 2016
@Siedlerchr
Merge remote-tracking branch 'upstream/master' into fileDirPattern
5af579a 
* upstream/master: (36 commits)
  Rework DBLPFetcher to new structure (#2314)
  Consider dev tag in version comparison (#2312)
  Update diffutils from 1.3.0 to 2.1.1
  Replace usages of Throwable with Exception (#2310)
  Add missing srcDir statement
  improve documentation of save order limitation - see #2305
  Adapt CSL tests to CRLF and LF (#2306)
  Use SPDX license identifiers
  Add key format of #2275 as requirement to CONTRIBUTING.md
  Add fallback exception handler (#2287)
  Update citeproc dependency to 1.0.1 (#2303)
  Remove duplicate code getResolvedFieldOrAlias (#2296)
  Update testCompile dependencies (mockito-core, wiremock)
  Change execution order (#2302)
  Use https for files.jabref.org
  Use compileJava instead of getdeps to decrease build time even more
  Update install4j from 6.1.1 to 6.1.3
  Fix local metadata synchronization
  Fix NullPointerException in ConnectToSharedDatabaseDialog (#2297)
  The error message "No suitable driver found" will not appear when JabRef connects on startup. (#2295)
  ...

# Conflicts:
#	src/main/java/net/sf/jabref/logic/util/io/FileUtil.java
koppor pushed a commit to JabRef/jabref-koppor that referenced this pull request Dec 7, 2016
@obraliar @koppor
Fix NullPointerException in ConnectToSharedDatabaseDialog (JabRef#2297)
2e34210
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Siedlerchr Siedlerchr Siedlerchr approved these changes

@tobiasdiez tobiasdiez tobiasdiez approved these changes

Assignees

No one assigned

Labels

status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@obraliar @tobiasdiez @Siedlerchr