ServiceRegistryJpaImpl.updateInternal can NPE with a null job

Steps to reproduce

There is no check that job is not null before it is accessed

protected Job updateInternal(EntityManager em, Job job) throws PersistenceException {
EntityTransaction tx = em.getTransaction();
try {
tx.begin();
JobJpaImpl fromDb;
fromDb = em.find(JobJpaImpl.class, job.getId());

Activity

Show:
Lukas Rohner
June 17, 2013, 9:04 AM

Can't be null, since this is an internal method and we know this.

Christopher Brooks
June 17, 2013, 3:17 PM

Lukas, I'm not really convinced that just because the method is protected we shouldn't verify parameters are not null. I agree it might be a pretty minor bug, but shouldn't all of our methods be hardened against NPE's unless there are significant performance considerations? This one bothers me more than others because it starts DB interactions and can NPE during those. Though we might be protected against bad data through transactions (a good design from whomever wrote it originally), I've seen some really buggy jdbc stuff in the past, and would worry that there is a chance for an orphaned DB connection. And the fix for this is pretty easy...

Christopher Brooks
June 17, 2013, 3:17 PM

I'll leave it closed for now, and we can just discuss it on the ticket before reopening it.

Greg Logan
October 29, 2019, 8:34 PM

While I agree this is suboptimal, let's leave this for now. Ideally we will rewrite the SR entirely in the semi-near future.

Assignee

Lukas Rohner

Reporter

Christopher Brooks

Severity

Incorrectly Functioning With Workaround

Tags (folksonomy)

Components

Affects versions

Priority

Major
Configure