<< Back to previous view

[CLJ-1380] Three-arg ExceptionInfo constructor permits nil data Created: 13/Mar/14  Updated: 12/Oct/15  Resolved: 12/Oct/15

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: Release 1.8

Type: Defect Priority: Minor
Reporter: Gordon Syme Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: checkargs, ft

Attachments: File clj-1380.diff     Text File clj-1380-v2.patch     Text File clj-1380-v3.patch    
Patch: Code and Test
Approval: Ok


The argument check in the two-arg clojure.lang.ExceptionInfo constructor isn't present in the three-arg constructor so it's possible to create an ExceptionInfo with arbitrary (or nil) data.


user=> (clojure-version)

user=> (ex-info "hi" nil)
IllegalArgumentException Additional data must be a persistent map: null  clojure.lang.ExceptionInfo.<init> (ExceptionInfo.java:26)

user=> (ex-info "hi" nil (Throwable.))
NullPointerException   clojure.lang.ExceptionInfo.toString (ExceptionInfo.java:40)

Patch: clj-1380-v3.patch

Screened by: Alex Miller

Comment by Gordon Syme [ 13/Mar/14 10:47 AM ]

Sorry, didn't meant to classify as "major" and I don't have permissions to edit.

Comment by Gordon Syme [ 13/Mar/14 11:11 AM ]

Patch + tests

I'm not at all familiar with the project so may have put tests in the wrong language and/or wrong place.

The ex-info-works test is a bit dorky but shows that both constructors are equivalent (and passes without the patch to ExceptionInfo).

Comment by Alex Miller [ 13/Mar/14 12:18 PM ]

No worries on the classification - I adjust most incoming tickets in some way or another.

Thanks for the patch, however it cannot be considered unless you complete the Clojure Contributor's Agreement - http://clojure.org/contributing. This is an important step in the process that keeps the Clojure codebase on a sound legal basis.

Someone else could develop a clean room patch implementation for this ticket later, but of course it would be ideal if you could become a contributor!

Comment by Gordon Syme [ 13/Mar/14 1:15 PM ]

Hi Alex,

sure, that makes sense. I'll get the contributor's agreement in the post. It may take a while to arrive since I'm based in Europe.

Comment by Gordon Syme [ 25/Mar/14 10:03 AM ]

I just checked http://clojure.org/contributing, looks like my CCA made it through

Comment by Andy Fingerhut [ 01/Oct/14 6:48 PM ]

Gordon, I do not know if your patch is of interest to the Clojure developers, so I can't comment on that aspect of this ticket.

Instructions for creating a patch in the expected format is given on the wiki page below. Your patch is not in the expected format.


Comment by Gordon Syme [ 27/Oct/14 5:30 AM ]

Whoops, sorry Andy.

I've rebased against master and added a correctly formatted patch.

Comment by Michael Blume [ 05/Aug/15 5:51 PM ]

Rebased onto current master (1d5237)

Comment by Alex Miller [ 07/Aug/15 9:25 AM ]

Can we change the data instanceof IPersistentMap check to data != null (which is the real thing being checked? If something non-null and not IPersistentMap is passed, a ClassCastException will be thrown on invocation.

The error message should then be modified as well to something like "Additional data must be non-nil."

Comment by Gordon Syme [ 07/Aug/15 3:03 PM ]

Done and done.
Throws on null rather than instanceof failure and updated the error message

Generated at Sat Nov 28 09:07:42 CST 2015 using JIRA 4.4#649-r158309.